Re: Memleak in glob()

[Tim Rühsen]

Hi Bruno,

On Sonntag, 2. Juli 2017 02:34:17 CEST Bruno Haible wrote:
> Hi Tim,
> 
> > Attached is a patch fixing the issue for me
> 
> The 'return 0;' in line 1104 is correct, because 'dirname' is stuffed into
> the result array in the lines before. Therefore dirname must NOT be freed
> here.

This is right for one path, but not all the paths before 'return 0' stuff 
'dirname' into that array.
IMO, the correct approach would be to reset 'malloc_dirname' whenever 
'dirname' is stuffed/moved somewhere. In code that I am responsible for, 
especially complex code as glob(), I also set pointers to NULL after free() or 
when assigning/moving to another variable.

> For the other ones, you may be right, but I wonder why scan.coverity.com did
> not display them? Probably we need to upload a new build to
> scan.coverity.com. Which brings me to a status question: Did you find
> volunteers for the triage of the "defects" on scan.coverity.com? I stopped
> looking into it when you said you would find some people.

Sadly, I was too optimistic. I didn't forget about, and working on those is 
still on my list. But currently I am overwhelmed with stuff that has higher 
priority.

> Ultimately, for functions as complex as glob(), I now trust coverity.com
> more than any person's review (including mine myself). Therefore I would
> like to pursue the approach of using scan.coverity.com.

That's always a good idea. You could set up a cron job to automatically upload 
to Coverity, e.g. once a week or once a day. You can find instructions one the 
'upload for file analysis' page.

I am currently into fuzzing which is not only good for finding *real* issues 
(leaks, slowness, illegal memory access, integer overflow, ...). Fuzzing also 
provides you with test data that (nearly) fully covers all code paths. Those 
test inputs can (and should) be integrated into a project's test suite.

Are you going to GHM ? Wish, I could meet you there.

Regards, Tim

signature.asc
Description: This is a digitally signed message part.