From: Andrey Ryabinin <a.ryabinin@samsung.com>
To: Sasha Levin <sasha.levin@oracle.com>, David Miller <davem@davemloft.net>
Cc: pablo@netfilter.org, mschmidt@redhat.com,
akpm@linux-foundation.org, linux-kernel@vger.kernel.org,
netdev@vger.kernel.org
Subject: Re: [PATCH] netlink: don't copy over empty attribute data
Date: Mon, 27 Oct 2014 19:46:33 +0300 [thread overview]
Message-ID: <544E76E9.3090909@samsung.com> (raw)
In-Reply-To: <544E59DC.3060906@oracle.com>
On 10/27/2014 05:42 PM, Sasha Levin wrote:
> On 10/26/2014 10:03 PM, David Miller wrote:
>> From: Sasha Levin <sasha.levin@oracle.com>
>> Date: Sun, 26 Oct 2014 19:32:42 -0400
>>
>>> How so? GCC states clearly that you should *never* pass a NULL
>>> pointer there:
>>>
>>> "The pointers passed to memmove (and similar functions in <string.h>) must
>>> be non-null even when nbytes==0" (https://gcc.gnu.org/gcc-4.9/porting_to.html).
>>>
>>> Even if it doesn't dereference it, it can break somehow in a subtle way. Leaving
>>> the kernel code assuming that gcc (or any other compiler) would always behave
>>> the same in a situation that shouldn't occur.
>>
>> Show me a legal way in which one could legally dereference the pointer
>> when length is zero, and I'll entertain this patch.
>
> The moment you've triggered an undefined behaviour you have GCC license to
> dereference anything it wants. GCC would be well within it's rights
> dereferencing a NULL "from".
>
> They even state it clearly in that GCC 4.9 porting guide I've linked above:
>
> """
> Calling copy(p, NULL, 0) can therefore deference a null pointer and crash.
>
> The example above needs to be fixed to avoid the invalid memmove call, for example:
>
>
> if (nbytes != 0)
> memmove (dest, src, nbytes);
> """
>
In example from link null ptr deref could happen because GCC will optimize away null pointer check after
memmove():
int copy (int* dest, int* src, size_t nbytes) {
memmove (dest, src, nbytes);
if (src != NULL) <---- GCC will eliminate this check because src can't be null.
return *src; <-- NULL ptr deref
return 0;
}
Even though GCC and C standard treats such code ( memmove(dest, NULL, 0); ) as invalid, it probably will not crash in linux kernel case,
because that kind of optimization disabled via -fno-delete-null-pointer-checks option.
>
> Thanks,
> Sasha
>
next prev parent reply other threads:[~2014-10-27 16:46 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-10-21 20:51 [PATCH] netlink: don't copy over empty attribute data Sasha Levin
2014-10-22 1:39 ` David Miller
2014-10-22 2:19 ` Sasha Levin
2014-10-22 6:15 ` David Miller
2014-10-26 23:32 ` Sasha Levin
2014-10-27 2:03 ` David Miller
2014-10-27 14:42 ` Sasha Levin
2014-10-27 16:46 ` Andrey Ryabinin [this message]
2014-10-22 8:55 ` David Laight
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=544E76E9.3090909@samsung.com \
--to=a.ryabinin@samsung.com \
--cc=akpm@linux-foundation.org \
--cc=davem@davemloft.net \
--cc=linux-kernel@vger.kernel.org \
--cc=mschmidt@redhat.com \
--cc=netdev@vger.kernel.org \
--cc=pablo@netfilter.org \
--cc=sasha.levin@oracle.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.