netdev - Re: [RFC][PATCH v3 3/3] Rename nla_strlcpy to nla

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]

Date:   Fri, 23 Oct 2020 17:15:08 +0200
From:   Francis Laniel <laniel_francis@...vacyrequired.com>
To:     David Laight <David.Laight@...lab.com>
Cc:     'Jakub Kicinski' <kuba@...nel.org>,
        Kees Cook <keescook@...omium.org>,
        "linux-hardening@...r.kernel.org" <linux-hardening@...r.kernel.org>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "davem@...emloft.net" <davem@...emloft.net>
Subject: Re: [RFC][PATCH v3 3/3] Rename nla_strlcpy to nla_strscpy.

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@...vacyrequired.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@...vacyrequired.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)