Re: [PATCH v1.1][BZ #15698] Fix memory overrun in getifaddrs_internal.

["H.J. Lu" <hjl dot tools at gmail dot com>]

On Tue, Oct 8, 2013 at 10:38 AM, OndÅej BÃlka <neleai@seznam.cz> wrote:
> On Tue, Oct 08, 2013 at 10:13:28AM -0700, H.J. Lu wrote:
>> On Tue, Oct 8, 2013 at 9:57 AM, OndÅej BÃlka <neleai@seznam.cz> wrote:
>> > Hi, a code at https://sourceware.org/bugzilla/show_bug.cgi?id=15698
>> > contains a simple off-by-one error when preflen is divisible by 8.
>> >
>> > Following code should fix this, as preflen is unsigned I added check for
>> > zero len to be sure we do not cause underflow.
>> >
>> > OK to commit?
>> >
>> >         * sysdeps/unix/sysv/linux/ifaddrs.c (getifaddrs_internal): Fix
>> >         memory overrun.
>>
>> Missing BZ #.
>>
>> >
>> > -                     for (i = 0; i < (preflen / 8); i++)
>> > +                     for (i = 0; preflen && i < ((preflen - 1) / 8); i++)
>> >                         *cp++ = 0xff;
>> >                       c = 0xff;
>> >                       c <<= (8 - (preflen % 8));
>>
>>
>> I don't think it is correct for netmask.  When
>> preflen == max_prefixlen, netmask should be all 1's.
>> Something like:
>
> I assumed that this shift sets correct value. It needed changing that it
> evaluates to 0 instead 8 and lefts mask intact.
>
>
>         [BZ #15698]
>         * sysdeps/unix/sysv/linux/ifaddrs.c (getifaddrs_internal): Fix
>         memory overrun.
>
> diff --git a/sysdeps/unix/sysv/linux/ifaddrs.c b/sysdeps/unix/sysv/linux/ifaddrs.c
> index 89fda15..e62bee0 100644
> --- a/sysdeps/unix/sysv/linux/ifaddrs.c
> +++ b/sysdeps/unix/sysv/linux/ifaddrs.c
> @@ -780,10 +780,10 @@ getifaddrs_internal (struct ifaddrs **ifap)
>                       else
>                         preflen = ifam->ifa_prefixlen;
>
> -                     for (i = 0; i < (preflen / 8); i++)
> +                     for (i = 0; i < ((preflen - 1) / 8); i++)
>                         *cp++ = 0xff;
>                       c = 0xff;
> -                     c <<= (8 - (preflen % 8));
> +                     c <<= ((128 - preflen) % 8);
>                       *cp = c;
>                     }
>                 }

It should work.

-- 
H.J.