Return-Path: Received: from mail-oi0-f48.google.com ([209.85.218.48]:32927 "EHLO mail-oi0-f48.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750853AbdGNUuH (ORCPT ); Fri, 14 Jul 2017 16:50:07 -0400 MIME-Version: 1.0 In-Reply-To: <1500064716.3755.1.camel@gmail.com> References: <20170714142543.k5xcbnb4mww3sxpy@codemonkey.org.uk> <4c68e120-5ada-6ce1-30fd-e26155c9704e@virtuozzo.com> <1500064716.3755.1.camel@gmail.com> From: Linus Torvalds Date: Fri, 14 Jul 2017 13:50:06 -0700 Message-ID: Subject: Re: [GIT PULL] Please pull NFS client changes for Linux 4.13 To: Daniel Micay Cc: Andrey Ryabinin , Kees Cook , Dave Jones , Anna Schumaker , Linux NFS Mailing List , linux-fsdevel , Linux Kernel Mailing List , "J. Bruce Fields" , Alexander Potapenko , Dmitry Vyukov , kasan-dev@googlegroups.com Content-Type: text/plain; charset="UTF-8" Sender: linux-nfs-owner@vger.kernel.org List-ID: On Fri, Jul 14, 2017 at 1:38 PM, Daniel Micay wrote: > > If strscpy treats the count parameter as a *guarantee* of the dest size > rather than a limit, No, it's a *limit*. And by a *limit*, I mean that we know that we can access both source and destination within that limit. > My initial patch used strlcpy there, because I wasn't aware of strscpy > before it was suggested: Since I'm looking at this, I note that the "strlcpy()" code is complete garbage too, and has that same p_size == (size_t)-1 && q_size == (size_t)-1 check which is wrong. Of course, in strlcpy, q_size is never actually *used*, so the whole check seems bogus. But no, strlcpy() is complete garbage, and should never be used. It is truly a shit interface, and anybody who uses it is by definition buggy. Why? Because the return value of "strlcpy()" is defined to be ignoring the limit, so you FUNDAMENTALLY must not use that thing on untrusted source strings. But since the whole *point* of people using it is for untrusted sources, it by definition is garbage. Ergo: don't use strlcpy(). It's unbelievable crap. It's wrong. There's a reason we defined "strscpy()" as the way to do safe copies (strncpy(), of course, is broken for both lack of NUL termination _and_ for excessive NUL termination when a NUL did exist). So quite frankly, this hardening code needs to be looked at again. And no, if it uses "strlcpy()", then it's not hardering, it's just a pile of crap. Yes, I'm annoyed. I really get very very annoyed by "hardening" code that does nothing of the kind. Linus