Messages in this thread | | | Date | Tue, 7 Feb 2023 15:28:23 -0800 | From | Kees Cook <> | Subject | Re: [PATCH] uaccess: Add minimum bounds check on kernel buffer size |
| |
On Tue, Feb 07, 2023 at 10:06:14AM +0100, Yann Droneaud wrote: > Hi, > > Le 06/02/2023 à 21:03, Geert Uytterhoeven a écrit : > > > Hi Arnd, > > > > On Fri, Feb 3, 2023 at 10:23 PM Arnd Bergmann <arnd@arndb.de> wrote: > > > On Fri, Feb 3, 2023, at 20:35, Kees Cook wrote: > > > > --- a/include/linux/uaccess.h > > > > +++ b/include/linux/uaccess.h > > > > @@ -329,6 +329,10 @@ copy_struct_from_user(void *dst, size_t ksize, > > > > const void __user *src, > > > > size_t size = min(ksize, usize); > > > > size_t rest = max(ksize, usize) - size; > > > > > > > > + /* Double check if ksize is larger than a known object size. */ > > > > + if (WARN_ON_ONCE(ksize > __builtin_object_size(dst, 1))) > > > > + return -E2BIG; > > > > + > > > WARN_ON_ONCE() may be a little expensive since that adds two > > > comparisons and a static variable to each copy, but it's probably > > > fine. > > When seeing this, I was a bit worried about the size increase. > > Hence I gave it a try on atari_defconfig and ran bloat-o-meter. > > Surprisingly, there was no size increase at all, as all checks > > were optimized away. > > > > Hence perhaps this can become a compile-time check? > > It should be a compile-time check, because one would not want __builtin_object_size(dst, 1) to return -1 if dst' size is not known at compile-time.
Note that it's size_t, so it's actually SIZE_MAX, which is why these tests will vanish most of the time. i.e. it cannot ever be possible for the SIZE_MAX case so the entire test is elided.
And when ksize is known at compile time and __bos is not SIZE_MAX, the result is also known to be either always true or always false, etc.
What's nice here is that when ksize is only run-time known and the buffer size is compile-time known, we'll keep the test.
-- Kees Cook
| |