| Message ID | 20220701142310.2188015-4-glider@google.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Add KernelMemorySanitizer infrastructure | expand |
On Fri, 1 Jul 2022 at 16:23, Alexander Potapenko <glider@google.com> wrote: > > Introduce instrument_copy_from_user_before() and > instrument_copy_from_user_after() hooks to be invoked before and after > the call to copy_from_user(). > > KASAN and KCSAN will be only using instrument_copy_from_user_before(), > but for KMSAN we'll need to insert code after copy_from_user(). > > Signed-off-by: Alexander Potapenko <glider@google.com> Reviewed-by: Marco Elver <elver@google.com> > --- > v4: > -- fix _copy_from_user_key() in arch/s390/lib/uaccess.c (Reported-by: > kernel test robot <lkp@intel.com>) > > Link: https://linux-review.googlesource.com/id/I855034578f0b0f126734cbd734fb4ae1d3a6af99 > --- > arch/s390/lib/uaccess.c | 3 ++- > include/linux/instrumented.h | 21 +++++++++++++++++++-- > include/linux/uaccess.h | 19 ++++++++++++++----- > lib/iov_iter.c | 9 ++++++--- > lib/usercopy.c | 3 ++- > 5 files changed, 43 insertions(+), 12 deletions(-) > > diff --git a/arch/s390/lib/uaccess.c b/arch/s390/lib/uaccess.c > index d7b3b193d1088..58033dfcb6d45 100644 > --- a/arch/s390/lib/uaccess.c > +++ b/arch/s390/lib/uaccess.c > @@ -81,8 +81,9 @@ unsigned long _copy_from_user_key(void *to, const void __user *from, > > might_fault(); > if (!should_fail_usercopy()) { > - instrument_copy_from_user(to, from, n); > + instrument_copy_from_user_before(to, from, n); > res = raw_copy_from_user_key(to, from, n, key); > + instrument_copy_from_user_after(to, from, n, res); > } > if (unlikely(res)) > memset(to + (n - res), 0, res); > diff --git a/include/linux/instrumented.h b/include/linux/instrumented.h > index 42faebbaa202a..ee8f7d17d34f5 100644 > --- a/include/linux/instrumented.h > +++ b/include/linux/instrumented.h > @@ -120,7 +120,7 @@ instrument_copy_to_user(void __user *to, const void *from, unsigned long n) > } > > /** > - * instrument_copy_from_user - instrument writes of copy_from_user > + * instrument_copy_from_user_before - add instrumentation before copy_from_user > * > * Instrument writes to kernel memory, that are due to copy_from_user (and > * variants). The instrumentation should be inserted before the accesses. > @@ -130,10 +130,27 @@ instrument_copy_to_user(void __user *to, const void *from, unsigned long n) > * @n number of bytes to copy > */ > static __always_inline void > -instrument_copy_from_user(const void *to, const void __user *from, unsigned long n) > +instrument_copy_from_user_before(const void *to, const void __user *from, unsigned long n) > { > kasan_check_write(to, n); > kcsan_check_write(to, n); > } > > +/** > + * instrument_copy_from_user_after - add instrumentation after copy_from_user > + * > + * Instrument writes to kernel memory, that are due to copy_from_user (and > + * variants). The instrumentation should be inserted after the accesses. > + * > + * @to destination address > + * @from source address > + * @n number of bytes to copy > + * @left number of bytes not copied (as returned by copy_from_user) > + */ > +static __always_inline void > +instrument_copy_from_user_after(const void *to, const void __user *from, > + unsigned long n, unsigned long left) > +{ > +} > + > #endif /* _LINUX_INSTRUMENTED_H */ > diff --git a/include/linux/uaccess.h b/include/linux/uaccess.h > index 5a328cf02b75e..da16e96680cf1 100644 > --- a/include/linux/uaccess.h > +++ b/include/linux/uaccess.h > @@ -58,20 +58,28 @@ > static __always_inline __must_check unsigned long > __copy_from_user_inatomic(void *to, const void __user *from, unsigned long n) > { > - instrument_copy_from_user(to, from, n); > + unsigned long res; > + > + instrument_copy_from_user_before(to, from, n); > check_object_size(to, n, false); > - return raw_copy_from_user(to, from, n); > + res = raw_copy_from_user(to, from, n); > + instrument_copy_from_user_after(to, from, n, res); > + return res; > } > > static __always_inline __must_check unsigned long > __copy_from_user(void *to, const void __user *from, unsigned long n) > { > + unsigned long res; > + > might_fault(); > + instrument_copy_from_user_before(to, from, n); > if (should_fail_usercopy()) > return n; > - instrument_copy_from_user(to, from, n); > check_object_size(to, n, false); > - return raw_copy_from_user(to, from, n); > + res = raw_copy_from_user(to, from, n); > + instrument_copy_from_user_after(to, from, n, res); > + return res; > } > > /** > @@ -115,8 +123,9 @@ _copy_from_user(void *to, const void __user *from, unsigned long n) > unsigned long res = n; > might_fault(); > if (!should_fail_usercopy() && likely(access_ok(from, n))) { > - instrument_copy_from_user(to, from, n); > + instrument_copy_from_user_before(to, from, n); > res = raw_copy_from_user(to, from, n); > + instrument_copy_from_user_after(to, from, n, res); > } > if (unlikely(res)) > memset(to + (n - res), 0, res); > diff --git a/lib/iov_iter.c b/lib/iov_iter.c > index 0b64695ab632f..fe5d169314dbf 100644 > --- a/lib/iov_iter.c > +++ b/lib/iov_iter.c > @@ -159,13 +159,16 @@ static int copyout(void __user *to, const void *from, size_t n) > > static int copyin(void *to, const void __user *from, size_t n) > { > + size_t res = n; > + > if (should_fail_usercopy()) > return n; > if (access_ok(from, n)) { > - instrument_copy_from_user(to, from, n); > - n = raw_copy_from_user(to, from, n); > + instrument_copy_from_user_before(to, from, n); > + res = raw_copy_from_user(to, from, n); > + instrument_copy_from_user_after(to, from, n, res); > } > - return n; > + return res; > } > > static size_t copy_page_to_iter_iovec(struct page *page, size_t offset, size_t bytes, > diff --git a/lib/usercopy.c b/lib/usercopy.c > index 7413dd300516e..1505a52f23a01 100644 > --- a/lib/usercopy.c > +++ b/lib/usercopy.c > @@ -12,8 +12,9 @@ unsigned long _copy_from_user(void *to, const void __user *from, unsigned long n > unsigned long res = n; > might_fault(); > if (!should_fail_usercopy() && likely(access_ok(from, n))) { > - instrument_copy_from_user(to, from, n); > + instrument_copy_from_user_before(to, from, n); > res = raw_copy_from_user(to, from, n); > + instrument_copy_from_user_after(to, from, n, res); > } > if (unlikely(res)) > memset(to + (n - res), 0, res); > -- > 2.37.0.rc0.161.g10f37bed90-goog >
diff --git a/arch/s390/lib/uaccess.c b/arch/s390/lib/uaccess.c index d7b3b193d1088..58033dfcb6d45 100644 --- a/arch/s390/lib/uaccess.c +++ b/arch/s390/lib/uaccess.c @@ -81,8 +81,9 @@ unsigned long _copy_from_user_key(void *to, const void __user *from, might_fault(); if (!should_fail_usercopy()) { - instrument_copy_from_user(to, from, n); + instrument_copy_from_user_before(to, from, n); res = raw_copy_from_user_key(to, from, n, key); + instrument_copy_from_user_after(to, from, n, res); } if (unlikely(res)) memset(to + (n - res), 0, res); diff --git a/include/linux/instrumented.h b/include/linux/instrumented.h index 42faebbaa202a..ee8f7d17d34f5 100644 --- a/include/linux/instrumented.h +++ b/include/linux/instrumented.h @@ -120,7 +120,7 @@ instrument_copy_to_user(void __user *to, const void *from, unsigned long n) } /** - * instrument_copy_from_user - instrument writes of copy_from_user + * instrument_copy_from_user_before - add instrumentation before copy_from_user * * Instrument writes to kernel memory, that are due to copy_from_user (and * variants). The instrumentation should be inserted before the accesses. @@ -130,10 +130,27 @@ instrument_copy_to_user(void __user *to, const void *from, unsigned long n) * @n number of bytes to copy */ static __always_inline void -instrument_copy_from_user(const void *to, const void __user *from, unsigned long n) +instrument_copy_from_user_before(const void *to, const void __user *from, unsigned long n) { kasan_check_write(to, n); kcsan_check_write(to, n); } +/** + * instrument_copy_from_user_after - add instrumentation after copy_from_user + * + * Instrument writes to kernel memory, that are due to copy_from_user (and + * variants). The instrumentation should be inserted after the accesses. + * + * @to destination address + * @from source address + * @n number of bytes to copy + * @left number of bytes not copied (as returned by copy_from_user) + */ +static __always_inline void +instrument_copy_from_user_after(const void *to, const void __user *from, + unsigned long n, unsigned long left) +{ +} + #endif /* _LINUX_INSTRUMENTED_H */ diff --git a/include/linux/uaccess.h b/include/linux/uaccess.h index 5a328cf02b75e..da16e96680cf1 100644 --- a/include/linux/uaccess.h +++ b/include/linux/uaccess.h @@ -58,20 +58,28 @@ static __always_inline __must_check unsigned long __copy_from_user_inatomic(void *to, const void __user *from, unsigned long n) { - instrument_copy_from_user(to, from, n); + unsigned long res; + + instrument_copy_from_user_before(to, from, n); check_object_size(to, n, false); - return raw_copy_from_user(to, from, n); + res = raw_copy_from_user(to, from, n); + instrument_copy_from_user_after(to, from, n, res); + return res; } static __always_inline __must_check unsigned long __copy_from_user(void *to, const void __user *from, unsigned long n) { + unsigned long res; + might_fault(); + instrument_copy_from_user_before(to, from, n); if (should_fail_usercopy()) return n; - instrument_copy_from_user(to, from, n); check_object_size(to, n, false); - return raw_copy_from_user(to, from, n); + res = raw_copy_from_user(to, from, n); + instrument_copy_from_user_after(to, from, n, res); + return res; } /** @@ -115,8 +123,9 @@ _copy_from_user(void *to, const void __user *from, unsigned long n) unsigned long res = n; might_fault(); if (!should_fail_usercopy() && likely(access_ok(from, n))) { - instrument_copy_from_user(to, from, n); + instrument_copy_from_user_before(to, from, n); res = raw_copy_from_user(to, from, n); + instrument_copy_from_user_after(to, from, n, res); } if (unlikely(res)) memset(to + (n - res), 0, res); diff --git a/lib/iov_iter.c b/lib/iov_iter.c index 0b64695ab632f..fe5d169314dbf 100644 --- a/lib/iov_iter.c +++ b/lib/iov_iter.c @@ -159,13 +159,16 @@ static int copyout(void __user *to, const void *from, size_t n) static int copyin(void *to, const void __user *from, size_t n) { + size_t res = n; + if (should_fail_usercopy()) return n; if (access_ok(from, n)) { - instrument_copy_from_user(to, from, n); - n = raw_copy_from_user(to, from, n); + instrument_copy_from_user_before(to, from, n); + res = raw_copy_from_user(to, from, n); + instrument_copy_from_user_after(to, from, n, res); } - return n; + return res; } static size_t copy_page_to_iter_iovec(struct page *page, size_t offset, size_t bytes, diff --git a/lib/usercopy.c b/lib/usercopy.c index 7413dd300516e..1505a52f23a01 100644 --- a/lib/usercopy.c +++ b/lib/usercopy.c @@ -12,8 +12,9 @@ unsigned long _copy_from_user(void *to, const void __user *from, unsigned long n unsigned long res = n; might_fault(); if (!should_fail_usercopy() && likely(access_ok(from, n))) { - instrument_copy_from_user(to, from, n); + instrument_copy_from_user_before(to, from, n); res = raw_copy_from_user(to, from, n); + instrument_copy_from_user_after(to, from, n, res); } if (unlikely(res)) memset(to + (n - res), 0, res);
Introduce instrument_copy_from_user_before() and instrument_copy_from_user_after() hooks to be invoked before and after the call to copy_from_user(). KASAN and KCSAN will be only using instrument_copy_from_user_before(), but for KMSAN we'll need to insert code after copy_from_user(). Signed-off-by: Alexander Potapenko <glider@google.com> --- v4: -- fix _copy_from_user_key() in arch/s390/lib/uaccess.c (Reported-by: kernel test robot <lkp@intel.com>) Link: https://linux-review.googlesource.com/id/I855034578f0b0f126734cbd734fb4ae1d3a6af99 --- arch/s390/lib/uaccess.c | 3 ++- include/linux/instrumented.h | 21 +++++++++++++++++++-- include/linux/uaccess.h | 19 ++++++++++++++----- lib/iov_iter.c | 9 ++++++--- lib/usercopy.c | 3 ++- 5 files changed, 43 insertions(+), 12 deletions(-)