From: Lance Richardson <lrichard@redhat•com>
To: Stephen Hemminger <stephen@networkplumber•org>
Cc: netdev@vger•kernel.org, Phil Sutter <phil@nwl•cc>
Subject: Re: [iproute PATCH 21/51] lib/libnetlink: Don't pass NULL parameter to memcpy()
Date: Fri, 18 Aug 2017 15:13:39 -0400 (EDT) [thread overview]
Message-ID: <1953007845.2202528.1503083619344.JavaMail.zimbra@redhat.com> (raw)
In-Reply-To: <20170815164255.GA10864@orbyte.nwl.cc>
> From: "Phil Sutter" <phil@nwl•cc>
> To: "Stephen Hemminger" <stephen@networkplumber•org>
> Cc: netdev@vger•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@nwl•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@nwl•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
next prev parent reply other threads:[~2017-08-18 19:13 UTC|newest]
Thread overview: 76+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-08-12 12:04 [iproute PATCH 00/51] Fix potential issues detected by Coverity tool Phil Sutter
2017-08-12 12:04 ` [iproute PATCH 01/51] devlink: Check return code of strslashrsplit() Phil Sutter
2017-08-15 15:09 ` Stephen Hemminger
2017-08-12 12:04 ` [iproute PATCH 02/51] devlink: No need for this self-assignment Phil Sutter
2017-08-12 12:04 ` [iproute PATCH 03/51] ipaddress: Make buffer for filter.flushb static Phil Sutter
2017-08-15 15:13 ` Stephen Hemminger
2017-08-15 16:11 ` Phil Sutter
2017-08-12 12:04 ` [iproute PATCH 04/51] ipaddress: Avoid accessing uninitialized variable lcl Phil Sutter
2017-08-12 12:04 ` [iproute PATCH 05/51] iplink_can: Prevent overstepping array bounds Phil Sutter
2017-08-15 15:10 ` Stephen Hemminger
2017-08-15 16:31 ` Phil Sutter
2017-08-12 12:04 ` [iproute PATCH 06/51] iplink_vrf: Complain if main table is not found Phil Sutter
2017-08-13 15:58 ` David Ahern
2017-08-12 12:04 ` [iproute PATCH 07/51] ipmaddr: Avoid accessing uninitialized data Phil Sutter
2017-08-12 12:04 ` [iproute PATCH 08/51] ipntable: No need to check and assign to parms_rta Phil Sutter
2017-08-12 12:04 ` [iproute PATCH 09/51] ipntable: Make sure filter.name is NULL-terminated Phil Sutter
2017-08-12 12:04 ` [iproute PATCH 10/51] iproute: Fix for missing 'Oifs:' display Phil Sutter
2017-08-12 12:04 ` [iproute PATCH 11/51] iproute: Check mark value input Phil Sutter
2017-08-12 12:04 ` [iproute PATCH 12/51] iproute_lwtunnel: csum_mode value checking was ineffective Phil Sutter
2017-08-12 12:04 ` [iproute PATCH 13/51] iproute_lwtunnel: Argument to strerror must be positive Phil Sutter
2017-08-12 12:04 ` [iproute PATCH 14/51] ipvrf: Don't try to close an invalid fd Phil Sutter
2017-08-13 15:59 ` David Ahern
2017-08-15 15:14 ` Stephen Hemminger
2017-08-12 12:04 ` [iproute PATCH 15/51] ipvrf: Fix error path of vrf_switch() Phil Sutter
2017-08-13 16:00 ` David Ahern
2017-08-12 12:04 ` [iproute PATCH 16/51] xfrm_state: Make sure alg_name is NULL-terminated Phil Sutter
2017-08-12 12:04 ` [iproute PATCH 17/51] lib/bpf: Don't leak fp in bpf_find_mntpt() Phil Sutter
2017-08-14 8:46 ` Daniel Borkmann
2017-08-12 12:04 ` [iproute PATCH 18/51] lib/fs: Fix format string in find_fs_mount() Phil Sutter
2017-08-12 12:04 ` [iproute PATCH 19/51] lib/fs: Fix and simplify make_path() Phil Sutter
2017-08-12 12:04 ` [iproute PATCH 20/51] lib/inet_proto: Make sure destination buffers are NULL-terminated Phil Sutter
2017-08-12 12:04 ` [iproute PATCH 21/51] lib/libnetlink: Don't pass NULL parameter to memcpy() Phil Sutter
2017-08-15 15:15 ` Stephen Hemminger
2017-08-15 16:42 ` Phil Sutter
2017-08-18 19:13 ` Lance Richardson [this message]
2017-08-12 12:04 ` [iproute PATCH 22/51] lib/rt_names: Drop dead code in rtnl_rttable_n2a() Phil Sutter
2017-08-12 12:04 ` [iproute PATCH 23/51] ifstat: Fix memleak in error case Phil Sutter
2017-08-12 12:04 ` [iproute PATCH 24/51] ifstat, nstat: Check fdopen() return value Phil Sutter
2017-08-12 12:04 ` [iproute PATCH 25/51] ifstat: Fix memleak in dump_kern_db() for json output Phil Sutter
2017-08-12 12:04 ` [iproute PATCH 26/51] lnstat_util: Simplify alloc_and_open() a bit Phil Sutter
2017-08-12 12:04 ` [iproute PATCH 27/51] nstat: Fix for potential NULL pointer dereference Phil Sutter
2017-08-12 12:04 ` [iproute PATCH 28/51] nstat: Avoid passing negative fd to fdopen() Phil Sutter
2017-08-12 12:04 ` [iproute PATCH 29/51] ss: Use C99 initializer in netlink_show_one() Phil Sutter
2017-08-12 12:04 ` [iproute PATCH 30/51] ss: Skip useless check in parse_hostcond() Phil Sutter
2017-08-12 12:04 ` [iproute PATCH 31/51] ss: Drop useless assignment Phil Sutter
2017-08-12 12:04 ` [iproute PATCH 32/51] ss: Make sure index variable is >= 0 Phil Sutter
2017-08-12 12:04 ` [iproute PATCH 33/51] ss: Don't leak fd in tcp_show_netlink_file() Phil Sutter
2017-08-12 12:04 ` [iproute PATCH 34/51] ss: Make sure scanned index value to unix_state_map is sane Phil Sutter
2017-08-12 12:04 ` [iproute PATCH 35/51] ss: Fix potential memleak in unix_stats_print() Phil Sutter
2017-08-12 12:04 ` [iproute PATCH 36/51] netem/maketable: Check return value of fstat() Phil Sutter
2017-08-12 12:04 ` [iproute PATCH 37/51] netem/maketable: Check return value of fscanf() Phil Sutter
2017-08-12 12:04 ` [iproute PATCH 38/51] tc/em_ipset: Don't leak sockfd on error path Phil Sutter
2017-08-12 12:04 ` [iproute PATCH 39/51] tc/m_gact: Drop dead code Phil Sutter
2017-08-12 12:04 ` [iproute PATCH 40/51] tc/m_xt: Fix for potential string buffer overflows Phil Sutter
2017-08-12 12:05 ` [iproute PATCH 41/51] tc/q_multiq: Don't pass garbage in TCA_OPTIONS Phil Sutter
2017-08-12 12:05 ` [iproute PATCH 42/51] tc/q_netem: Don't dereference possibly NULL pointer Phil Sutter
2017-08-12 12:05 ` [iproute PATCH 43/51] tc/tc_filter: Make sure filter name is not empty Phil Sutter
2017-08-12 12:05 ` [iproute PATCH 44/51] tipc/bearer: Fix resource leak in error path Phil Sutter
2017-08-12 12:05 ` [iproute PATCH 45/51] tipc/bearer: Prevent NULL pointer dereference Phil Sutter
2017-08-12 12:05 ` [iproute PATCH 46/51] tipc/node: Fix socket fd check in cmd_node_get_addr() Phil Sutter
2017-08-12 12:05 ` [iproute PATCH 47/51] examples: Some shell fixes to cbq.init Phil Sutter
2017-08-12 12:05 ` [iproute PATCH 48/51] ifcfg: Quote left-hand side of [ ] expression Phil Sutter
2017-08-12 12:05 ` [iproute PATCH 49/51] lib/ll_map: Make sure im->name is NULL-terminated Phil Sutter
2017-08-12 12:05 ` [iproute PATCH 50/51] Check user supplied interface name lengths Phil Sutter
2017-08-15 16:09 ` Stephen Hemminger
2017-08-15 16:51 ` Phil Sutter
2017-09-01 16:56 ` Phil Sutter
2017-08-12 12:05 ` [iproute PATCH 51/51] lib/bpf: Check return value of write() Phil Sutter
2017-08-14 9:17 ` Daniel Borkmann
2017-08-14 17:25 ` Phil Sutter
2017-08-14 20:35 ` Daniel Borkmann
2017-08-15 12:31 ` David Laight
2017-08-15 13:00 ` Daniel Borkmann
2017-08-15 15:07 ` [iproute PATCH 00/51] Fix potential issues detected by Coverity tool Stephen Hemminger
2017-08-15 16:04 ` Phil Sutter
2017-08-15 16:14 ` Stephen Hemminger
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=1953007845.2202528.1503083619344.JavaMail.zimbra@redhat.com \
--to=lrichard@redhat$(echo .)com \
--cc=netdev@vger$(echo .)kernel.org \
--cc=phil@nwl$(echo .)cc \
--cc=stephen@networkplumber$(echo .)org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox