Hi David, On Fri, Aug 18, 2017 at 09:19:16AM +0000, David Laight wrote: > From: Phil Sutter > > Sent: 17 August 2017 18:09 > > To: Stephen Hemminger > > Cc: netdev@vger.kernel.org > > Subject: [iproute PATCH v2 1/7] ipntable: Make sure filter.name is > > NULL-terminated > > > > Signed-off-by: Phil Sutter <p...@nwl.cc> > > --- > > ip/ipntable.c | 3 ++- > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > diff --git a/ip/ipntable.c b/ip/ipntable.c > > index 879626ee4f491..7be1f04d33d90 100644 > > --- a/ip/ipntable.c > > +++ b/ip/ipntable.c > > @@ -633,7 +633,8 @@ static int ipntable_show(int argc, char **argv) > > } else if (strcmp(*argv, "name") == 0) { > > NEXT_ARG(); > > > > - strncpy(filter.name, *argv, sizeof(filter.name)); > > + strncpy(filter.name, *argv, sizeof(filter.name) - 1); > > + filter.name[sizeof(filter.name) - 1] = '\0'; > > Why not check for overflow instead? > if (filter.name[sizeof(filter.name) - 1]) > usage("filer name too long");
sizeof(filter.name) is 1024, which is maybe a bit over the top for something a user would input. So I found a better way avoiding all this at once: I made filter.name a const char *, then just assigned *argv to it. This should be safe since rtnl_dump_filter() and therefore print_ntable() callback is called from inside ipntable_show() so *argv is not accessed outside of it's scope. What do you think? Thanks, Phil