cc'ing linux-xfs, since I'm sure this has come up there and bcachefs and
xfs verify and fsck are structured similararly at a very high level -
I'd like to get their input.

On Sun, Jul 14, 2024 at 09:34:01PM GMT, Mickaël Salaün wrote:

On Fri, Jul 12, 2024 at 10:55:11AM -0400, Paul Moore wrote:

quoted

On Thu, Jul 11, 2024 at 5:53 PM syzbot
[off-list ref] wrote:

quoted

Hello,

syzbot found the following issue on:

HEAD commit:    8a03d70c27fc Merge remote-tracking branch 'tglx/devmsi-arm..
git tree:       git://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux.git for-kernelci
console output: https://syzkaller.appspot.com/x/log.txt?x=174b0e6e980000
kernel config:  https://syzkaller.appspot.com/x/.config?x=15349546db652fd3
dashboard link: https://syzkaller.appspot.com/bug?extid=34b68f850391452207df
compiler:       Debian clang version 15.0.6, GNU ld (GNU Binutils for Debian) 2.40
userspace arch: arm64
syz repro:      https://syzkaller.appspot.com/x/repro.syz?x=13cd1b69980000
C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=12667fd1980000

Downloadable assets:
disk image: https://storage.googleapis.com/syzbot-assets/efb354033e75/disk-8a03d70c.raw.xz
vmlinux: https://storage.googleapis.com/syzbot-assets/c747c205d094/vmlinux-8a03d70c.xz
kernel image: https://storage.googleapis.com/syzbot-assets/5641f4fb7265/Image-8a03d70c.gz.xz
mounted in repro: https://storage.googleapis.com/syzbot-assets/4e4d1faacdef/mount_0.gz

IMPORTANT: if you fix the issue, please add the following tag to the commit:
Reported-by: syzbot+34b68f850391452207df@syzkaller.appspotmail.com

bcachefs (loop0): resume_logged_ops... done
bcachefs (loop0): delete_dead_inodes... done
bcachefs (loop0): done starting filesystem
------------[ cut here ]------------
WARNING: CPU: 0 PID: 6284 at security/landlock/fs.c:971 current_check_refer_path+0x4e0/0xaa8 security/landlock/fs.c:1132

I'll let Mickaël answer this for certain, but based on a quick look it
appears that the fs object being moved has a umode_t that Landlock is
not setup to handle?

syzbot found an issue with bcachefs: in some cases umode_t is invalid (i.e.
a weird file).

Kend, Brian, you'll find the incorrect filesystem with syzbot's report.
Could you please investigate the issue?

Here is the content of the file system:
# losetup --find --show mount_0
/dev/loop0
# mount /dev/loop0 /mnt/
# ls -la /mnt/
ls: cannot access '/mnt/file2': No such file or directory
ls: cannot access '/mnt/file3': No such file or directory
total 24
drwxr-xr-x 4 root root   0 May  2 20:21 .
drwxr-xr-x 1 root root 130 Oct 31  2023 ..
drwxr-xr-x 2 root root   0 May  2 20:21 file0
?rwxr-xr-x 1 root root  10 May  2 20:21 file1
-????????? ? ?    ?      ?            ? file2
-????????? ? ?    ?      ?            ? file3
-rwxr-xr-x 1 root root 100 May  2 20:21 file.cold
drwx------ 2 root root   0 May  2 20:21 lost+found
# stat /mnt/file1
  File: /mnt/file1
  Size: 10              Blocks: 8          IO Block: 4096   weird file
Device: 7,0     Inode: 1073741824  Links: 1
Access: (0755/?rwxr-xr-x)  Uid: (    0/    root)   Gid: (    0/    root)
Access: 2024-05-02 20:21:07.747039697 +0000
Modify: 2024-05-02 20:21:07.747039697 +0000
Change: 2024-05-02 20:21:07.747039697 +0000
 Birth: 2024-05-02 20:21:07.747039697 +0000

Ok, this is an interesting one.

So we don't seem to be checking for invwalid i_mode at all - that's a bug.

But if we don't want to be exposing invalid i_modes at all, that's
tricky, since we (currently) can only repair when running fsck. "This is
invalid and we never want to expose this" checks are done in bkey
.invalid methods, and those can only cause the key to be deleted - we
can't run complex repair in e.g. btree node read, and that's what would
be required here (e.g. checking the extents and dirents btrees to guess
if this should be a regular file or a directory).

Long term I plan on running our existing fsck checks (including repair)
in our normal runtime paths - whenever we're looking at one or more keys
and there's a fsck check we can run, just run it.

I wasn't planning on doing that for awhile, because I'm waiting on
getting comprehensive filesystem error injection merged so we can make
sure those repair paths are all well tested before running them
automatically like that, but if this is a security issue perhaps as a
special case we should do that now.

Thoughts?