<From:>
git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?id=a180bd1d7e16173d965b263c5a536aa40afa2a2a
> iov_iter: remove uaccess_kernel() warning from iov_iter_init()
> This warning was there to catch any architectures that still use
> CONFIG_SET_FS, and that would mis-use iov_iter_init() for anything that
> wasn't a proper user space pointer. So that
>
> WARN_ON_ONCE(uaccess_kernel());
>
> makes perfect conceptual sense: you really shouldn't use a kernel
> pointer with set_fs(KERNEL_DS) and then pass it to iov_iter_init().
>
> HOWEVER.
>
> Guenter Roeck reports that this warning actually triggers in no-mmu
> configurations of both ARM and m68k. And the reason isn't that they
> pass in a kernel pointer under set_fs(KERNEL_DS) at all: the reason is
> that in those configurations, "uaccess_kernel()" is simply not reliable.
>
> Those no-mmu setups set USER_DS and KERNEL_DS to the same values, so you
> can't test for the difference.
>
> In particular, the no-mmu case for ARM does
>
> #define USER_DS KERNEL_DS
> #define uaccess_kernel() (true)
>
> so USER_DS and KERNEL_DS have the same value, and uaccess_kernel() is
> always trivially true.
>
> The m68k case is slightly different and not quite as obvious. It does
> (spread out over multiple header files just to be extra exciting:
> asm/processor.h, asm/segment.h and asm-generic/uaccess.h):
>
> #define TASK_SIZE (0xFFFFFFFFUL)
> #define USER_DS MAKE_MM_SEG(TASK_SIZE)
> #define KERNEL_DS MAKE_MM_SEG(~0UL)
> #define get_fs() (current_thread_info()->addr_limit)
> #define uaccess_kernel() (get_fs().seg == KERNEL_DS.seg)
>
> but the end result is the same: uaccess_kernel() will always be true,
> because USER_DS and KERNEL_DS end up having the same value, even if that
> value is defined differently.
>
> This is very arguably a misfeature in those implementations, but in the
> end we don't really care. All modern architectures have gotten rid of
> set_fs() already, and generic kernel code never uses it. And while the
> sanity check was a nice idea, an architecture would have to go the extra
> mile to actually break this.
>
> So this well-intentioned warning isn't really all that likely to find
> anything but these known false positives, and as such just isn't worth
> maintaining.
>
> -rw-r--r-- lib/iov_iter.c 1
> 1 files changed, 0 insertions, 1 deletions
> diff --git a/lib/iov_iter.c b/lib/iov_iter.c
> index 97e04c5dbeef2..e23123ae3a137 100644
> --- a/lib/iov_iter.c
> +++ b/lib/iov_iter.c
> @@ -465,7 +465,6 @@ void iov_iter_init(struct iov_iter *i, unsigned int direction,
> size_t count)
> {
> WARN_ON(direction & ~(READ | WRITE));
> - WARN_ON_ONCE(uaccess_kernel());
Even though uaccess_kernel() is not reliable, it is not best way to remove
the whole statement with uaccess_kernel().
> *i = (struct iov_iter) {
> .iter_type = ITER_IOVEC,
> .data_source = direction,
uaccess_kernel() is simply used to check whether current is a kernel thread.
So uaccess_kernel() can be replaced with 'current->flags & PF_KTHREAD' statement.
diff --git a/lib/iov_iter.c b/lib/iov_iter.c
index 66a740e6e153..369b278fe215 100644
--- a/lib/iov_iter.c
+++ b/lib/iov_iter.c
@@ -511,6 +511,7 @@ void iov_iter_init(struct iov_iter *i, unsigned int direction,
size_t count)
{
WARN_ON(direction & ~(READ | WRITE));
+ WARN_ON(current->flags & PF_KTHREAD);
*i = (struct iov_iter) {
.iter_type = ITER_IOVEC,
.nofault = false,
최근 덧글