From 574fe269f5aaf62a3ec862bf430adf91a20823bd Mon Sep 17 00:00:00 2001 From: Linus Torvalds Date: Tue, 25 Jul 2023 10:09:31 -0700 Subject: [PATCH] scsi: fix legacy /proc parsing buffer overflow 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 possioble 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 traverse the terminating NUL. Reported-by: Tony Battersby Link: https://lore.kernel.org/linux-scsi/b570f5fe-cb7c-863a-6ed9-f6774c219b88@cybernetics.com/ Cc: Martin K Petersen Cc: James Bottomley Cc: Willy Tarreau Cc: stable@kernel.org Signed-off-by: Linus Torvalds --- drivers/scsi/scsi_proc.c | 23 +++++++++++++---------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/drivers/scsi/scsi_proc.c b/drivers/scsi/scsi_proc.c index 4a6eb1741be0..8aa8208ceb7f 100644 --- a/drivers/scsi/scsi_proc.c +++ b/drivers/scsi/scsi_proc.c @@ -383,6 +383,9 @@ static int scsi_remove_single_device(uint host, uint channel, uint id, uint lun) return error; } +/* increment 'p', but not past the end */ +static inline char *next_p(char *p) { return p + !!*p; } + /** * proc_scsi_write - handle writes to /proc/scsi/scsi * @file: not used @@ -431,12 +434,12 @@ static ssize_t proc_scsi_write(struct file *file, const char __user *buf, * with "0 1 2 3" replaced by your "Host Channel Id Lun". */ if (!strncmp("scsi add-single-device", buffer, 22)) { - p = buffer + 23; + p = buffer + 22; - 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 = simple_strtoul(next_p(p), &p, 0); + channel = simple_strtoul(next_p(p), &p, 0); + id = simple_strtoul(next_p(p), &p, 0); + lun = simple_strtoul(next_p(p), &p, 0); err = scsi_add_single_device(host, channel, id, lun); @@ -445,12 +448,12 @@ static ssize_t proc_scsi_write(struct file *file, const char __user *buf, * with "0 1 2 3" replaced by your "Host Channel Id Lun". */ } else if (!strncmp("scsi remove-single-device", buffer, 25)) { - p = buffer + 26; + p = buffer + 25; - 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 = simple_strtoul(next_p(p), &p, 0); + channel = simple_strtoul(next_p(p), &p, 0); + id = simple_strtoul(next_p(p), &p, 0); + lun = simple_strtoul(next_p(p), &p, 0); err = scsi_remove_single_device(host, channel, id, lun); } -- 2.41.0.327.gaa9166bcc0