| Message ID | 20210318171111.706303-11-samitolvanen@google.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | BPF |
| Headers | show |
| Series | Add support for Clang CFI | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Not a local patch |
On Thu, Mar 18, 2021 at 10:11 AM Sami Tolvanen <samitolvanen@google.com> wrote: > > To ensure we take the actual address of a function in kernel text, use > __va_function. Otherwise, with CONFIG_CFI_CLANG, the compiler replaces > the address with a pointer to the CFI jump table, which is actually in > the module when compiled with CONFIG_LKDTM=m. Should patch 10 and 12 be reordered against one another? Otherwise it looks like 12 defines __va_function while 10 uses it? > > Signed-off-by: Sami Tolvanen <samitolvanen@google.com> > Acked-by: Kees Cook <keescook@chromium.org> > --- > drivers/misc/lkdtm/usercopy.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/misc/lkdtm/usercopy.c b/drivers/misc/lkdtm/usercopy.c > index 109e8d4302c1..d173d6175c87 100644 > --- a/drivers/misc/lkdtm/usercopy.c > +++ b/drivers/misc/lkdtm/usercopy.c > @@ -314,7 +314,7 @@ void lkdtm_USERCOPY_KERNEL(void) > > pr_info("attempting bad copy_to_user from kernel text: %px\n", > vm_mmap); > - if (copy_to_user((void __user *)user_addr, vm_mmap, > + if (copy_to_user((void __user *)user_addr, __va_function(vm_mmap), > unconst + PAGE_SIZE)) { > pr_warn("copy_to_user failed, but lacked Oops\n"); > goto free_user; > -- > 2.31.0.291.g576ba9dcdaf-goog > -- Thanks, ~Nick Desaulniers
On Thu, Mar 18, 2021 at 11:43 AM Nick Desaulniers <ndesaulniers@google.com> wrote: > > On Thu, Mar 18, 2021 at 10:11 AM Sami Tolvanen <samitolvanen@google.com> wrote: > > > > To ensure we take the actual address of a function in kernel text, use > > __va_function. Otherwise, with CONFIG_CFI_CLANG, the compiler replaces > > the address with a pointer to the CFI jump table, which is actually in > > the module when compiled with CONFIG_LKDTM=m. > > Should patch 10 and 12 be reordered against one another? Otherwise it > looks like 12 defines __va_function while 10 uses it? Ah, nvm patch 3 defines a generic version, I see. > > > > > > Signed-off-by: Sami Tolvanen <samitolvanen@google.com> > > Acked-by: Kees Cook <keescook@chromium.org> > > --- > > drivers/misc/lkdtm/usercopy.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/misc/lkdtm/usercopy.c b/drivers/misc/lkdtm/usercopy.c > > index 109e8d4302c1..d173d6175c87 100644 > > --- a/drivers/misc/lkdtm/usercopy.c > > +++ b/drivers/misc/lkdtm/usercopy.c > > @@ -314,7 +314,7 @@ void lkdtm_USERCOPY_KERNEL(void) > > > > pr_info("attempting bad copy_to_user from kernel text: %px\n", > > vm_mmap); > > - if (copy_to_user((void __user *)user_addr, vm_mmap, > > + if (copy_to_user((void __user *)user_addr, __va_function(vm_mmap), > > unconst + PAGE_SIZE)) { > > pr_warn("copy_to_user failed, but lacked Oops\n"); > > goto free_user; > > -- > > 2.31.0.291.g576ba9dcdaf-goog > > > > > -- > Thanks, > ~Nick Desaulniers
diff --git a/drivers/misc/lkdtm/usercopy.c b/drivers/misc/lkdtm/usercopy.c index 109e8d4302c1..d173d6175c87 100644 --- a/drivers/misc/lkdtm/usercopy.c +++ b/drivers/misc/lkdtm/usercopy.c @@ -314,7 +314,7 @@ void lkdtm_USERCOPY_KERNEL(void) pr_info("attempting bad copy_to_user from kernel text: %px\n", vm_mmap); - if (copy_to_user((void __user *)user_addr, vm_mmap, + if (copy_to_user((void __user *)user_addr, __va_function(vm_mmap), unconst + PAGE_SIZE)) { pr_warn("copy_to_user failed, but lacked Oops\n"); goto free_user;