Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

arm64: uaccess: avoid blocking within critical sections

As Vincent reports in:

https://lore.kernel.org/r/20211118163417.21617-1-vincent.whitchurch@axis.com

The put_user() in schedule_tail() can get stuck in a livelock, similar
to a problem recently fixed on riscv in commit:

285a76bb2cf51b0c ("riscv: evaluate put_user() arg before enabling user access")

In __raw_put_user() we have a critical section between
uaccess_ttbr0_enable() and uaccess_ttbr0_disable() where we cannot
safely call into the scheduler without having taken an exception, as
schedule() and other scheduling functions will not save/restore the
TTBR0 state. If either of the `x` or `ptr` arguments to __raw_put_user()
contain a blocking call, we may call into the scheduler within the
critical section. This can result in two problems:

1) The access within the critical section will occur without the
required TTBR0 tables installed. This will fault, and where the
required tables permit access, the access will be retried without the
required tables, resulting in a livelock.

2) When TTBR0 SW PAN is in use, check_and_switch_context() does not
modify TTBR0, leaving a stale value installed. The mappings of the
blocked task will erroneously be accessible to regular accesses in
the context of the new task. Additionally, if the tables are
subsequently freed, local TLB maintenance required to reuse the ASID
may be lost, potentially resulting in TLB corruption (e.g. in the
presence of CnP).

The same issue exists for __raw_get_user() in the critical section
between uaccess_ttbr0_enable() and uaccess_ttbr0_disable().

A similar issue exists for __get_kernel_nofault() and
__put_kernel_nofault() for the critical section between
__uaccess_enable_tco_async() and __uaccess_disable_tco_async(), as the
TCO state is not context-switched by direct calls into the scheduler.
Here the TCO state may be lost from the context of the current task,
resulting in unexpected asynchronous tag check faults. It may also be
leaked to another task, suppressing expected tag check faults.

To fix all of these cases, we must ensure that we do not directly call
into the scheduler in their respective critical sections. This patch
reworks __raw_put_user(), __raw_get_user(), __get_kernel_nofault(), and
__put_kernel_nofault(), ensuring that parameters are evaluated outside
of the critical sections. To make this requirement clear, comments are
added describing the problem, and line spaces added to separate the
critical sections from other portions of the macros.

For __raw_get_user() and __raw_put_user() the `err` parameter is
conditionally assigned to, and we must currently evaluate this in the
critical section. This behaviour is relied upon by the signal code,
which uses chains of put_user_error() and get_user_error(), checking the
return value at the end. In all cases, the `err` parameter is a plain
int rather than a more complex expression with a blocking call, so this
is safe.

In future we should try to clean up the `err` usage to remove the
potential for this to be a problem.

Aside from the changes to time of evaluation, there should be no
functional change as a result of this patch.

Reported-by: Vincent Whitchurch <vincent.whitchurch@axis.com>
Link: https://lore.kernel.org/r/20211118163417.21617-1-vincent.whitchurch@axis.com
Fixes: f253d827f33c ("arm64: uaccess: refactor __{get,put}_user")
Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Cc: Will Deacon <will@kernel.org>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Link: https://lore.kernel.org/r/20211122125820.55286-1-mark.rutland@arm.com
Signed-off-by: Will Deacon <will@kernel.org>

authored by

Mark Rutland and committed by
Will Deacon
94902d84 d3eb70ea

