This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PING^3][PATCH v1.1][BZ #15698] Fix memory overrun in getifaddrs_internal.
- From: "H.J. Lu" <hjl dot tools at gmail dot com>
- To: Ondřej Bílka <neleai at seznam dot cz>
- Cc: GNU C Library <libc-alpha at sourceware dot org>
- Date: Sat, 11 Jan 2014 06:22:45 -0800
- Subject: Re: [PING^3][PATCH v1.1][BZ #15698] Fix memory overrun in getifaddrs_internal.
- Authentication-results: sourceware.org; auth=none
- References: <20131008165738 dot GA14975 at domone dot podge> <CAMe9rOoDWk48LmZys2LUAyKxf3tVmhJEi+4BH+dJVTwddHnRmw at mail dot gmail dot com> <20131008173847 dot GA15464 at domone dot podge> <20131023124439 dot GD5434 at domone dot podge> <20131105151035 dot GA30035 at domone dot podge> <20140102203347 dot GG25179 at domone> <20140111123955 dot GA27905 at domone dot podge>
On Sat, Jan 11, 2014 at 4:39 AM, OndÅej BÃlka <neleai@seznam.cz> wrote:
> ping
> On Thu, Jan 02, 2014 at 09:33:47PM +0100, OndÅej BÃlka wrote:
>> ping
>> On Tue, Nov 05, 2013 at 04:10:35PM +0100, OndÅej BÃlka wrote:
>> > ping
>> > > ping
>> > > On Tue, Oct 08, 2013 at 07:38:47PM +0200, OndÅej BÃlka 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 looks good to me. I think you should check it in.
Thanks.
--
H.J.