[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAE2F3rCcEC=fMd+_gxwksTW=07ZzAt-vA5SO3w8AdAXGy1OVDg@mail.gmail.com>
Date: Tue, 17 Oct 2023 14:56:33 -0700
From: Daniel Mentz <danielmentz@...gle.com>
To: Bart Van Assche <bvanassche@....org>,
Avri Altman <avri.altman@....com>
Cc: "linux-scsi@...r.kernel.org" <linux-scsi@...r.kernel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
Tomas Winkler <tomas.winkler@...el.com>,
"Martin K . Petersen" <martin.petersen@...cle.com>,
Mars Cheng <marscheng@...gle.com>,
Yen-lin Lai <yenlinlai@...gle.com>
Subject: Re: [PATCH] scsi: ufs: Leave space for '\0' in utf8 desc string
On Tue, Oct 17, 2023 at 12:33 PM Bart Van Assche <bvanassche@....org> wrote:
>
> On 10/17/23 12:20, Avri Altman wrote:
> >> Fixes: 4b828fe156a6 ("scsi: ufs: revamp string descriptor reading")
> > I think this code goes back to commit b573d484e4ff (scsi: ufs: add support to read device and string descriptors)
>
> Hmm ... it seems to me that there was no buffer overflow in commit
> b573d484e4ff but that the buffer overflow was introduced by commit
> 4b828fe156a6?
Thank you for the review Avri.
To me, it appears as if those two commits had different issues:
commit b573d484e4ff ("scsi: ufs: add support to read device and string
descriptors") failed to reliably NULL terminate the output string (in
the case where ascii_len == size - QUERY_DESC_HDR_SIZE).
commit 4b828fe156a6 ("scsi: ufs: revamp string descriptor reading")
potentially performs an out-of-bounds array access while NULL
terminating the output string.
I would argue that the proposed fix wouldn't even fix the former and
older commit b573d484e4ff, because that commit might have required
more fixes like using kzalloc instead of kmalloc.
I find that the newer commit 4b828fe156a6 did enough of refactoring
for it to be considered the commit that needs this fix.
Powered by blists - more mailing lists