lkml.org 
[lkml]   [2021]   [May]   [3]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [syzbot] UBSAN: array-index-out-of-bounds in udf_statfs
On Sun 02-05-21 20:03:46, Randy Dunlap wrote:
> Hi all--
>
> On 4/30/21 12:28 PM, syzbot wrote:
> > Hello,
> >
> > syzbot found the following issue on:
> >
> > HEAD commit: e77a830c Merge branch 'akpm' (patches from Andrew)
> > git tree: upstream
> > console output: https://syzkaller.appspot.com/x/log.txt?x=14c63e6dd00000
> > kernel config: https://syzkaller.appspot.com/x/.config?x=c0a6882014fd3d45
> > dashboard link: https://syzkaller.appspot.com/bug?extid=7fbfe5fed73ebb675748
> > compiler: Debian clang version 11.0.1-2
> > syz repro: https://syzkaller.appspot.com/x/repro.syz?x=17612825d00000
> > C reproducer: https://syzkaller.appspot.com/x/repro.c?x=132cb56dd00000
> >
> > IMPORTANT: if you fix the issue, please add the following tag to the commit:
> > Reported-by: syzbot+7fbfe5fed73ebb675748@syzkaller.appspotmail.com
> >
> > loop0: detected capacity change from 0 to 3974
> > UDF-fs: INFO Mounting volume 'LinuxUDF', timestamp 2020/09/19 18:44 (1000)
> > ================================================================================
> > UBSAN: array-index-out-of-bounds in fs/udf/super.c:2524:12
> > index 0 is out of range for type '__le32 [0]'
>
>
> Is this just due to (from fs/udf/ecma_167.h) the "[0]" struct items?
> Do they need to be "[]" instead? Will that satisfy USBAN?
>
>
> /* Logical Volume Integrity Descriptor (ECMA 167r3 3/10.10) */
> struct logicalVolIntegrityDesc {
> struct tag descTag;
> struct timestamp recordingDateAndTime;
> __le32 integrityType;
> struct extent_ad nextIntegrityExt;
> uint8_t logicalVolContentsUse[32];
> __le32 numOfPartitions;
> __le32 lengthOfImpUse;
> __le32 freeSpaceTable[0]; // <<<<<<<<<<<<<<<<
> __le32 sizeTable[0]; // <<<<<<<<<<<<<<<<<<<
> uint8_t impUse[0]; // <<<<<<<<<<<<<<<<<<<<<<<
> } __packed;
>
>
> (I ask because I cannot reproduce the problem -- maybe a bad GCC
> version?)

Well, checks for numOfPartitions and lengthOfImpUse are certainly missing
as well so maliciously corrupted filesystem (we have checksums for random
corruptions) could certainly cause bad access. I'll fix that. You have a
valid point that [0] arrays could confuse the compiler as well and
certainly are not the suggested way of doing stuff like this these days.
I'll get rid of those as well.

Honza
--
Jan Kara <jack@suse.com>
SUSE Labs, CR

\
 
 \ /
  Last update: 2021-05-03 10:55    [W:0.053 / U:0.036 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site