linux-kernel - Re: SCSI: fix parsing of /proc/scsci/scsi file

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CAHk-=whXgrO8dEYAQX6E8ffZXhzJ_+VPZyx1TcYpdRhQHD=L0A@mail.gmail.com>
Date:   Tue, 25 Jul 2023 11:41:15 -0700
From:   Linus Torvalds <torvalds@...ux-foundation.org>
To:     Tony Battersby <tonyb@...ernetics.com>
Cc:     Martin K Petersen <martin.petersen@...cle.com>,
        James Bottomley <jejb@...ux.ibm.com>, Willy Tarreau <w@....eu>,
        linux-scsi <linux-scsi@...r.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: SCSI: fix parsing of /proc/scsci/scsi file

On Tue, 25 Jul 2023 at 11:27, Tony Battersby <tonyb@...ernetics.com> 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

View attachment "patch.diff" of type "text/x-patch" (852 bytes)