netdev - Re: [PATCH net-next 12/19] net neighbour: Convert to use register_net

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]

Message-ID: <1335062215.3209.330.camel@deadeye>
Date:	Sun, 22 Apr 2012 03:36:55 +0100
From:	Ben Hutchings <bhutchings@...arflare.com>
To:	"Eric W. Biederman" <ebiederm@...ssion.com>
CC:	Pavel Emelyanov <xemul@...allels.com>,
	David Miller <davem@...emloft.net>,
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
	"Serge E. Hallyn" <serge@...lyn.com>,
	Gao feng <gaofeng@...fujitsu.com>,
	"pablo@...filter.org" <pablo@...filter.org>,
	Stephen Hemminger <shemminger@...tta.com>
Subject: Re: [PATCH net-next 12/19] net neighbour:  Convert to use
 register_net_sysctl

On Fri, 2012-04-20 at 00:25 -0700, Eric W. Biederman wrote:
> Pavel Emelyanov <xemul@...allels.com> writes:
> 
> >> @@ -2925,19 +2924,7 @@ int neigh_sysctl_register(struct net_device *dev, struct neigh_parms *p,
> >>  {
> >>  	struct neigh_sysctl_table *t;
> >>  	const char *dev_name_source = NULL;
> >> -
> >> -#define NEIGH_CTL_PATH_ROOT	0
> >> -#define NEIGH_CTL_PATH_PROTO	1
> >> -#define NEIGH_CTL_PATH_NEIGH	2
> >> -#define NEIGH_CTL_PATH_DEV	3
> >> -
> >> -	struct ctl_path neigh_path[] = {
> >> -		{ .procname = "net",	 },
> >> -		{ .procname = "proto",	 },
> >> -		{ .procname = "neigh",	 },
> >> -		{ .procname = "default", },
> >> -		{ },
> >> -	};
> >> +	char neigh_path[ sizeof("net//neigh/") + IFNAMSIZ + IFNAMSIZ ];
> >
> > Why two IFNAMSIZ-es? One is for the dev->name, but the other one is not.
> > Is it just for not having any other better constant at hands?
> 
> Yep.  We don't seem to have any proto name size constants, and all
> of decnet ipv4 and ipv6 are all shorter than the 16 bytes of IFNAMSIZ.

I don't think it makes any sense to put in IFNAMSIZ as a size for a
string that isn't a device name.

> Even if I am wrong the snprintf below truncates it's output to the
> buffer size and null terminates it so in the worst case we won't cause
> a buffer overflow, we will just get a truncated path name to pass
> to sysctl.
> 
> Shrug I stopped at good enough but I am happy for a better number.

Truncation by snprintf() is definitely better than overflow, but we
should also check and WARN so that if someone breaks this it's hard to
miss.

Ben.

-- 
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html