+41 -7
+41 -7
arch/arm64/include/asm/uaccess.h
··· 281 281 (x) = (__force __typeof__(*(ptr)))__gu_val; \ 282 282 } while (0) 283 283 284 + /* 285 + * We must not call into the scheduler between uaccess_ttbr0_enable() and 286 + * uaccess_ttbr0_disable(). As `x` and `ptr` could contain blocking functions, 287 + * we must evaluate these outside of the critical section. 288 + */ 284 289 #define __raw_get_user(x, ptr, err) \ 285 290 do { \ 291 + __typeof__(*(ptr)) __user *__rgu_ptr = (ptr); \ 292 + __typeof__(x) __rgu_val; \ 286 293 __chk_user_ptr(ptr); \ 294 + \ 287 295 uaccess_ttbr0_enable(); \ 288 - __raw_get_mem("ldtr", x, ptr, err); \ 296 + __raw_get_mem("ldtr", __rgu_val, __rgu_ptr, err); \ 289 297 uaccess_ttbr0_disable(); \ 298 + \ 299 + (x) = __rgu_val; \ 290 300 } while (0) 291 301 292 302 #define __get_user_error(x, ptr, err) \ ··· 320 310 321 311 #define get_user __get_user 322 312 313 + /* 314 + * We must not call into the scheduler between __uaccess_enable_tco_async() and 315 + * __uaccess_disable_tco_async(). As `dst` and `src` may contain blocking 316 + * functions, we must evaluate these outside of the critical section. 317 + */ 323 318 #define __get_kernel_nofault(dst, src, type, err_label) \ 324 319 do { \ 320 + __typeof__(dst) __gkn_dst = (dst); \ 321 + __typeof__(src) __gkn_src = (src); \ 325 322 int __gkn_err = 0; \ 326 323 \ 327 324 __uaccess_enable_tco_async(); \ 328 - __raw_get_mem("ldr", *((type *)(dst)), \ 329 - (__force type *)(src), __gkn_err); \ 325 + __raw_get_mem("ldr", *((type *)(__gkn_dst)), \ 326 + (__force type *)(__gkn_src), __gkn_err); \ 330 327 __uaccess_disable_tco_async(); \ 328 + \ 331 329 if (unlikely(__gkn_err)) \ 332 330 goto err_label; \ 333 331 } while (0) ··· 369 351 } \ 370 352 } while (0) 371 353 354 + /* 355 + * We must not call into the scheduler between uaccess_ttbr0_enable() and 356 + * uaccess_ttbr0_disable(). As `x` and `ptr` could contain blocking functions, 357 + * we must evaluate these outside of the critical section. 358 + */ 372 359 #define __raw_put_user(x, ptr, err) \ 373 360 do { \ 374 - __chk_user_ptr(ptr); \ 361 + __typeof__(*(ptr)) __user *__rpu_ptr = (ptr); \ 362 + __typeof__(*(ptr)) __rpu_val = (x); \ 363 + __chk_user_ptr(__rpu_ptr); \ 364 + \ 375 365 uaccess_ttbr0_enable(); \ 376 - __raw_put_mem("sttr", x, ptr, err); \ 366 + __raw_put_mem("sttr", __rpu_val, __rpu_ptr, err); \ 377 367 uaccess_ttbr0_disable(); \ 378 368 } while (0) 379 369 ··· 406 380 407 381 #define put_user __put_user 408 382 383 + /* 384 + * We must not call into the scheduler between __uaccess_enable_tco_async() and 385 + * __uaccess_disable_tco_async(). As `dst` and `src` may contain blocking 386 + * functions, we must evaluate these outside of the critical section. 387 + */ 409 388 #define __put_kernel_nofault(dst, src, type, err_label) \ 410 389 do { \ 390 + __typeof__(dst) __pkn_dst = (dst); \ 391 + __typeof__(src) __pkn_src = (src); \ 411 392 int __pkn_err = 0; \ 412 393 \ 413 394 __uaccess_enable_tco_async(); \ 414 - __raw_put_mem("str", *((type *)(src)), \ 415 - (__force type *)(dst), __pkn_err); \ 395 + __raw_put_mem("str", *((type *)(__pkn_src)), \ 396 + (__force type *)(__pkn_dst), __pkn_err); \ 416 397 __uaccess_disable_tco_async(); \ 398 + \ 417 399 if (unlikely(__pkn_err)) \ 418 400 goto err_label; \ 419 401 } while(0)