linux-kernel - Re: [PATCH v3] lib/string.c: implement stpcpy

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:   Thu, 27 Aug 2020 11:59:24 +0300
From:   Andy Shevchenko <andy.shevchenko@...il.com>
To:     Kees Cook <keescook@...omium.org>
Cc:     Masahiro Yamada <masahiroy@...nel.org>,
        Nick Desaulniers <ndesaulniers@...gle.com>,
        Joe Perches <joe@...ches.com>,
        clang-built-linux <clang-built-linux@...glegroups.com>,
        stable <stable@...r.kernel.org>, Andy Lavr <andy.lavr@...il.com>,
        Arvind Sankar <nivedita@...m.mit.edu>,
        Rasmus Villemoes <linux@...musvillemoes.dk>,
        Sami Tolvanen <samitolvanen@...gle.com>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
        Alexandru Ardelean <alexandru.ardelean@...log.com>,
        Yury Norov <yury.norov@...il.com>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v3] lib/string.c: implement stpcpy

On Thu, Aug 27, 2020 at 2:40 AM Kees Cook <keescook@...omium.org> wrote:
> On Thu, Aug 27, 2020 at 07:59:45AM +0900, Masahiro Yamada wrote:
> > On Thu, Aug 27, 2020 at 1:58 AM Nick Desaulniers
> > <ndesaulniers@...gle.com> wrote:
> > > On Wed, Aug 26, 2020 at 9:57 AM Joe Perches <joe@...ches.com> wrote:
> > > > On Thu, 2020-08-27 at 01:49 +0900, Masahiro Yamada wrote:
> > > > > I do not have time to keep track of the discussion fully,
> > > > > but could you give me a little more context why
> > > > > the usage of stpcpy() is not recommended ?
> > > > >
> > > > > The implementation of strcpy() is almost the same.
> > > > > It is unclear to me what makes stpcpy() unsafe..
> > >
> > > https://lore.kernel.org/lkml/202008150921.B70721A359@keescook/
> > >
> > > >
> > > > It's the same thing that makes strcpy unsafe:
> > > >
> > > > Unchecked buffer lengths with no guarantee src is terminated.
> > >
> >
> >
> > OK, then stpcpy(), strcpy() and sprintf()
> > have the same level of unsafety.
>
> Yes. And even snprintf() is dangerous because its return value is how
> much it WOULD have written, which when (commonly) used as an offset for
> further pointer writes, causes OOB writes too. :(
> https://github.com/KSPP/linux/issues/105
>
> > strcpy() is used everywhere.
>
> Yes. It's very frustrating, but it's not an excuse to continue
> using it nor introducing more bad APIs.

strcpy() is not a bad API for the cases when you know what you are
doing. A problem that most of the developers do not know what they are
doing.
No need to split everything to bad and good by its name or semantics,
each API has its own pros and cons and programmers must use their
brains.

>
> $ git grep '\bstrcpy\b' | wc -l
> 2212
> $ git grep '\bstrncpy\b' | wc -l
> 751
> $ git grep '\bstrlcpy\b' | wc -l
> 1712
>
> $ git grep '\bstrscpy\b' | wc -l
> 1066
>
> https://www.kernel.org/doc/html/latest/process/deprecated.html#strcpy
> https://github.com/KSPP/linux/issues/88
>
> https://www.kernel.org/doc/html/latest/process/deprecated.html#strncpy-on-nul-terminated-strings
> https://github.com/KSPP/linux/issues/89
>
> https://www.kernel.org/doc/html/latest/process/deprecated.html#strlcpy
> https://github.com/KSPP/linux/issues/90
>
> We have no way right now to block the addition of deprecated API usage,
> which makes ever catching up on this replacement very challenging. The
> only way we caught up with VLA removal was because of -Wvla on sfr's
> -next builds.
>
> I guess we could set up a robot to just watch -next commits and yell
> about new instances, but patches come and go -- I worry it'd be noisy...
>
> > I am not convinced why only stpcpy() should be hidden.
>
> Because nothing uses it right now. It's only the compiler suddenly now
> trying to use it directly...
>
> --
> Kees Cook



-- 
With Best Regards,
Andy Shevchenko