Skip to content

Commit

Permalink
scsi: core: Fix legacy /proc parsing buffer overflow
Browse files Browse the repository at this point in the history
commit 9426d3c upstream.

(lightly modified commit message mostly by Linus Torvalds)

The parsing code for /proc/scsi/scsi is disgusting and broken.  We should
have just used 'sscanf()' or something simple like that, but the logic may
actually predate our kernel sscanf library routine for all I know.  It
certainly predates both git and BK histories.

And we can't change it to be something sane like that now, because the
string matching at the start is done case-insensitively, and the separator
parsing between numbers isn't done at all, so *any* separator will work,
including a possible terminating NUL character.

This interface is root-only, and entirely for legacy use, so there is
absolutely no point in trying to tighten up the parsing.  Because any
separator has traditionally worked, it's entirely possible that people have
used random characters rather than the suggested space.

So don't bother to try to pretty it up, and let's just make a minimal patch
that can be back-ported and we can forget about this whole sorry thing for
another two decades.

Just make it at least not read past the end of the supplied data.

Link: https://lore.kernel.org/linux-scsi/b570f5fe-cb7c-863a-6ed9-f6774c219b88@cybernetics.com/
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Martin K Petersen <martin.petersen@oracle.com>
Cc: James Bottomley <jejb@linux.ibm.com>
Cc: Willy Tarreau <w@1wt.eu>
Cc: stable@kernel.org
Fixes: 1da177e ("Linux-2.6.12-rc2")
Signed-off-by: Tony Battersby <tonyb@cybernetics.com>
Signed-off-by: Martin K Petersen <martin.petersen@oracle.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
  • Loading branch information
abattersby authored and gregkh committed Aug 16, 2023
1 parent e044b1b commit 2083176
Showing 1 changed file with 17 additions and 13 deletions.
30 changes: 17 additions & 13 deletions drivers/scsi/scsi_proc.c
Expand Up @@ -406,7 +406,7 @@ static ssize_t proc_scsi_write(struct file *file, const char __user *buf,
size_t length, loff_t *ppos)
{
int host, channel, id, lun;
char *buffer, *p;
char *buffer, *end, *p;
int err;

if (!buf || length > PAGE_SIZE)
Expand All @@ -421,10 +421,14 @@ static ssize_t proc_scsi_write(struct file *file, const char __user *buf,
goto out;

err = -EINVAL;
if (length < PAGE_SIZE)
buffer[length] = '\0';
else if (buffer[PAGE_SIZE-1])
goto out;
if (length < PAGE_SIZE) {
end = buffer + length;
*end = '\0';
} else {
end = buffer + PAGE_SIZE - 1;
if (*end)
goto out;
}

/*
* Usage: echo "scsi add-single-device 0 1 2 3" >/proc/scsi/scsi
Expand All @@ -433,10 +437,10 @@ static ssize_t proc_scsi_write(struct file *file, const char __user *buf,
if (!strncmp("scsi add-single-device", buffer, 22)) {
p = buffer + 23;

host = simple_strtoul(p, &p, 0);
channel = simple_strtoul(p + 1, &p, 0);
id = simple_strtoul(p + 1, &p, 0);
lun = simple_strtoul(p + 1, &p, 0);
host = (p < end) ? simple_strtoul(p, &p, 0) : 0;
channel = (p + 1 < end) ? simple_strtoul(p + 1, &p, 0) : 0;
id = (p + 1 < end) ? simple_strtoul(p + 1, &p, 0) : 0;
lun = (p + 1 < end) ? simple_strtoul(p + 1, &p, 0) : 0;

err = scsi_add_single_device(host, channel, id, lun);

Expand All @@ -447,10 +451,10 @@ static ssize_t proc_scsi_write(struct file *file, const char __user *buf,
} else if (!strncmp("scsi remove-single-device", buffer, 25)) {
p = buffer + 26;

host = simple_strtoul(p, &p, 0);
channel = simple_strtoul(p + 1, &p, 0);
id = simple_strtoul(p + 1, &p, 0);
lun = simple_strtoul(p + 1, &p, 0);
host = (p < end) ? simple_strtoul(p, &p, 0) : 0;
channel = (p + 1 < end) ? simple_strtoul(p + 1, &p, 0) : 0;
id = (p + 1 < end) ? simple_strtoul(p + 1, &p, 0) : 0;
lun = (p + 1 < end) ? simple_strtoul(p + 1, &p, 0) : 0;

err = scsi_remove_single_device(host, channel, id, lun);
}
Expand Down

0 comments on commit 2083176

Please sign in to comment.