Messages in this thread | | | Subject | Re: [PATCH] lib/strscpy: avoid KASAN false positive | From | Chris Metcalf <> | Date | Wed, 19 Jul 2017 11:39:32 -0400 |
| |
On 7/18/2017 6:04 PM, Andrew Morton wrote: > On Wed, 19 Jul 2017 00:31:36 +0300 Andrey Ryabinin <aryabinin@virtuozzo.com> wrote: > >> On 07/18/2017 11:26 PM, Linus Torvalds wrote: >>> On Tue, Jul 18, 2017 at 1:15 PM, Andrey Ryabinin >>> <aryabinin@virtuozzo.com> wrote: >>>> No, it does warn about valid users. The report that Dave posted wasn't about wrong strscpy() usage >>>> it was about reading 8-bytes from 5-bytes source string. It wasn't about buggy 'count' at all. >>>> So KASAN will warn for perfectly valid code like this: >>>> char dest[16]; >>>> strscpy(dest, "12345", sizeof(dest)): >>> Ugh, ok, yes. >>> >>>> For strscpy() that would mean making the *whole* read from 'src' buffer unchecked by KASAN. >>> So we do have that READ_ONCE_NOCHECK(), but could we perhaps have >>> something that doesn't do a NOCHECK but a partial check and is simply >>> ok with "this is an optimistc longer access" >>> >> This can be dont, I think. >> >> Something like this: >> static inline unsigned long read_partial_nocheck(unsigned long *x) >> { >> unsigned long ret = READ_ONCE_NOCHECK(x); >> kasan_check_partial(x, sizeof(unsigned long)); >> return ret; >> } >> > (Cc Chris) > > We could just remove all that word-at-a-time logic. Do we have any > evidence that this would harm anything?
The word-at-a-time logic was part of the initial commit since I wanted to ensure that strscpy could be used to replace strlcpy or strncpy without serious concerns about performance. It seems unfortunate to remove it unconditionally to support KASAN, but I haven't looked deeply at the tradeoffs here.
-- Chris Metcalf, Mellanox Technologies http://www.mellanox.com
| |