Re: [Linux-kernel-mentees] [PATCH v2] block/floppy: Prevent kernel-infoleak in raw_cmd_copyout()

[Denis Efremov]

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