On Tue, 25 Jul 2023 at 11:27, Tony Battersby <tonyb@xxxxxxxxxxxxxxx> wrote:
>
> Something that I just thought of: the old parser could also skip over
> NUL characters used as separators within the buffer that aren't at the
> end of the buffer, as in: "host\0id\0channel\0lun". If you want to
> continue to allow that unlikely usage, then my patch comparing p to the
> end pointer would work better.
Yeah, that would probably be better still. Ack on that.
That said, I just realized that *all* of this is completely
unnecessarily complicated. We allow up to a PAGE_SIZE, but you cannot
actually fill even *remotely* that much without using insane
zero-padding, and at that point you're not doing something useful,
you're trying to actively break something.
So the simple fix is to just limit the size of the buffer to slightly
less than PAGE_SIZE, and just pad more than one NUL character at the
end. Technically we're skipping four characters, and then we have the
last "real" NUL terminator, so 5 would be sufficient, but let's make
it easy for the compiler to just generate one single 64-bit store (or
two 32-bit ones) and clear 8 bytes.
IOW, we could do something *this* simple instead.
But I'm ok with your "track the end" version too.
Linus
drivers/scsi/scsi_proc.c | 7 ++-----
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/drivers/scsi/scsi_proc.c b/drivers/scsi/scsi_proc.c
index 4a6eb1741be0..da66d9260232 100644
--- a/drivers/scsi/scsi_proc.c
+++ b/drivers/scsi/scsi_proc.c
@@ -409,7 +409,7 @@ static ssize_t proc_scsi_write(struct file *file, const char __user *buf,
char *buffer, *p;
int err;
- if (!buf || length > PAGE_SIZE)
+ if (!buf || length > PAGE_SIZE-8)
return -EINVAL;
buffer = (char *)__get_free_page(GFP_KERNEL);
@@ -421,10 +421,7 @@ 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;
+ memset(buffer + length, 0, 8);
/*
* Usage: echo "scsi add-single-device 0 1 2 3" >/proc/scsi/scsi
