On Thu, Jul 07, 2022 at 10:56:53AM +0200, Alexander Potapenko wrote: > On Thu, Jul 7, 2022 at 1:14 AM Kirill A. Shutemov > <kirill.shutemov@xxxxxxxxxxxxxxx> wrote: > > > > On Tue, Jul 05, 2022 at 05:42:21PM +0200, Alexander Potapenko wrote: > > > Kirill, > > > > > > > > > > diff --git a/lib/strnlen_user.c b/lib/strnlen_user.c > > > > index feeb935a2299..abc096a68f05 100644 > > > > --- a/lib/strnlen_user.c > > > > +++ b/lib/strnlen_user.c > > > > @@ -97,7 +97,7 @@ long strnlen_user(const char __user *str, long count) > > > > return 0; > > > > > > > > max_addr = TASK_SIZE_MAX; > > > > - src_addr = (unsigned long)untagged_addr(str); > > > > + src_addr = (unsigned long)untagged_addr(current->mm, str); > > > > > > In a downstream kernel with LAM disabled I'm seeing current->mm being > > > NULL at this point, because strnlen_user() is being called by > > > kdevtmpfs. > > > IIUC current->mm is only guaranteed to be non-NULL in the userspace > > > process context, whereas untagged_addr() may get called in random > > > places. > > > > > > Am I missing something? > > > > Hm. Could you show a traceback? > > > > As strnlen_user() intended to be used on an user string I expected it to > > be called from a process context. I guess I'm wrong, but I don't yet > > understand why. > > Oh, I see now. The old implementation of devtmpfsd() > (https://elixir.bootlin.com/linux/v5.4/source/drivers/base/devtmpfs.c#L397) > uses ksys_mount(), which assumes that the strings must be copied from > the userspace, whereas they are actually constants in kernel .rodata > > Wonder if the validity of mm->current for userspace accesses is > actually enforced anyhow in newer kernels. I think it is. See 967747bbc084 and how it changes strnlen_user(). With max_addr equal to TASK_SIZE_MAX, strnlen_user() will always fail on a kernel string. -- Kirill A. Shutemov
