On Fri, Oct 28, 2022 at 6:14 PM Luck, Tony <tony.luck@xxxxxxxxx> wrote: > > >> + vfrom = kmap_local_page(from); > >> + vto = kmap_local_page(to); > >> + ret = copy_mc_to_kernel(vto, vfrom, PAGE_SIZE); > > > > In copy_user_highpage(), kmsan_unpoison_memory(page_address(to), PAGE_SIZE) is done after the copy when > > __HAVE_ARCH_COPY_USER_HIGHPAGE isn't defined. Do we need to do something similar here? But I'm not familiar > > with kmsan, so I can easy be wrong. > > It looks like that kmsan_unpoison_memory() call was added recently, after I copied > copy_user_highpage() to create copy_mc_user_highpage(). I'm not familiar with > kmsan either. Adding Alexander to this thread since they added that code. > Given that copy_mc_user_highpage() replaces one of the calls to copy_user_highpage(), it sure makes sense to call kmsan_unpoison_memory() here. KMSAN tracks the status (initialized/uninitialized) of the kernel memory. Newly allocated memory is marked uninitialized, copying memory preserves its status, and writing constants to that memory makes it initialized. Userspace memory does not have its status tracked by KMSAN, so when values are copied from the userspace, KMSAN does nothing with their status. That's why every (successful) copy_from_user event should be followed by kmsan_unpoison_memory(), which marks the corresponding kernel buffer initialized - otherwise the status of that buffer may get stale. > > Anyway, this patch looks good to me. Thanks. > > > > Reviewed-by: Miaohe Lin <linmiaohe@xxxxxxxxxx> > > Thanks for the review. > > -Tony -- Alexander Potapenko Software Engineer Google Germany GmbH Erika-Mann-Straße, 33 80636 München Geschäftsführer: Paul Manicle, Liana Sebastian Registergericht und -nummer: Hamburg, HRB 86891 Sitz der Gesellschaft: Hamburg
