Received-SPF: pass (google.com: domain of linux-kernel+bounces-49848-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.48.161 as permitted sender) client-ip=147.75.48.161;
Precedence: bulk
MIME-Version: 1.0
References: <CABOYnLxOc6fTOEX4C_=0t4_+Qv9VrUTJUg4g6+JSAg+v34UaRg@mail.gmail.com>
In-Reply-To: <CABOYnLxOc6fTOEX4C_=0t4_+Qv9VrUTJUg4g6+JSAg+v34UaRg@mail.gmail.com>
From: Amir Goldstein <amir73il@gmail.com>
Date: Fri, 2 Feb 2024 14:34:46 +0200
Message-ID: <CAOQ4uxiqoP95=BpG2x=s2vr4mriBk1R76NgDFCuvXFRz=qr+Wg@mail.gmail.com>
Subject: Re: WARNING in ovl_copy_up_file
To: xingwei lee <xrivendell7@gmail.com>
Cc: linux-unionfs@vger.kernel.org, miklos@szeredi.hu, 
	linux-kernel@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Fri, Feb 2, 2024 at 1:09=E2=80=AFPM xingwei lee <xrivendell7@gmail.com> =
wrote:
>
> Hello I found a bug in latest upstream titled "WARNING in
> ovl_copy_up_file" with modified syzkaller and maybe is related with
> overlays, and I also reproduce it with repro.c and repro.txt
>
> If you fix this issue, please add the following tag to the commit:
> Reported-by: xingwei lee <xrivendell7@gmail.com>
>
> kernel: upstream 021533194476035883300d60fbb3136426ac8ea5
> kernel config: https://syzkaller.appspot.com/text?tag=3DKernelConfig&x=3D=
457249c250b93697
> with KASAN enabled
> compiler: gcc (Debian 12.2.0-14) 12.2.0
>
> ------------[ cut here ]------------
> WARNING: CPU: 3 PID: 77145 at fs/overlayfs/copy_up.c:239
> ovl_copy_up_file+0x6ca/0x6f0 fs/overlayfs/copy_up.c:332
> Modules linked in:
> CPU: 3 PID: 77145 Comm: syz-executor.0 Not tainted
> 6.8.0-rc1-00208-g6af191034c8f-dirty #3
> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS
> 1.16.2-debian-1.16.2-1 04/01/2014
> RIP: 0010:ovl_verify_area fs/overlayfs/copy_up.c:239 [inline]
> RIP: 0010:ovl_copy_up_file+0x6ca/0x6f0 fs/overlayfs/copy_up.c:332
> Code: ff e8 6a 62 47 ff 0f 0b 41 bc fb ff ff ff 48 8b 5c 24 18 e9 7c
> ff ff ff 31 ff e8 51 62 47 ff 0f 0b eb 14 31 ff e8 46 62 47 ff <0f> 0b
> eb 09 31 ff e8 3b 62 47 ff 0f 0b 41 bc fb ff ff ff e9 1d fb
> RSP: 0018:ffff88804417f440 EFLAGS: 00010246
> RAX: ffffffff81fb8c4a RBX: 0000000000800000 RCX: ffffc9000c04c000
> RDX: ffffffff81fb8c4a RSI: 0000000000000000 RDI: 0000000000000000
> RBP: ffff88804417f570 R08: 000000000000a4dd R09: 000000000000a4de
> R10: ffffed102063f949 R11: ffffed102063f949 R12: ffff8881031fca40
> R13: 0000000000800000 R14: ffff88804417f4c0 R15: ffffffffffeac000
> FS:  00007fd4f579c6c0(0000) GS:ffff888135d00000(0000) knlGS:0000000000000=
000
> CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> CR2: 00007fc0bff9d988 CR3: 0000000013cc4000 CR4: 0000000000752ef0
> DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
> PKRU: 55555554
> Call Trace:
> <TASK>
> ovl_copy_up_tmpfile fs/overlayfs/copy_up.c:865 [inline]
> ovl_do_copy_up fs/overlayfs/copy_up.c:978 [inline]
> ovl_copy_up_one fs/overlayfs/copy_up.c:1170 [inline]
> ovl_copy_up_flags+0x1717/0x2cc0 fs/overlayfs/copy_up.c:1225
> ovl_copy_up+0x20/0x30 fs/overlayfs/copy_up.c:1265
> ovl_setattr+0xc1/0x370 fs/overlayfs/inode.c:41
> notify_change+0x8d2/0x960 fs/attr.c:499
> chmod_common+0x1d5/0x330 fs/open.c:648
> do_fchmodat fs/open.c:696 [inline]
> __do_sys_fchmodat fs/open.c:715 [inline]
> __se_sys_fchmodat fs/open.c:712 [inline]
> __x64_sys_fchmodat+0xea/0x180 fs/open.c:712
> do_syscall_x64 arch/x86/entry/common.c:52 [inline]
> do_syscall_64+0x59/0x120 arch/x86/entry/common.c:83
> entry_SYSCALL_64_after_hwframe+0x63/0x6b
> RIP: 0033:0x7fd4f647cc29
> Code: 28 00 00 00 75 05 48 83 c4 28 c3 e8 e1 20 00 00 90 48 89 f8 48
> 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d
> 01 f0 ff ff 73 01 c3 48 c7 c1 b0 ff ff ff f7 d8 64 89 01 48
> RSP: 002b:00007fd4f579c0c8 EFLAGS: 00000246 ORIG_RAX: 000000000000010c
> RAX: ffffffffffffffda RBX: 00007fd4f659c1f0 RCX: 00007fd4f647cc29
> RDX: 0000000000000000 RSI: 00000000200000c0 RDI: 0000000000000005
> RBP: 00007fd4f64c847a R08: 0000000000000000 R09: 0000000000000000
> R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000000000
> R13: 000000000000006e R14: 00007fd4f659c1f0 R15: 00007ffc819fff88
> </TASK>
> ---[ end trace 0000000000000000 ]---
>
[...]

