Ubuntu

Merge lp:~akkzilla/ubuntu/natty/gmemusage/gmemusage-fix-370735 into lp:ubuntu/natty/gmemusage

Proposed by Akkana Peck on 2011-05-16
Status: Work in progress
Proposed branch: lp:~akkzilla/ubuntu/natty/gmemusage/gmemusage-fix-370735
Merge into: lp:ubuntu/natty/gmemusage
Diff against target: 115 lines (+18/-7) 5 files modified
To merge this branch: bzr merge lp:~akkzilla/ubuntu/natty/gmemusage/gmemusage-fix-370735
Reviewer Review Type Date Requested Status
Chris Coulson (community) Needs Fixing on 2011-06-06
Marc Deslauriers Needs Fixing on 2011-05-27
Ubuntu branches 2011-05-16 Pending
Review via email: mp+61171@code.launchpad.net

Description of the Change

Merge patch that's been sitting in bug 370735, so gmemusage will no longer crash on startup with a stack-smashing error.

To post a comment you must log in.
Marc Deslauriers (mdeslaur) wrote :

The gmemusage uses the CDBS patch system. Please add the fix as a proper patch.

review: Needs Fixing
Chris Coulson (chrisccoulson) wrote :

Oh, I didn't see this had a merge proposal too. I'll add my comment from the bug report in here too:

Hi,

Thanks for working on this. I have a couple of comments though:

- The scope of the patch seems to expand beyond the original bug. Whilst it's perfectly ok for you to fix the additional issues, would you mind splitting those changes in to a separate patch? (I'm referring to the cast you added in draw_window and the prototype change to main() ).

- You hardcode "%15s" in the format string for sscanf, when you use a constant everywhere else. Why not do something like this?

     char *format ;
     asprintf ( &format , "%%*s %%%ds" , PROCNAMELEN - 1 ) ;
     sscanf ( buf , format , procName ) ;
     free ( format ) ;

(Note that this doesn't make sure asprintf succeeded, which isn't good - but it doesn't look like this application checks that any other call succeeds either, so I'm not sure that matters so much here).

Let me know what you think, but I'd certainly sponsor this once you've addressed the first point

review: Needs Fixing

Unmerged revisions

5. By Akkana Peck on 2011-05-16

Fix stack smashing crash on startup, bug 370735.

Preview Diff

1=== modified file 'common.h'
2--- common.h 2002-04-11 20:54:15 +0000
3+++ common.h 2011-05-16 20:18:30 +0000
4@@ -8,11 +8,12 @@
5 */
6 #define kernelname "linux"
7 #define freename "free"
8+#define PROCNAMELEN 16
9
10 struct ProcInfo
11 {
12 char
13- procname [14] ;
14+ procname [PROCNAMELEN] ;
15 int
16 totMem ,
17 totRSS ,
18
19=== modified file 'debian/changelog'
20--- debian/changelog 2008-11-28 14:18:17 +0000
21+++ debian/changelog 2011-05-16 20:18:30 +0000
22@@ -1,3 +1,9 @@
23+gmemusage (0.2-11ubuntu1) natty; urgency=low
24+
25+ * Fix stack smashing crash on startup, bug 370735.
26+
27+ -- Akkana Peck <akkana@shallowsky.com> Mon, 16 May 2011 12:54:25 -0700
28+
29 gmemusage (0.2-11) unstable; urgency=low
30
31 * Adopt the package (Closes: #486759).
32
33=== modified file 'gmemusage.c'
34--- gmemusage.c 2008-11-28 14:18:17 +0000
35+++ gmemusage.c 2011-05-16 20:18:30 +0000
36@@ -92,7 +92,7 @@
37 void draw_window ( void ) ;
38 void TooSmall ( Window win , GC gc , XFontStruct *font_into ) ;
39
40-void main ( int argc , char **argv )
41+int main ( int argc , char **argv )
42 {
43 int
44 default_height ,
45@@ -700,7 +700,7 @@
46 /*
47 * We can accept alarms again
48 */
49- (void) signal ( SIGALRM , draw_window ) ;
50+ (void) signal ( SIGALRM , (__sighandler_t)draw_window ) ;
51 return ;
52 }
53
54
55=== modified file 'hash.c'
56--- hash.c 2002-04-11 20:54:15 +0000
57+++ hash.c 2011-05-16 20:18:30 +0000
58@@ -7,6 +7,8 @@
59 * See file COPYING (included in this distribution) for copyright information.
60 */
61 #include <stdio.h>
62+#include <stdlib.h>
63+#include <string.h>
64 #include <malloc.h>
65 #include "common.h"
66
67@@ -71,7 +73,7 @@
68 }
69 /* printf("allocated %d procs\n",lastallocated); */
70 thisproc = nextproc = procs ;
71- strcpy ( thisproc -> procname , procname ) ;
72+ strncpy ( thisproc -> procname , procname , PROCNAMELEN ) ;
73 thisproc -> totMem = mem ;
74 thisproc -> totRSS = rss ;
75 thisproc -> nProcs = 1 ;
76@@ -98,7 +100,7 @@
77 /* printf("allocated %d procs\n",lastallocated); */
78 }
79 thisproc = procs + nProcs ;
80- strcpy ( thisproc -> procname , procname ) ;
81+ strncpy ( thisproc -> procname , procname , PROCNAMELEN ) ;
82 thisproc -> totMem = mem ;
83 thisproc -> totRSS = rss ;
84 thisproc -> nProcs = 1 ;
85
86=== modified file 'proc.c'
87--- proc.c 2008-11-28 14:18:17 +0000
88+++ proc.c 2011-05-16 20:18:30 +0000
89@@ -7,6 +7,7 @@
90 * See file COPYING (included in this distribution) for copyright information.
91 */
92 #include <stdio.h>
93+#include <stdlib.h>
94 #include <string.h>
95 #include <sys/types.h>
96 #include <dirent.h>
97@@ -93,7 +94,7 @@
98 char
99 buf [128] ;
100 char
101- procName [14] ;
102+ procName [PROCNAMELEN] ;
103 int
104 procSize ,
105 procRSS ,
106@@ -164,7 +165,8 @@
107 if ( !strncmp ( buf , NameLine , NameLineLen ) )
108 {
109 /* Name: procName */
110- sscanf ( buf , "%*s %s" , procName ) ;
111+ /* The field width here should correspond to PROCNAMELEN-1 */
112+ sscanf ( buf , "%*s %15s" , procName ) ;
113 }
114 else if ( !strncmp ( buf , VmSizeLine , VmSizeLineLen ) )
115 {

Subscribers

People subscribed via source and target branches

to all changes: