Re: PATH: fix buffer overrun at sysdeps/unix/sysv/linux/ifaddrs.c

["Carlos O'Donell" <carlos at redhat dot com>]

On 02/07/2013 10:42 AM, Dmitrieva Liubov wrote:
> Please, consider my fix.

This is not a sufficient description of the problem.
Please help reviewers understand the issue by providing
a detailed explanation of the issue, particularly if it's
a buffer overrun.

Please see:
http://sourceware.org/glibc/wiki/Contribution%20checklist#Detailed_Explanation_of_the_Patch
 
> diff --git a/sysdeps/unix/sysv/linux/ifaddrs.c
> b/sysdeps/unix/sysv/linux/ifaddrs.c
> index 5c1e380..56fbedd 100644
> --- a/sysdeps/unix/sysv/linux/ifaddrs.c
> +++ b/sysdeps/unix/sysv/linux/ifaddrs.c
> @@ -780,7 +780,7 @@ getifaddrs_internal (struct ifaddrs **ifap)
>                       else
>                         preflen = ifam->ifa_prefixlen;
> 
> -                     for (i = 0; i < (preflen / 8); i++)
> +                     for (i = 0; i < (preflen / 8) - 1; i++)

This goes from "0" to "(preflen / 8) - 1" because of the "<",
why does it need to go to "(preflen / 8) - 2"?

I need your help Dmitrieva :-)

>                         *cp++ = 0xff;
>                       c = 0xff;
>                       c <<= (8 - (preflen % 8));
> 
> 
> -----
> 
> ChangeLog:
> 
> 2013-02-07  Liubov Dmitrieva  <liubov.dmitrieva@gmail.com>
> 
>         * sysdeps/unix/sysv/linux/ifaddrs.c: Fix
>         buffer overrun in the loop.

Two issues with your ChangeLog:

(1) This ChangeLog is not formatted correctly (missing function name).

It should read:

	* sysdeps/unix/sysv/linux/ifaddrs.c (getifaddrs_internal): <Blah>.

(2) The ChangeLog should indicate what was changed in the file.

For example an appropriate change might read:

	"Stop before (preflen / 8) - 1"

That describes the change.

Cheers,
Carlos.