Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <527eee19-532f-b2e7-a42f-a1e199094fbe@rasmusvillemoes.dk>
Date:   Fri, 26 Aug 2022 09:40:20 +0200
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
 Thunderbird/91.11.0
Subject: Re: strlcpy() notes (was Re: [GIT PULL] smb3 client fixes)
Content-Language: en-US
To:     Linus Torvalds <torvalds@linux-foundation.org>,
        Catalin Marinas <catalin.marinas@arm.com>
Cc:     Steve French <smfrench@gmail.com>,
        CIFS <linux-cifs@vger.kernel.org>,
        LKML <linux-kernel@vger.kernel.org>,
        linux-arch <linux-arch@vger.kernel.org>
References: <CAH2r5mtDFpaAWeGCtrfm_WPM6j-Gkt_O80=nKfp6y39aXaBr6w@mail.gmail.com>
 <CAHk-=wi+xbVq++uqW9YgWpHjyBHNB8a-xad+Xp23-B+eodLCEA@mail.gmail.com>
 <YwSWLH4Wp6yDMeKf@arm.com>
 <CAHk-=whU7QiwWeO81Rf+KGh0rGS9CEfKUXc5eik+Z0GaVJgu4A@mail.gmail.com>
From:   Rasmus Villemoes <linux@rasmusvillemoes.dk>
In-Reply-To: <CAHk-=whU7QiwWeO81Rf+KGh0rGS9CEfKUXc5eik+Z0GaVJgu4A@mail.gmail.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 23/08/2022 19.37, Linus Torvalds wrote:
> On Tue, Aug 23, 2022 at 1:56 AM Catalin Marinas <catalin.marinas@arm.com> wrote:
>>
>> With load_unaligned_zeropad(), the arm64 implementation disables tag
>> checking temporarily. We could do the same with read_word_at_a_time()
>> (there is a kasan_check_read() in this function but it wrongly uses a
>> size of 1).
> 
> The "size of 1" is not wrong, it's intentional, exactly because people
> do things like
> 
>     strscpy(dst, "string", sizeof(dst));
> 
> which is a bit unfortunate, but very understandable and intended to
> work. So that thing may over-read the string by up to a word. And
> KASAN ends up being unhappy.

So, while we're doing all the churn of replacing strlcpy anyway, may I
once again suggest we add (name can be bikeshedded) literal_strcpy():

#define literal_strcpy(d, s) ({ \
  static_assert(__same_type(d, char[]), "destination must be char array"); \
  static_assert(__same_type(s, const char[]), "source must be a string
literal"); \
  static_assert(sizeof(d) >= sizeof("" s ""), "source does not fit in
destination"); \
  strcpy(d, s); \
})

That interface _cannot_ be misused, because all the checking happens at
build time, including enforcement that the source really is a string
literal (the "" s "" trick - but for nicer error message the
static_assert on the previous line is there as well). So unlike all the
uses of str[ls]cpy which don't check the return value, we cannot
silently do a truncated copy. Also, if somebody down the line changes
the size of the destination or the literal string, again it will be
caught at build time.

And since gcc knows the semantics of strcpy(), it will also generate
better code, because it will usually not emit a call at all (or even put
the string in .rodata); it will simply emit a series of "mov immediate"
instructions.

Sloppy grepping for places where that could be used shows around ~800
places.

Btw, Steve, since you're incidentally on cc here anyway, perhaps you
want to take a look at

  strscpy(extension, "___", strlen("___"));

and see if this really just wants two underscores copied to extension.

Rasmus