| Message ID | 70f99f7474826883877e84f93224e937d9c974de.1579767339.git.christophe.leroy@c-s.fr (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | lib: Reduce user_access_begin() boundaries in strncpy_from_user() and strnlen_user() | expand |
On Thu, Jan 23, 2020 at 12:34 AM Christophe Leroy <christophe.leroy@c-s.fr> wrote: > > The range passed to user_access_begin() by strncpy_from_user() and > strnlen_user() starts at 'src' and goes up to the limit of userspace > allthough reads will be limited by the 'count' param. > > On 32 bits powerpc (book3s/32) access has to be granted for each 256Mbytes > segment and the cost increases with the number of segments to unlock. > > Limit the range with 'count' param. Ack. I'm tempted to take this for 5.5 too, just so that the unquestionably trivial fixes are in that baseline, and the infrastructure is ready for any architecture that has issues like this. Adding 'linux-arch' to the participants, to see if other architectures are at all looking at actually implementing the whole user_access_begin/end() dance too.. Linus
Le 23/01/2020 à 19:47, Linus Torvalds a écrit : > On Thu, Jan 23, 2020 at 12:34 AM Christophe Leroy > <christophe.leroy@c-s.fr> wrote: >> >> The range passed to user_access_begin() by strncpy_from_user() and >> strnlen_user() starts at 'src' and goes up to the limit of userspace >> allthough reads will be limited by the 'count' param. >> >> On 32 bits powerpc (book3s/32) access has to be granted for each 256Mbytes >> segment and the cost increases with the number of segments to unlock. >> >> Limit the range with 'count' param. > > Ack. I'm tempted to take this for 5.5 too, just so that the > unquestionably trivial fixes are in that baseline, and the > infrastructure is ready for any architecture that has issues like > this. It would be nice, then the user_access_begin stuff for powerpc could go for 5.6 without worring about. Thanks Christophe
diff --git a/lib/strncpy_from_user.c b/lib/strncpy_from_user.c index dccb95af6003..706020b06617 100644 --- a/lib/strncpy_from_user.c +++ b/lib/strncpy_from_user.c @@ -30,13 +30,6 @@ static inline long do_strncpy_from_user(char *dst, const char __user *src, const struct word_at_a_time constants = WORD_AT_A_TIME_CONSTANTS; unsigned long res = 0; - /* - * Truncate 'max' to the user-specified limit, so that - * we only have one limit we need to check in the loop - */ - if (max > count) - max = count; - if (IS_UNALIGNED(src, dst)) goto byte_at_a_time; @@ -114,6 +107,13 @@ long strncpy_from_user(char *dst, const char __user *src, long count) unsigned long max = max_addr - src_addr; long retval; + /* + * Truncate 'max' to the user-specified limit, so that + * we only have one limit we need to check in the loop + */ + if (max > count) + max = count; + kasan_check_write(dst, count); check_object_size(dst, count, false); if (user_access_begin(src, max)) { diff --git a/lib/strnlen_user.c b/lib/strnlen_user.c index 6c0005d5dd5c..41670d4a5816 100644 --- a/lib/strnlen_user.c +++ b/lib/strnlen_user.c @@ -26,13 +26,6 @@ static inline long do_strnlen_user(const char __user *src, unsigned long count, unsigned long align, res = 0; unsigned long c; - /* - * Truncate 'max' to the user-specified limit, so that - * we only have one limit we need to check in the loop - */ - if (max > count) - max = count; - /* * Do everything aligned. But that means that we * need to also expand the maximum.. @@ -109,6 +102,13 @@ long strnlen_user(const char __user *str, long count) unsigned long max = max_addr - src_addr; long retval; + /* + * Truncate 'max' to the user-specified limit, so that + * we only have one limit we need to check in the loop + */ + if (max > count) + max = count; + if (user_access_begin(str, max)) { retval = do_strnlen_user(str, count, max); user_access_end();
The range passed to user_access_begin() by strncpy_from_user() and strnlen_user() starts at 'src' and goes up to the limit of userspace allthough reads will be limited by the 'count' param. On 32 bits powerpc (book3s/32) access has to be granted for each 256Mbytes segment and the cost increases with the number of segments to unlock. Limit the range with 'count' param. Fixes: 594cc251fdd0 ("make 'user_access_begin()' do 'access_ok()'") Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr> --- lib/strncpy_from_user.c | 14 +++++++------- lib/strnlen_user.c | 14 +++++++------- 2 files changed, 14 insertions(+), 14 deletions(-)