Introduction

A few days ago, linux upstream fixed a stack-based overflow vulnerability is CVE-2017-7187, that impact Linux Kernel 4.10.4 or more, allows local user to cause a denial of service or possibly have unspecified other.

In this paper through code view show you to analysis the vulnerability. Good luck! :)

Vulnerability Analysis

It is an interesting vulnerability, that application program can send a large number to set next command size in user space.

At first code path,

ioctl() -> sg_ioctl() -> [return]

next review the source code to see detail information.

// drivers/scsi/sg.c: sg_ioctl
static long
sg_ioctl(struct file *filp, unsigned int cmd_in, unsigned long arg)
{
        void __user *p = (void __user *)arg;[1]
        int __user *ip = p;[2]
        int result, val, read_only;
        Sg_device *sdp;
        Sg_fd *sfp;
        Sg_request *srp;
        unsigned long iflags;

        if ((!(sfp = (Sg_fd *) filp->private_data)) || (!(sdp = sfp->parentdp)))
                return -ENXIO;
	    [...]

        switch (cmd_in) {
	    [...]  
        case SG_NEXT_CMD_LEN:
                result = get_user(val, ip);[3]
                if (result)
                        return result;
                sfp->next_cmd_len = (val > 0) ? val : 0;[4] // Vulnerable, do not check.
                return 0;   
        [...]
        }
[...]
}                  

These show we some useful informations. The value of arg parameter comes from user space, it is unsigned long, and we can control its. After ioctl(), pointer p and ip points to arg[1][2], subsequently, called get_user() function in the switch case to switch SG_NEXT_CMD_LEN command, that copy a number from user space[3]. get_uesr() function looks like copy_from_user() function, but it supports simple types only like char and int, but not large data types such as structures or arrays. In the end, sfp->next_cmd_len set a value from val, if val greater than 0, the value is val, but not, is 0. OK, in the [4], dose not check the maximum of val, we can set a large number in user space.

The next code path.

write()/writev() -> [interrupt] -> sys_write() -> vfs_write()-> __vfs_write() -> sg_write()-> __copy_from_user()

The code path has a lovely function is __copy_from_user(), which called by sg_write() function. Let’s continue.

// drivers/scsi/sg.c: sg_wirite
static ssize_t
sg_write(struct file *filp, const char __user *buf, size_t count, loff_t * ppos)
{
        int mxsize, cmd_size, k;
        int input_size, blocking;
        unsigned char opcode;
        Sg_device *sdp;
        Sg_fd *sfp;
        Sg_request *srp;
        struct sg_header old_hdr;
        sg_io_hdr_t *hp;
        unsigned char cmnd[SG_MAX_CDB_SIZE];
[...]
        if (sfp->next_cmd_len > 0) {
                cmd_size = sfp->next_cmd_len; [2]
                sfp->next_cmd_len = 0;  /* reset so only this write() effected */ [3]
        } else {
              [...]
        }
        [...]
        if (__copy_from_user(cmnd, buf, cmd_size))[1] // stack overflow
                return -EFAULT;        
[...]
}               

Similarly, we can control the buf parameter. In the [1], __copy_from_user() called, that copy a cmd_size size buffer to cmmd. In first code path, the cmd_size was set, store in sfp->next_cmd_len variable, and the value assign to cmd_size[2].

cmnd size is SG_MAX_CDB_SIZE, defined as below.

// drivers/scsi/sg.c
#define SG_MAX_CDB_SIZE 252

So, we assign a large buffer in user space, that greater than 252 by invoking ioctl(). It’s overflow.

Therefore, trigger the vulnerability has two steps as below.

  1. Using SG_NEXT_CMD_LEN command assign a number greater than 252 by ioctl(), that set next command size make second code path can assign a large buffer.
  2. Called write()/writev() function.

In the [3], this comment tell we which reset so only this write() effected, so use writev() function better, maybe.

ssize_t writev(int fd, const struct iovec *iov, int iovcnt);

Writing trigger code steps:

  1. open /dev/sg0
  2. ioctl(fd, SG_NEXT_CMD_LEN, 1024)
  3. writev(fd, &buffer, 1024)

Patch Analysis

diff --git a/drivers/scsi/sg.c b/drivers/scsi/sg.c
index e831e01..849ff81 100644
--- a/drivers/scsi/sg.c
+++ b/drivers/scsi/sg.c
@@ -996,6 +996,8 @@ sg_ioctl(struct file *filp, unsigned int cmd_in, unsigned long arg)
 		result = get_user(val, ip);
 		if (result)
 			return result;
+		if (val > SG_MAX_CDB_SIZE)
+			return -ENOMEM;
 		sfp->next_cmd_len = (val > 0) ? val : 0;
 		return 0;
 	case SG_GET_VERSION_NUM:

It has a so easy way to fix it, that before assign sfp->next_cmd_len to check maixnum of val.

Reference

  1. https://www.kernel.org
  2. https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2017-7187
  3. https://git.kernel.org/pub/scm/linux/kernel/git/mkp/scsi.git/commit/?h=4.11/scsi-fixes&id=bf33f87dd04c371ea33feb821b60d63d754e3124
  4. https://gist.github.com/dvyukov/48ad14e84de45b0be92b7f0eda20ff1b#file-gistfile1-txt-L86
  5. Linux Device Drivers, Third Edition