netdev - Re: [iproute PATCH 21/51] lib/libnetlink: Don't pass NULL parameter to memcpy()

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 18 Aug 2017 15:13:39 -0400 (EDT)
From:   Lance Richardson <lrichard@...hat.com>
To:     Stephen Hemminger <stephen@...workplumber.org>
Cc:     netdev@...r.kernel.org, Phil Sutter <phil@....cc>
Subject: Re: [iproute PATCH 21/51] lib/libnetlink: Don't pass NULL parameter
 to memcpy()

> From: "Phil Sutter" <phil@....cc>
> To: "Stephen Hemminger" <stephen@...workplumber.org>
> Cc: netdev@...r.kernel.org
> Sent: Tuesday, August 15, 2017 12:42:55 PM
> Subject: Re: [iproute PATCH 21/51] lib/libnetlink: Don't pass NULL parameter to memcpy()
> 
> On Tue, Aug 15, 2017 at 08:15:55AM -0700, Stephen Hemminger wrote:
> > On Sat, 12 Aug 2017 14:04:40 +0200
> > Phil Sutter <phil@....cc> wrote:
> > 
> > > Both addattr_l() and rta_addattr_l() may be called with NULL data
> > > pointer and 0 alen parameters. Avoid calling memcpy() in that case.
> > > 
> > > Signed-off-by: Phil Sutter <phil@....cc>
> > 
> > What are you fixing. memcpy(dest, NULL, 0) should be harmless NOP
> 
> Yes, if that turns into a NOP this patch is not needed.
> 
> Thanks, Phil
> 

It is a NOP in this case, but it is also "undefined behavior" and can lead
to the compiler assuming that dest != NULL, which would be problematic
if dest were dereferenced later in the code (it isn't in this case, but
might be in general).

A small example with current gcc:

foo.c:
    #include <stdio.h>

    extern void foo(char *, size_t);

    int main(int argc, char **argv)
    {
            char x[128];

            foo(x, sizeof x);
            foo(NULL, 0);

            return 0;
    }

bar.c:
    #include <stdio.h>
    #include <string.h>

    void foo(char *ptr, size_t len)
    {
            memset(ptr, 0, len);

            if (ptr)
                    printf("ptr is non-null: %p\n", ptr);
    }

Compile the code:

    $ gcc -o foobar -O2 foo.c bar.c

Execute it (note second line of output, which might be surprising):

    $ ./foobar
    ptr is non-null: 0x7ffdc47daef0
    ptr is non-null: (nil)


Regards,

    Lance Richardson