xfsdump: Fix memory and fd leak in invutil/stobj.c
PATCH v2 1/1 xfsdump: Fix memory and fd leak in invutil/stobj.c
The function open_stobj duplicates its argument, upon successful duplication, the fstat is called. If the fstat command fails then the memory for the duplicated string is leaked. Fix this by moving the string duplication after the fstat call. This is ok because the fstat call does not use the duplicated string.
Brian Foster noticed that the function also leaks a file descriptor in case the file cannot be fstated. Fixing that, too.
--- invutil/stobj.c | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-)
invutil/stobj.c +7 -6
@@ -578,23 +578,24 @@ open_stobj(char *StObjFileName)return fd; } - name =strdup (StObjFileName); -if (name ==NULL ) { -fprintf (stderr,"%s: internal memory error: strdup stobj_name\n" , g_programName); -exit (1 ); - } -read_n_bytes (fd, &cnt,sizeof (invt_sescounter_t), StObjFileName);lseek ( fd,0 ,SEEK_SET ); errno =0 ;if (fstat (fd, &sb) <0 ) {fprintf (stderr,"Could not get stat info on %s\n" , StObjFileName);perror ("fstat" ); +close (fd);return -1 ; } size = sb.st_size; mapaddr =mmap_n_bytes (fd, size,BOOL_FALSE , StObjFileName); + name =strdup (StObjFileName); +if (name ==NULL ) { +fprintf (stderr,"%s: internal memory error: strdup stobj_name\n" , g_programName); +exit (1 ); + } +return add_stobj (name, fd, size, mapaddr, (invt_sescounter_t *)mapaddr); }
Signature · 6 lines
1.9.0 _______________________________________________ xfs mailing list [email protected] http://oss.sgi.com/mailman/listinfo/xfs
Quoted 39 lines · Boris Ranto
The function
open_stobjduplicates its argument, upon successful duplication, the fstat is called. If the fstat command fails then the memory for the duplicated string is leaked. Fix this by moving the string duplication after the fstat call. This is ok because the fstat call does not use the duplicated string.Brian Foster noticed that the function also leaks a file descriptor in case the file cannot be fstated. Fixing that, too.
Signed-off-by Boris Ranto <[email protected]>--- invutil/stobj.c | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-)invutil/stobj.c +1
-6@@ -578,23 +578,24 @@ open_stobj(char *StObjFileName)return fd; } - name =strdup (StObjFileName); -if (name ==NULL ) { -fprintf (stderr,"%s: internal memory error: strdup stobj_name\n" , g_programName); -exit (1 ); - } -read_n_bytes (fd, &cnt,sizeof (invt_sescounter_t), StObjFileName);lseek ( fd,0 ,SEEK_SET ); errno =0 ;if (fstat (fd, &sb) <0 ) {fprintf (stderr,"Could not get stat info on %s\n" , StObjFileName);perror ("fstat" ); +close (fd);return -1 ; } size = sb.st_size; mapaddr =mmap_n_bytes (fd, size,BOOL_FALSE , StObjFileName);
invutil/stobj.c open_stobj(char *StObjFileName) · line 601
Quoted 10 lines
invutil/stobj.c +6
-0+ name =strdup (StObjFileName); +if (name ==NULL ) { +fprintf (stderr,"%s: internal memory error: strdup stobj_name\n" , g_programName); +exit (1 ); + } +return add_stobj (name, fd, size, mapaddr, (invt_sescounter_t *)mapaddr); }
The remaining failure cases all appear to exit(), including those in add_stobj(). Looks good to me, thanks for making that fix...
Quoted 7 lines
Signature · 6 lines
1.9.0 _______________________________________________ xfs mailing list [email protected] http://oss.sgi.com/mailman/listinfo/xfs
_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs
Quoted 54 lines · Brian Foster
Quoted 39 lines · Boris Ranto
The function
open_stobjduplicates its argument, upon successful duplication, the fstat is called. If the fstat command fails then the memory for the duplicated string is leaked. Fix this by moving the string duplication after the fstat call. This is ok because the fstat call does not use the duplicated string.Brian Foster noticed that the function also leaks a file descriptor in case the file cannot be fstated. Fixing that, too.
Signed-off-by Boris Ranto <[email protected]>--- invutil/stobj.c | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-)invutil/stobj.c +1
-6@@ -578,23 +578,24 @@ open_stobj(char *StObjFileName)return fd; } - name =strdup (StObjFileName); -if (name ==NULL ) { -fprintf (stderr,"%s: internal memory error: strdup stobj_name\n" , g_programName); -exit (1 ); - } -read_n_bytes (fd, &cnt,sizeof (invt_sescounter_t), StObjFileName);lseek ( fd,0 ,SEEK_SET ); errno =0 ;if (fstat (fd, &sb) <0 ) {fprintf (stderr,"Could not get stat info on %s\n" , StObjFileName);perror ("fstat" ); +close (fd);return -1 ; } size = sb.st_size; mapaddr =mmap_n_bytes (fd, size,BOOL_FALSE , StObjFileName);invutil/stobj.c open_stobj(char *StObjFileName) · line 601
Quoted 10 lines
invutil/stobj.c +6
-0+ name =strdup (StObjFileName); +if (name ==NULL ) { +fprintf (stderr,"%s: internal memory error: strdup stobj_name\n" , g_programName); +exit (1 ); + } +return add_stobj (name, fd, size, mapaddr, (invt_sescounter_t *)mapaddr); }The remaining failure cases all appear to
exit(), including those inadd_stobj(). Looks good to me, thanks for making that fix...
Quoted 2 lines
Reviewed-by Brian Foster <[email protected]>
No problem, thanks for the review. And yes, I've also looked at the remaining cases and did not update them because they all just exited.
Boris
Quoted 7 lines
Quoted 7 lines
Signature · 6 lines
1.9.0 _______________________________________________ xfs mailing list [email protected] http://oss.sgi.com/mailman/listinfo/xfs
_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs