[Bug 214711] New: Memory leakage from kernel to user space

[Bug 214711] New: Memory leakage from kernel to user space

[bugzilla-daemon]

https://bugzilla.kernel.org/show_bug.cgi?id=214711

            Bug ID: 214711
           Summary: Memory leakage from kernel to user space
           Product: SCSI Drivers
           Version: 2.5
    Kernel Version: 5.15-rc5
          Hardware: All
                OS: Linux
              Tree: Mainline
            Status: NEW
          Severity: normal
          Priority: P1
         Component: Other
          Assignee: scsi_drivers-other@kernel-bugs.osdl.org
          Reporter: bao00065@umn.edu
        Regression: No

Hi Maintainer,
I just found an uninitialized value use bug that causes memory leakage from
kernel to user space. Here are the details:

Vulnerable function is in /drivers/scsi/scsi_ioctl.c



static int scsi_put_cdrom_generic_arg(const struct cdrom_generic_command *cgc,
                                      void __user *arg)
{
#ifdef CONFIG_COMPAT
        if (in_compat_syscall()) {
                struct compat_cdrom_generic_command cgc32 = {
                        .buffer         = (uintptr_t)(cgc->buffer),
                        .buflen         = cgc->buflen,
                        .stat           = cgc->stat,
                        .sense          = (uintptr_t)(cgc->sense),
                        .data_direction = cgc->data_direction,
                        .quiet          = cgc->quiet,
                        .timeout        = cgc->timeout,
                        .unused         = (uintptr_t)(cgc->unused),
                };
                memcpy(&cgc32.cmd, &cgc->cmd, CDROM_PACKET_SIZE);

                if (copy_to_user(arg, &cgc32, sizeof(cgc32))) 
                        return -EFAULT;

                return 0;
        }
#endif
        if (copy_to_user(arg, cgc, sizeof(*cgc)))
                return -EFAULT;

        return 0;
}

The issue is, struct cgc32 is partially initialized since pad[3] are not
initialized. Then this struct is passed to copy_to_user, and 3 bytes are leaked
from kernel space to userspace. 


The struct is declared here:
struct compat_cdrom_generic_command {
        unsigned char   cmd[CDROM_PACKET_SIZE];
        compat_caddr_t  buffer;
        compat_uint_t   buflen;
        compat_int_t    stat;
        compat_caddr_t  sense;
        unsigned char   data_direction;
        unsigned char   pad[3];
        compat_int_t    quiet;
        compat_int_t    timeout;
        compat_caddr_t  unused;
};
#endif

-- 
You may reply to this email to add a comment.

You are receiving this mail because:
You are watching the assignee of the bug.

[Bug 214711] Memory leakage from kernel to user space in scsi_ioctl.c

[bugzilla-daemon]

https://bugzilla.kernel.org/show_bug.cgi?id=214711

Andrew Bao (bao00065@umn.edu) changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|Memory leakage from kernel  |Memory leakage from kernel
                   |to user space               |to user space in
                   |                            |scsi_ioctl.c

-- 
You may reply to this email to add a comment.

You are receiving this mail because:
You are watching the assignee of the bug.

[Bug 214711] Memory leakage from kernel to user space in scsi_ioctl.c

[bugzilla-daemon]

https://bugzilla.kernel.org/show_bug.cgi?id=214711

Bart Van Assche (bvanassche@acm.org) changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |bvanassche@acm.org

--- Comment #1 from Bart Van Assche (bvanassche@acm.org) ---
Isn't this called an information leak instead of a memory leak?

Additionally, my understanding of the C standard is that a compiler is required
to zero-initialize members that have not been mentioned in an initializer list.
From the ANSI C 202x draft: "The initialization shall occur in initializer list
order, each initializer provided for a particular subobject overriding any
previously listed initializer for the same subobject; all subobjects that are
not
initialized explicitly shall be initialized implicitly the same as objects that
have static storage duration."

-- 
You may reply to this email to add a comment.

You are receiving this mail because:
You are watching the assignee of the bug.

[Bug 214711] Information leakage from kernel to user space in scsi_ioctl.c

[bugzilla-daemon]

https://bugzilla.kernel.org/show_bug.cgi?id=214711

Andrew Bao (bao00065@umn.edu) changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|Memory leakage from kernel  |Information leakage from
                   |to user space in            |kernel to user space in
                   |scsi_ioctl.c                |scsi_ioctl.c

-- 
You may reply to this email to add a comment.

You are receiving this mail because:
You are watching the assignee of the bug.

[Bug 214711] Information leak from kernel to user space in scsi_ioctl.c

[bugzilla-daemon]

https://bugzilla.kernel.org/show_bug.cgi?id=214711

Andrew Bao (bao00065@umn.edu) changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|Information leakage from    |Information leak from
                   |kernel to user space in     |kernel to user space in
                   |scsi_ioctl.c                |scsi_ioctl.c

