Messages in this thread | | | Subject | Re: [Linux-kernel-mentees] [PATCH v2] block/floppy: Prevent kernel-infoleak in raw_cmd_copyout() | From | Denis Efremov <> | Date | Wed, 29 Jul 2020 16:22:41 +0300 |
| |
On 7/29/20 3:58 PM, Dan Carpenter wrote: > Argh... This isn't right still. The "ptr" comes from raw_cmd_copyin() > > ptr = kmalloc(sizeof(struct floppy_raw_cmd), GFP_KERNEL); >
copy_from_user overwrites the padding bytes: ptr = kmalloc(sizeof(struct floppy_raw_cmd), GFP_KERNEL); if (!ptr) return -ENOMEM; *rcmd = ptr; ret = copy_from_user(ptr, param, sizeof(*ptr));
I think memcpy should be safe in this patch.
I've decided to dig a bit into the issue and to run some tests. Here are my observations:
$ cat test.c
#include <stdio.h> #include <string.h>
#define __user
struct floppy_raw_cmd { unsigned int flags; void __user *data; char *kernel_data; /* location of data buffer in the kernel */ struct floppy_raw_cmd *next; /* used for chaining of raw cmd's * within the kernel */ long length; /* in: length of dma transfer. out: remaining bytes */ long phys_length; /* physical length, if different from dma length */ int buffer_length; /* length of allocated buffer */
unsigned char rate;
#define FD_RAW_CMD_SIZE 16 #define FD_RAW_REPLY_SIZE 16 #define FD_RAW_CMD_FULLSIZE (FD_RAW_CMD_SIZE + 1 + FD_RAW_REPLY_SIZE)
unsigned char cmd_count; union { struct { unsigned char cmd[FD_RAW_CMD_SIZE]; unsigned char reply_count; unsigned char reply[FD_RAW_REPLY_SIZE]; }; unsigned char fullcmd[FD_RAW_CMD_FULLSIZE]; }; int track; int resultcode;
int reserved1; int reserved2; };
void __attribute__((noinline)) stack_alloc() { struct floppy_raw_cmd stack; memset(&stack, 0xff, sizeof(struct floppy_raw_cmd)); asm volatile ("" ::: "memory"); }
int __attribute__((noinline)) test(struct floppy_raw_cmd *ptr) { struct floppy_raw_cmd cmd = *ptr; int i;
for (i = 0; i < sizeof(struct floppy_raw_cmd); ++i) { if (((char *)&cmd)[i]) { printf("leak[%d]\n", i); return i; } }
return 0; }
int main(int argc, char *argv[]) { struct floppy_raw_cmd zero;
memset(&zero, 0, sizeof(struct floppy_raw_cmd)); // For selfcheck uncomment: // zero.resultcode = 1; stack_alloc(); return test(&zero); }
Next, I've prepared containers with gcc 4.8 5 6 7 8 9 10 versions with this tool (https://github.com/a13xp0p0v/kernel-build-containers).
And checked for leaks on x86_64 with the script test.sh $ cat test.sh #!/bin/bash
for i in 4.8 5 6 7 8 9 10 do ./run_container.sh gcc-$i $(pwd)/src $(pwd)/out bash -c 'gcc test.c; ./a.out' ./run_container.sh gcc-$i $(pwd)/src $(pwd)/out bash -c 'gcc -O2 test.c; ./a.out' ./run_container.sh gcc-$i $(pwd)/src $(pwd)/out bash -c 'gcc -O3 test.c; ./a.out' done
No leaks reported. Is it really possible this this kind of init, i.e. cmd = *ptr?
https://lwn.net/Articles/417989/ (December 1, 2010). GCC 4.9.4 released [2016-08-03] Maybe this behavior changed.
https://www.nccgroup.com/us/about-us/newsroom-and-events/blog/2019/october/padding-the-struct-how-a-compiler-optimization-can-disclose-stack-memory/ Reports for >= 4.7, < 8.0 version. But I can't find a word about this kind of inits: cmd = *ptr.
Thanks, Denis
| |