patches.lists.linux.dev archive mirror
 help / color / mirror / Atom feed
From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: stable@vger.kernel.org
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	patches@lists.linux.dev,
	Linus Torvalds <torvalds@linux-foundation.org>,
	Martin K Petersen <martin.petersen@oracle.com>,
	James Bottomley <jejb@linux.ibm.com>, Willy Tarreau <w@1wt.eu>,
	stable@kernel.org, Tony Battersby <tonyb@cybernetics.com>
Subject: [PATCH 4.14 21/26] scsi: core: Fix legacy /proc parsing buffer overflow
Date: Sun, 13 Aug 2023 23:19:14 +0200	[thread overview]
Message-ID: <20230813211703.776460963@linuxfoundation.org> (raw)
In-Reply-To: <20230813211702.980427106@linuxfoundation.org>

From: Tony Battersby <tonyb@cybernetics.com>

commit 9426d3cef5000824e5f24f80ed5f42fb935f2488 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: 1da177e4c3f4 ("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>
---
 drivers/scsi/scsi_proc.c |   30 +++++++++++++++++-------------
 1 file changed, 17 insertions(+), 13 deletions(-)

--- a/drivers/scsi/scsi_proc.c
+++ b/drivers/scsi/scsi_proc.c
@@ -311,7 +311,7 @@ static ssize_t proc_scsi_write(struct fi
 			       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)
@@ -326,10 +326,14 @@ static ssize_t proc_scsi_write(struct fi
 		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
@@ -338,10 +342,10 @@ static ssize_t proc_scsi_write(struct fi
 	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);
 
@@ -352,10 +356,10 @@ static ssize_t proc_scsi_write(struct fi
 	} 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);
 	}



  parent reply	other threads:[~2023-08-13 21:21 UTC|newest]

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-08-13 21:18 [PATCH 4.14 00/26] 4.14.323-rc1 review Greg Kroah-Hartman
2023-08-13 21:18 ` [PATCH 4.14 01/26] sparc: fix up arch_cpu_finalize_init() build breakage Greg Kroah-Hartman
2023-08-13 21:18 ` [PATCH 4.14 02/26] mmc: moxart: read scr register without changing byte order Greg Kroah-Hartman
2023-08-13 21:18 ` [PATCH 4.14 03/26] ipv6: adjust ndisc_is_useropt() to also return true for PIO Greg Kroah-Hartman
2023-08-13 21:18 ` [PATCH 4.14 04/26] dmaengine: pl330: Return DMA_PAUSED when transaction is paused Greg Kroah-Hartman
2023-08-13 21:18 ` [PATCH 4.14 05/26] radix tree test suite: fix incorrect allocation size for pthreads Greg Kroah-Hartman
2023-08-13 21:18 ` [PATCH 4.14 06/26] nilfs2: fix use-after-free of nilfs_root in dirtying inodes via iput Greg Kroah-Hartman
2023-08-13 21:19 ` [PATCH 4.14 07/26] test_firmware: return ENOMEM instead of ENOSPC on failed memory allocation Greg Kroah-Hartman
2023-08-13 21:19 ` [PATCH 4.14 08/26] iio: cros_ec: Fix the allocation size for cros_ec_command Greg Kroah-Hartman
2023-08-13 21:19 ` [PATCH 4.14 09/26] usb-storage: alauda: Fix uninit-value in alauda_check_media() Greg Kroah-Hartman
2023-08-13 21:19 ` [PATCH 4.14 10/26] usb: dwc3: Properly handle processing of pending events Greg Kroah-Hartman
2023-08-13 21:19 ` [PATCH 4.14 11/26] x86/mm: Fix VDSO and VVAR placement on 5-level paging machines Greg Kroah-Hartman
2023-08-13 21:19 ` [PATCH 4.14 12/26] x86: Move gds_ucode_mitigated() declaration to header Greg Kroah-Hartman
2023-08-13 21:19 ` [PATCH 4.14 13/26] drm/nouveau/disp: Revert a NULL check inside nouveau_connector_get_modes Greg Kroah-Hartman
2023-08-13 21:19 ` [PATCH 4.14 14/26] net/packet: annotate data-races around tp->status Greg Kroah-Hartman
2023-08-13 21:19 ` [PATCH 4.14 15/26] bonding: Fix incorrect deletion of ETH_P_8021AD protocol vid from slaves Greg Kroah-Hartman
2023-08-13 21:19 ` [PATCH 4.14 16/26] dccp: fix data-race around dp->dccps_mss_cache Greg Kroah-Hartman
2023-08-13 21:19 ` [PATCH 4.14 17/26] drivers: net: prevent tun_build_skb() to exceed the packet size limit Greg Kroah-Hartman
2023-08-13 21:19 ` [PATCH 4.14 18/26] IB/hfi1: Fix possible panic during hotplug remove Greg Kroah-Hartman
2023-08-13 21:19 ` [PATCH 4.14 19/26] btrfs: dont stop integrity writeback too early Greg Kroah-Hartman
2023-08-13 21:19 ` [PATCH 4.14 20/26] netfilter: nf_tables: report use refcount overflow Greg Kroah-Hartman
2023-08-13 21:19 ` Greg Kroah-Hartman [this message]
2023-08-13 21:19 ` [PATCH 4.14 22/26] scsi: storvsc: Fix handling of virtual Fibre Channel timeouts Greg Kroah-Hartman
2023-08-13 21:19 ` [PATCH 4.14 23/26] scsi: 53c700: Check that command slot is not NULL Greg Kroah-Hartman
2023-08-13 21:19 ` [PATCH 4.14 24/26] scsi: snic: Fix possible memory leak if device_add() fails Greg Kroah-Hartman
2023-08-13 21:19 ` [PATCH 4.14 25/26] scsi: core: " Greg Kroah-Hartman
2023-08-13 21:19 ` [PATCH 4.14 26/26] alpha: remove __init annotation from exported page_is_ram() Greg Kroah-Hartman
2023-08-14 14:54 ` [PATCH 4.14 00/26] 4.14.323-rc1 review Thierry Reding
2023-08-14 18:22 ` Guenter Roeck
2023-08-15  1:49 ` Harshit Mogalapalli
2023-08-15  6:05 ` Daniel Díaz

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20230813211703.776460963@linuxfoundation.org \
    --to=gregkh@linuxfoundation.org \
    --cc=jejb@linux.ibm.com \
    --cc=martin.petersen@oracle.com \
    --cc=patches@lists.linux.dev \
    --cc=stable@kernel.org \
    --cc=stable@vger.kernel.org \
    --cc=tonyb@cybernetics.com \
    --cc=torvalds@linux-foundation.org \
    --cc=w@1wt.eu \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).