-- 
You may reply to this email to add a comment.

You are receiving this mail because:
You are watching the assignee of the bug.

[Bug 214711] Information leak from kernel to user space in scsi_ioctl.c

[bugzilla-daemon]

https://bugzilla.kernel.org/show_bug.cgi?id=214711

--- Comment #2 from Andrew Bao (bao00065@umn.edu) ---
Hi Bart,
Yes, It is an information leak.

"my understanding of the C standard is that a compiler is required to
zero-initialize members that have not been mentioned in an initializer list.
From the ANSI C 202x d"raft: "The initialization shall occur in initializer
list order, each initializer provided for a particular subobject overriding any
previously listed initializer for the same subobject; all subobjects that are
not
initialized explicitly shall be initialized implicitly the same as objects that
have static storage duration."

I am wondering in what condition the compiler will zero-initialize the field in
a struct. And what is the initializer in the context? Let say we have a struct
foo:

struct foo{
       int  a;
       int b;
       int c;
};
method 1:

struct foo f; 
f.a = 1;
f.b = 2;

In method 1, will the compiler zero-initialize the field f.c?

method 2:
struct foo f = {
                        .a      =   1
                        .b      =   2

                };
In method 2, will the compiler zero-initialize the field f.c?


By the way,
struct compat_cdrom_generic_command {
        unsigned char   cmd[CDROM_PACKET_SIZE];
        compat_caddr_t  buffer;
        compat_uint_t   buflen;
        compat_int_t    stat;
        compat_caddr_t  sense;
        unsigned char   data_direction;
        unsigned char   pad[3];
        compat_int_t    quiet;
        compat_int_t    timeout;
        compat_caddr_t  unused;
};
If this struct does not declare unsigned char pad[3] in order to fill with
padding, will the compiler zero-initialize 3 bytes holes for this struct?

-- 
You may reply to this email to add a comment.

You are receiving this mail because:
You are watching the assignee of the bug.

[Bug 214711] Information leak from kernel to user space in scsi_ioctl.c

[bugzilla-daemon]

https://bugzilla.kernel.org/show_bug.cgi?id=214711

--- Comment #3 from Andrew Bao (bao00065@umn.edu) ---
Hi Bart,
Yes, It is an information leak.

"my understanding of the C standard is that a compiler is required to
zero-initialize members that have not been mentioned in an initializer list.
From the ANSI C 202x d"raft: "The initialization shall occur in initializer
list order, each initializer provided for a particular subobject overriding any
previously listed initializer for the same subobject; all subobjects that are
not
initialized explicitly shall be initialized implicitly the same as objects that
have static storage duration."

I am wondering in what condition the compiler will zero-initialize the field in
a struct. And what is the initializer in the context? Let say we have a struct
foo:

struct foo{
       int  a;
       int b;
       int c;
};
method 1:

struct foo f; 
f.a = 1;
f.b = 2;

In method 1, will the compiler zero-initialize the field f.c?

method 2:
struct foo f = {
                        .a      =   1
                        .b      =   2

                };
In method 2, will the compiler zero-initialize the field f.c?


By the way,
struct compat_cdrom_generic_command {
        unsigned char   cmd[CDROM_PACKET_SIZE];
        compat_caddr_t  buffer;
        compat_uint_t   buflen;
        compat_int_t    stat;
        compat_caddr_t  sense;
        unsigned char   data_direction;
        unsigned char   pad[3];
        compat_int_t    quiet;
        compat_int_t    timeout;
        compat_caddr_t  unused;
};
If this struct does not declare unsigned char pad[3] in order to fill with
padding, will the compiler zero-initialize 3 bytes holes for this struct?

-- 
You may reply to this email to add a comment.

You are receiving this mail because:
You are watching the assignee of the bug.

[Bug 214711] Information leak from kernel to user space in scsi_ioctl.c

[bugzilla-daemon]

https://bugzilla.kernel.org/show_bug.cgi?id=214711

--- Comment #4 from Bart Van Assche (bvanassche@acm.org) ---
C and C++ compilers always initialize all named data members of a data
structure in case of aggregate initialization. See also
https://stackoverflow.com/questions/10828294/c-and-c-partial-initialization-of-automatic-structure.
However, whether or not unnamed padding bytes and bits are initialized depends
on the language standard supported by the compiler. See e.g.
https://gustedt.wordpress.com/2012/10/24/c11-defects-initialization-of-padding/

-- 
You may reply to this email to add a comment.

You are receiving this mail because:
You are watching the assignee of the bug.

[Bug 214711] Information leak from kernel to user space in scsi_ioctl.c

[bugzilla-daemon]

https://bugzilla.kernel.org/show_bug.cgi?id=214711

--- Comment #5 from Andrew Bao (bao00065@umn.edu) ---
Thank you

-- 
You may reply to this email to add a comment.

You are receiving this mail because:
You are watching the assignee of the bug.