linux-kernel - Re: [PATCH 06/11] sysctl: Add size to 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: <20230621113625.o54p6qfvr5duskfb@localhost>
Date:   Wed, 21 Jun 2023 13:36:25 +0200
From:   Joel Granados <j.granados@...sung.com>
To:     Dan Carpenter <dan.carpenter@...aro.org>
CC:     <mcgrof@...nel.org>, Jason Gunthorpe <jgg@...pe.ca>,
        Leon Romanovsky <leon@...nel.org>,
        David Ahern <dsahern@...nel.org>,
        "David S. Miller" <davem@...emloft.net>,
        Eric Dumazet <edumazet@...gle.com>,
        Jakub Kicinski <kuba@...nel.org>,
        Paolo Abeni <pabeni@...hat.com>,
        Joerg Reuter <jreuter@...na.de>,
        Ralf Baechle <ralf@...ux-mips.org>,
        Pablo Neira Ayuso <pablo@...filter.org>,
        Jozsef Kadlecsik <kadlec@...filter.org>,
        Florian Westphal <fw@...len.de>,
        Roopa Prabhu <roopa@...dia.com>,
        Nikolay Aleksandrov <razor@...ckwall.org>,
        Alexander Aring <alex.aring@...il.com>,
        Stefan Schmidt <stefan@...enfreihafen.org>,
        Miquel Raynal <miquel.raynal@...tlin.com>,
        Steffen Klassert <steffen.klassert@...unet.com>,
        Herbert Xu <herbert@...dor.apana.org.au>,
        Matthieu Baerts <matthieu.baerts@...sares.net>,
        Mat Martineau <martineau@...nel.org>,
        Simon Horman <horms@...ge.net.au>,
        Julian Anastasov <ja@....bg>,
        Remi Denis-Courmont <courmisch@...il.com>,
        Santosh Shilimkar <santosh.shilimkar@...cle.com>,
        David Howells <dhowells@...hat.com>,
        Marc Dionne <marc.dionne@...istor.com>,
        Neil Horman <nhorman@...driver.com>,
        Marcelo Ricardo Leitner <marcelo.leitner@...il.com>,
        Xin Long <lucien.xin@...il.com>,
        Karsten Graul <kgraul@...ux.ibm.com>,
        Wenjia Zhang <wenjia@...ux.ibm.com>,
        Jan Karcher <jaka@...ux.ibm.com>,
        Jon Maloy <jmaloy@...hat.com>,
        Ying Xue <ying.xue@...driver.com>,
        Martin Schiller <ms@....tdt.de>, <linux-rdma@...r.kernel.org>,
        <linux-kernel@...r.kernel.org>, <netdev@...r.kernel.org>,
        <linux-hams@...r.kernel.org>, <netfilter-devel@...r.kernel.org>,
        <coreteam@...filter.org>, <bridge@...ts.linux-foundation.org>,
        <dccp@...r.kernel.org>, <linux-wpan@...r.kernel.org>,
        <mptcp@...ts.linux.dev>, <lvs-devel@...r.kernel.org>,
        <rds-devel@....oracle.com>, <linux-afs@...ts.infradead.org>,
        <linux-sctp@...r.kernel.org>, <linux-s390@...r.kernel.org>,
        <tipc-discussion@...ts.sourceforge.net>,
        <linux-x25@...r.kernel.org>
Subject: Re: [PATCH 06/11] sysctl: Add size to register_net_sysctl function

On Wed, Jun 21, 2023 at 12:47:30PM +0300, Dan Carpenter wrote:
> The patchset doesn't include the actual interesting changes, just a
> bunch of mechanical prep work.
Yep, The thread got mangled on the way out. But hopefully the rest of
the patch made its way to the lists and maintainers.

> 
> On Wed, Jun 21, 2023 at 11:09:55AM +0200, Joel Granados wrote:
> > diff --git a/net/ieee802154/6lowpan/reassembly.c b/net/ieee802154/6lowpan/reassembly.c
> > index a91283d1e5bf..7b717434368c 100644
> > --- a/net/ieee802154/6lowpan/reassembly.c
> > +++ b/net/ieee802154/6lowpan/reassembly.c
> > @@ -379,7 +379,8 @@ static int __net_init lowpan_frags_ns_sysctl_register(struct net *net)
> >  	table[1].extra2	= &ieee802154_lowpan->fqdir->high_thresh;
> >  	table[2].data	= &ieee802154_lowpan->fqdir->timeout;
> >  
> > -	hdr = register_net_sysctl(net, "net/ieee802154/6lowpan", table);
> > +	hdr = register_net_sysctl(net, "net/ieee802154/6lowpan", table,
> > +				  ARRAY_SIZE(lowpan_frags_ns_ctl_table));
> 
> For example, in lowpan_frags_ns_sysctl_register() the sentinel is
> sometimes element zero if the user doesn't have enough permissions.  I
> would want to ensure that was handled correctly, but that's going to be
> done later in a completely different patchset.  I'm definitely not going
> to remember to check.
Very good catch! I have fixed this as well as ensure_safe_net_sysctl
that was missing a table_size arg.

