Re: [syzbot] UBSAN: array-index-out-of-bounds in udf_statfs

[Jan Kara <jack@suse.cz>]

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