Re: [syzbot] [xfs?] UBSAN: array-index-out-of-bounds in xfs_attr3_leaf_add_work

[Dave Chinner <david@fromorbit.com>]

On Sat, Jun 17, 2023 at 04:22:59AM -0700, syzbot wrote:
> Hello,
> 
> syzbot found the following issue on:
> 
> HEAD commit:    1f6ce8392d6f Add linux-next specific files for 20230613
> git tree:       linux-next
> console+strace: https://syzkaller.appspot.com/x/log.txt?x=14e629dd280000
> kernel config:  https://syzkaller.appspot.com/x/.config?x=d103d5f9125e9fe9
> dashboard link: https://syzkaller.appspot.com/bug?extid=510dcbdc6befa1e6b2f6
> compiler:       gcc (Debian 10.2.1-6) 10.2.1 20210110, GNU ld (GNU Binutils for Debian) 2.35.2
> syz repro:      https://syzkaller.appspot.com/x/repro.syz?x=139d8d2d280000
> C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=11b371f1280000
> 
> Downloadable assets:
> disk image: https://storage.googleapis.com/syzbot-assets/2d9bf45aeae9/disk-1f6ce839.raw.xz
> vmlinux: https://storage.googleapis.com/syzbot-assets/e0b03ef83e17/vmlinux-1f6ce839.xz
> kernel image: https://storage.googleapis.com/syzbot-assets/b6c21a24174d/bzImage-1f6ce839.xz
> mounted in repro: https://storage.googleapis.com/syzbot-assets/65eca6891c21/mount_0.gz
> 
> IMPORTANT: if you fix the issue, please add the following tag to the commit:
> Reported-by: syzbot+510dcbdc6befa1e6b2f6@syzkaller.appspotmail.com
> 
> XFS (loop0): Mounting V4 Filesystem 5e6273b8-2167-42bb-911b-418aa14a1261
> XFS (loop0): Ending clean mount
> xfs filesystem being mounted at /root/file0 supports timestamps until 2038-01-19 (0x7fffffff)
> ================================================================================
> UBSAN: array-index-out-of-bounds in fs/xfs/libxfs/xfs_attr_leaf.c:1560:3
> index 14 is out of range for type '__u8 [1]'
> CPU: 1 PID: 5021 Comm: syz-executor198 Not tainted 6.4.0-rc6-next-20230613-syzkaller #0
> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 05/25/2023
> Call Trace:
>  <TASK>
>  __dump_stack lib/dump_stack.c:88 [inline]
>  dump_stack_lvl+0x136/0x150 lib/dump_stack.c:106
>  ubsan_epilogue lib/ubsan.c:217 [inline]
>  __ubsan_handle_out_of_bounds+0xd5/0x140 lib/ubsan.c:348
>  xfs_attr3_leaf_add_work+0x1528/0x1730 fs/xfs/libxfs/xfs_attr_leaf.c:1560
>  xfs_attr3_leaf_add+0x750/0x880 fs/xfs/libxfs/xfs_attr_leaf.c:1438
>  xfs_attr_leaf_try_add+0x1b7/0x660 fs/xfs/libxfs/xfs_attr.c:1242
>  xfs_attr_leaf_addname fs/xfs/libxfs/xfs_attr.c:444 [inline]
>  xfs_attr_set_iter+0x16c4/0x2f90 fs/xfs/libxfs/xfs_attr.c:721
>  xfs_xattri_finish_update+0x3c/0x140 fs/xfs/xfs_attr_item.c:332

The on disk format for this field is defined as:

typedef struct xfs_attr_leaf_name_local {
        __be16  valuelen;               /* number of bytes in value */
        __u8    namelen;                /* length of name bytes */
        __u8    nameval[1];             /* name/value bytes */
} xfs_attr_leaf_name_local_t

If someone wants to do change the on-disk format definition to use
"kernel proper" flex arrays in both the kernel code and user space,
update all the documentation and do all the validation work that
on-disk format changes require for all XFS disk structures that are
defined this way, then we'll fix this.

But as it stands, these structures have been defined this way for 25
years and the code accessing them has been around for just as long.
The code is not broken and it does not need fixing. We have way more
important things to be doing that fiddling with on disk format
definitions and long standing, working code just to shut up UBSAN
and/or syzbot.

WONTFIX, NOTABUG.

-Dave.
-- 
Dave Chinner
david@fromorbit.com