xfs
[Top] [All Lists]

Re: [PATCH v2] xfsdump: Fix memory and fd leak in invutil/stobj.c

To: Brian Foster <bfoster@xxxxxxxxxx>
Subject: Re: [PATCH v2] xfsdump: Fix memory and fd leak in invutil/stobj.c
From: Boris Ranto <ranto.boris@xxxxxxxxx>
Date: Tue, 11 Mar 2014 14:55:42 +0100
Cc: xfs@xxxxxxxxxxx, Boris Ranto <branto@xxxxxxxxxx>
Delivered-to: xfs@xxxxxxxxxxx
Dkim-signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=message-id:subject:from:to:cc:date:in-reply-to:references :content-type:mime-version:content-transfer-encoding; bh=JW5TyJuI0pmraEN+HdCrO/1pHWb0RKyhYnEqPcFp1Q4=; b=ypYFr8NkSkvx1U15oxCKfhh/qI4X4YdyhLuvIqap2654uuH+pDpPofCQ0d5A4qDxWa /69Znaprmu/VR08e2jWHwODrTj0ebdJ8Tu88Pga1df3Y5GoWXDACOV9xrtf49aYUcU7C RMuQhtrdzUi1eTqLiY4I9LJCBbJfcXF0X3YYvBHlRX49Ygn1NLVU0fvoW62xYVpf4yUX 7NX5q0/sQ8mVLBZnc3Xhxij3R9yUFyotZ8yRrMRXchKVkO5B4pl8gGXq6nwvXEdQs2KR UmuP4kdvVYsN8KGqGgjJpKgOZIpafNYaTWxTKNDv+1qAS2vg1xcClArSNjbeUWWbe0se fE+w==
In-reply-to: <20140307184812.GA19284@xxxxxxxxxxxxxx>
References: <1394203962-17080-1-git-send-email-ranto.boris@xxxxxxxxx> <20140307184812.GA19284@xxxxxxxxxxxxxx>
On Fri, 2014-03-07 at 13:48 -0500, Brian Foster wrote:
> On Fri, Mar 07, 2014 at 03:52:42PM +0100, Boris Ranto wrote:
> > 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.
> > 
> > Signed-off-by: Boris Ranto <ranto.boris@xxxxxxxxx>
> > ---
> >  invutil/stobj.c | 13 +++++++------
> >  1 file changed, 7 insertions(+), 6 deletions(-)
> > 
> > diff --git a/invutil/stobj.c b/invutil/stobj.c
> > index 428b419..a74ba0f 100644
> > --- a/invutil/stobj.c
> > +++ b/invutil/stobj.c
> > @@ -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);
> >  }
> >  
> 
> The remaining failure cases all appear to exit(), including those in
> add_stobj(). Looks good to me, thanks for making that fix...
> 
> Reviewed-by: Brian Foster <bfoster@xxxxxxxxxx>
> 

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

> > -- 
> > 1.9.0
> > 
> > _______________________________________________
> > xfs mailing list
> > xfs@xxxxxxxxxxx
> > http://oss.sgi.com/mailman/listinfo/xfs


<Prev in Thread] Current Thread [Next in Thread>