Re: [RFC][PATCH v3 3/3] Rename nla_strlcpy to nla_strscpy.

From: Francis Laniel <laniel_francis@privacyrequired.com>
To: David Laight <David.Laight@aculab.com>
Cc: 'Jakub Kicinski' <kuba@kernel.org>,
	Kees Cook <keescook@chromium.org>,
	"linux-hardening@vger.kernel.org"
	<linux-hardening@vger.kernel.org>,
	"netdev@vger.kernel.org" <netdev@vger.kernel.org>,
	"davem@davemloft.net" <davem@davemloft.net>
Subject: Re: [RFC][PATCH v3 3/3] Rename nla_strlcpy to nla_strscpy.
Date: Fri, 23 Oct 2020 17:15:08 +0200	[thread overview]
Message-ID: <1915509.OMjZjUUbeY@machine> (raw)
In-Reply-To: <b55d502089c44b3589973fa4e0d90617@AcuMS.aculab.com>

Le vendredi 23 octobre 2020, 10:07:44 CEST David Laight a écrit :
> From: Jakub Kicinski
> 
> > Sent: 23 October 2020 00:06
> > 
> > On Thu, 22 Oct 2020 13:04:32 -0700 Kees Cook wrote:
> > > > > > From: Francis Laniel <laniel_francis@privacyrequired.com>
> > > > > > 
> > > > > > Calls to nla_strlcpy are now replaced by calls to nla_strscpy
> > > > > > which is the
> > > > > > new name of this function.
> > > > > > 
> > > > > > Signed-off-by: Francis Laniel <laniel_francis@privacyrequired.com>
> > > > > 
> > > > > The Subject could also be: "treewide: Rename nla_strlcpy to
> > > > > nla_strscpy"
> > > > > 
> > > > > But otherwise, yup, easy mechanical change.
> > > > 
> > > > Should I submit a v4 for this change?
> > > 
> > > I'll say yes. :) Drop the RFC, bump to v4, and send it to netdev (along
> > > with all the other CCs you have here already), and add the Reviewed-bys
> > > from v3.
> > 
> > Maybe wait until next week, IIRC this doesn't fix any bugs, so it's
> > -next material. We don't apply anything to net-next during the merge
> > window.
> 
> Is this just a rename, or have you changed the result value?
> In the latter case the subject is really right.

I changed the result value so it mimics the return value of strscpy.

> FWIW I suspect  the 'return -ERR on overflow' is going to bite us.
> Code that does p += strsxxx(p, ..., lim - p, ...) assuming (or not
> caring) about overflow goes badly wrong.

Normally, I updated all parts of the code that check the value returned by 
nla_strscpy.
But, if I understood correctly you are afraid of this type of code:
nla_strscpy(p, nla, p_len);
p += strncat(p, something, lim - p, ...);
Am I correct?

> To my mind returning the full buffer length (ie include the '\0')
> on overflow still allows overflow be checked but makes writes
> outside the buffer very unlikely.

Maybe I can keep the original behavior and add a pointer as argument which is 
used to contain -ERR?

> 	David
> 
> -
> Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1
> 1PT, UK Registration No: 1397386 (Wales)