> 
> > diff --git a/net/mpls/af_mpls.c b/net/mpls/af_mpls.c
> > index dc5165d3eec4..6f96aae76537 100644
> > --- a/net/mpls/af_mpls.c
> > +++ b/net/mpls/af_mpls.c
> > @@ -1395,6 +1395,40 @@ static const struct ctl_table mpls_dev_table[] = {
> >  	{ }
> >  };
> >  
> > +static int mpls_platform_labels(struct ctl_table *table, int write,
> > +				void *buffer, size_t *lenp, loff_t *ppos);
> > +#define MPLS_NS_SYSCTL_OFFSET(field)		\
> > +	(&((struct net *)0)->field)
> > +
> > +static const struct ctl_table mpls_table[] = {
> > +	{
> > +		.procname	= "platform_labels",
> > +		.data		= NULL,
> > +		.maxlen		= sizeof(int),
> > +		.mode		= 0644,
> > +		.proc_handler	= mpls_platform_labels,
> > +	},
> > +	{
> > +		.procname	= "ip_ttl_propagate",
> > +		.data		= MPLS_NS_SYSCTL_OFFSET(mpls.ip_ttl_propagate),
> > +		.maxlen		= sizeof(int),
> > +		.mode		= 0644,
> > +		.proc_handler	= proc_dointvec_minmax,
> > +		.extra1		= SYSCTL_ZERO,
> > +		.extra2		= SYSCTL_ONE,
> > +	},
> > +	{
> > +		.procname	= "default_ttl",
> > +		.data		= MPLS_NS_SYSCTL_OFFSET(mpls.default_ttl),
> > +		.maxlen		= sizeof(int),
> > +		.mode		= 0644,
> > +		.proc_handler	= proc_dointvec_minmax,
> > +		.extra1		= SYSCTL_ONE,
> > +		.extra2		= &ttl_max,
> > +	},
> > +	{ }
> > +};
> > +
> >  static int mpls_dev_sysctl_register(struct net_device *dev,
> >  				    struct mpls_dev *mdev)
> >  {
> > @@ -1410,7 +1444,7 @@ static int mpls_dev_sysctl_register(struct net_device *dev,
> >  	/* Table data contains only offsets relative to the base of
> >  	 * the mdev at this point, so make them absolute.
> >  	 */
> > -	for (i = 0; i < ARRAY_SIZE(mpls_dev_table); i++) {
> > +	for (i = 0; i < ARRAY_SIZE(mpls_dev_table) - 1; i++) {
> 
> Adding the " - 1" is just a gratuitous change.  It's not required.
> It makes that patch more confusing to review.  And you're just going
> to have to change it back to how it was if you remove the sentinel.
Removed this for convenience. Thx.


> 
> >  		table[i].data = (char *)mdev + (uintptr_t)table[i].data;
> >  		table[i].extra1 = mdev;
> >  		table[i].extra2 = net;
> > @@ -1418,7 +1452,8 @@ static int mpls_dev_sysctl_register(struct net_device *dev,
> >  
> >  	snprintf(path, sizeof(path), "net/mpls/conf/%s", dev->name);
> >  
> > -	mdev->sysctl = register_net_sysctl(net, path, table);
> > +	mdev->sysctl = register_net_sysctl(net, path, table,
> > +					   ARRAY_SIZE(mpls_dev_table));
> >  	if (!mdev->sysctl)
> >  		goto free;
> >  
> > @@ -1432,6 +1467,7 @@ static int mpls_dev_sysctl_register(struct net_device *dev,
> >  	return -ENOBUFS;
> >  }
> >  
> > +
Oops. thx. fixed

> 
> Double blank line.
> 
> >  static void mpls_dev_sysctl_unregister(struct net_device *dev,
> >  				       struct mpls_dev *mdev)
> >  {
> 
> regards,
> dan carpenter

-- 

Joel Granados

Download attachment "signature.asc" of type "application/pgp-signature" (660 bytes)