Re: [PATCH] Fixing returning a userspace address for return value

On 10/19/2021 11:41 AM, James Morris wrote:

On Tue, 19 Oct 2021, T. Williams wrote:

quoted

Fixing user memory dereference bug.

Signed-off-by: Thelford Williams <redacted>

Casey, can you check the logic on this?

quoted

---
  security/security.c | 2 ++
  1 file changed, 2 insertions(+)

diff --git a/security/security.c b/security/security.c
index 9ffa9e9c5c55..7c41b5d732ab 100644
--- a/security/security.c
+++ b/security/security.c

@@ -1737,6 +1737,8 @@ int security_kernel_read_file(struct file *file, enum

kernel_read_file_id id,
         int ret;

         ret = call_int_hook(kernel_read_file, 0, file, id, contents);
+       if (ret > 0)
+               return -EINVAL;

Does the failing configuration include a BPF program that might be
invoked for this hook? SELinux is the only traditional security module
that provides a hook, and it never returns a positive value. I don't
see a case where the proposed check makes any sense. If the problem is
in a BPF program it should be fixed there.

quoted

         if (ret)
                 return ret;
         return ima_read_file(file, id, contents);
-- 
2.25.1

This commit is to fix a userspace address dereference found by
syzkaller.
The crash is triggered by passing a file descriptor to an incorrectly
formed kernel module to finit_module.

Kernel/module.c:4175 : Within the finit_module, info.len is set to the
return value from kernel_read_file_from_fd. This value is then
dereferenced by memcmp within module_sig_check from inside load_module.
The value is 0xb000000 so the kernel dereferences user memory and kernel
panics.

To prevent this adding a check from within security_kernel_read_file to
make sure call_int_hook doesn't return an address which in the syzkaller
program is what causes the return value to be 0xb000000. Then the return
value of security_kernel_read_file is returned to kernel_read_file(also
in security/security.c) which returns the value to
kernel_read_file_from_fd (fs/kernel_read_file.c) and this returns the
value into err then being set to info.len causing the dereference when
info is passed into load_module.