[vdr] 1.3.22: memory leaks

Stefan Huelswitt s.huelswitt at gmx.de
Tue Mar 29 19:23:50 CEST 2005


On 29 Mar 2005 Daniel THOMPSON <daniel.thompson at st.com> wrote:

Hi,

> There are routes through tComponent::FromString() that explicitly set 
> description to NULL without checking its value first (when n != 4). This 
> appears to me to be the leak.
> 
> Running the following code it is obvious that glibc malloc'ed desc even
> when desc is not converted.

Wow, this is cool. How did you get the idea to search in that
direction?

Anyways, I think the man page isn't very clear on this fact...

> Thus the simpler fix for this leak, and one that is not prone to buffer 
> overflow, is:
> 
> --- vdr-1.3.22/epg.c.orig       2005-03-29 09:03:23.484024000 +0100
> +++ vdr-1.3.22/epg.c    2005-03-29 09:04:16.359024000 +0100
> @@ -29,9 +29,7 @@
>   {
>     unsigned int Stream, Type;
>     int n = sscanf(s, "%X %02X %3c %a[^\n]", &Stream, &Type, language, 
> &description);
> -  if (n != 4)
> -     description = NULL;
> -  else if (isempty(description)) {
> +  if (n != 4 || isempty(description)) {
>        free(description);
>        description = NULL;
>        }
> 
> While that are more robust ways to handle the case then desc is NULL 
> (which for my glibc it never is) the above code is safe since both 
> isempty() and glibc's free() can safely handle NULL pointers.

What about this?

if (description!=NULL && (n != 4 || isempty(description)))

Regards.

-- 
Stefan Huelswitt
s.huelswitt at gmx.de  | http://www.muempf.de/



More information about the vdr mailing list