On Fri, May 21, 2021 at 04:00:10PM -0400, Martin K. Petersen wrote: > > Hello Phillip! > > > In sd_revalidate_disk, if sdkp->media_present is set, then sdkp->capacity > > should not be zero. Therefore, jump to end of if block and skip remaining > > checks/calls. Fixes a KMSAN-found uninit-value bug reported by syzbot at: > > https://syzkaller.appspot.com/bug?id=197c8a3a2de61720a9b500ad485a7aba0065c6af > > The reported read of an uninitialized value is in scsi_mode_sense() > while inspecting a buffer returned from sending a MODE SENSE command to > the device. The buffer in question is memset() before executing the MODE > SENSE command. And we only look at the buffer contents if the MODE SENSE > operation was successful. > > As far as I can tell the only way to end up reading uninitialized data > is if the device successfully completes the command but fails to > transfer the data buffer. > > But maybe I'm missing something? > > -- > Martin K. Petersen Oracle Linux Engineering Dear Martin, Thank you for your feedback firstly, much appreciated. I may be misunderstanding this issue, but in my mind, if this issue is possible to trigger with a reproducer, then uninitialised data is being read? It occurred to me that a capacity of zero for a media which is present would make the following function calls/checks invalid, hence the motivation for my patch, as skipping all those checks with such a size prevents this bug. Another thing I noticed was that (unless I'm reading this wrong which is certainly possible) the buffer is never fully memset. It is allocated to be 512 bytes in size (as SD_BUF_SIZE) and yet sd_do_mode_sense/scsi_mode_sense is never called with a len param of this size but in fact much lower. Perhaps you're right though and my patch is not required? Certainly many KMSAN bugs are probably in areas where logic is not affected by the uninitialised access. Regards, Phil