> =3D* repro.txt =3D*
> syz_mount_image$fuse(0x0, &(0x7f0000001040)=3D'./file2\x00', 0x0, 0x0,
> 0x0, 0x0, 0x0)
> syz_mount_image$fuse(0x0, &(0x7f0000002080)=3D'./file0\x00', 0x0, 0x0,
> 0x0, 0x0, 0x0)
> mount$overlay(0x0, &(0x7f0000000040)=3D'./file0\x00', &(0x7f0000000000),
> 0x0, &(0x7f0000000100)=3D{[{@workdir=3D{'workdir', 0x3d, './file0'}},
> {@lowerdir=3D{'lowerdir', 0x3d, '.'}}, {@upperdir=3D{'upperdir', 0x3d,
> './file2'}}], [], 0x2c})
> r0 =3D openat(0xffffffffffffff9c, &(0x7f0000000040)=3D'./file1\x00', 0xa4=
2, 0x0)
> write$P9_RXATTRCREATE(r0, &(0x7f0000000000)=3D{0xfffffffffffffe98}, 0x800=
600)
> openat$dir(0xffffff9c, &(0x7f0000000080)=3D'./file1\x00', 0x20200, 0x0)
> write$P9_RLCREATE(r0, &(0x7f0000000300)=3D{0x18}, 0x18)
> r1 =3D openat$dir(0xffffffffffffff9c, &(0x7f00000000c0)=3D'./file0\x00', =
0x0, 0x0)
> fchmodat(r1, &(0x7f00000000c0)=3D'./file1\x00', 0xd00)
>
>
> See aslo https://gist.github.com/xrivendell7/86a9486e8e1e703c4f4453abec06=
4fb5
>
> I hope it helps.

There are no details about how this repro was run like there are in
syzkaller reports , e.g.:

{"threaded":true,"repeat":true,"procs":5,"slowdown":1,...

I assume this was run in concurrent threads that write to a file that
is an underlying
file for some ovl mounts (because they all mount over the same path).

The only way I see this WARN_ON() get hit:

if (WARN_ON_ONCE(pos < 0 || len < 0 || totlen < 0))
                return -EIO;

is if len < 0, after a seek over a hole did:
                len -=3D hole_len;

If racing to grow the underlying lower file under overlayfs this might happ=
en -
not sure if this was the case here.

If that happens it falls well under the category of:
https://docs.kernel.org/filesystems/overlayfs.html#changes-to-underlying-fi=
lesystems

Perhaps a WARN_ON is a bit too harsh for this case, but since it is
a WARN_ON_ONCE it is not a big issue IMO.

Then again, my analysis may be wrong.
Please check if the assertion is indeed about len < 0
and whether it really did get to be negative after seeking a hole
to an offset beyond the original file size (when copy up started).

Thanks,
Amir.