[syzbot] [overlayfs?] general protection fault in ovl_llseek

[syzbot] [overlayfs?] general protection fault in ovl_llseek

[syzbot]

Hello,

syzbot found the following issue on:

HEAD commit:    11a299a7933e Merge tag 'for-6.12/block-20240925' of git://..
git tree:       upstream
console output: https://syzkaller.appspot.com/x/log.txt?x=1549da80580000
kernel config:  https://syzkaller.appspot.com/x/.config?x=25e41eb82fab6c0b
dashboard link: https://syzkaller.appspot.com/bug?extid=d9efec94dcbfa0de1c07
compiler:       Debian clang version 15.0.6, GNU ld (GNU Binutils for Debian) 2.40
syz repro:      https://syzkaller.appspot.com/x/repro.syz?x=1349da80580000
C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=15870507980000

Downloadable assets:
disk image (non-bootable): https://storage.googleapis.com/syzbot-assets/7bc7510fe41f/non_bootable_disk-11a299a7.raw.xz
vmlinux: https://storage.googleapis.com/syzbot-assets/b59f47d0c0da/vmlinux-11a299a7.xz
kernel image: https://storage.googleapis.com/syzbot-assets/bf395abcfb64/bzImage-11a299a7.xz

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

RBP: 0000000000000001 R08: 00007fff0dcc56f7 R09: 00000000000000a0
R10: 0000000000000001 R11: 0000000000000246 R12: 0000000000000001
R13: 431bde82d7b634db R14: 0000000000000001 R15: 0000000000000001
 </TASK>
Oops: general protection fault, probably for non-canonical address 0xdffffc0000000022: 0000 [#1] PREEMPT SMP KASAN NOPTI
KASAN: null-ptr-deref in range [0x0000000000000110-0x0000000000000117]
CPU: 0 UID: 0 PID: 5106 Comm: syz-executor776 Not tainted 6.11.0-syzkaller-10669-g11a299a7933e #0
Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.16.3-debian-1.16.3-2~bpo12+1 04/01/2014
RIP: 0010:ovl_llseek+0x2a4/0x3f0 fs/overlayfs/file.c:214
Code: 8d 7c 24 60 e8 ad db e0 fe 48 8b 44 24 60 48 89 44 24 30 48 83 e0 fc 48 89 44 24 20 4c 8d b0 20 01 00 00 4d 89 f7 49 c1 ef 03 <43> 80 3c 27 00 74 08 4c 89 f7 e8 6d dc e0 fe 49 89 1e 48 8b 1c 24
RSP: 0018:ffffc90002d6fe00 EFLAGS: 00010207
RAX: fffffffffffffff4 RBX: 0000000000000000 RCX: 0000000000000001
RDX: 0000000000000000 RSI: ffffffff8c610080 RDI: 0000000000000001
RBP: ffffc90002d6fec8 R08: ffffffff901ce56f R09: 1ffffffff2039cad
R10: dffffc0000000000 R11: fffffbfff2039cae R12: dffffc0000000000
R13: 1ffff11007b2e7ac R14: 0000000000000114 R15: 0000000000000022
FS:  0000555590dd5380(0000) GS:ffff88801fc00000(0000) knlGS:0000000000000000
CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 00007fcababb3226 CR3: 000000004186a000 CR4: 0000000000350ef0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
Call Trace:
 <TASK>
 vfs_llseek fs/read_write.c:382 [inline]
 ksys_lseek fs/read_write.c:395 [inline]
 __do_sys_lseek fs/read_write.c:406 [inline]
 __se_sys_lseek fs/read_write.c:404 [inline]
 __x64_sys_lseek+0x150/0x1e0 fs/read_write.c:404
 do_syscall_x64 arch/x86/entry/common.c:52 [inline]
 do_syscall_64+0xf3/0x230 arch/x86/entry/common.c:83
 entry_SYSCALL_64_after_hwframe+0x77/0x7f
RIP: 0033:0x7fcabab5d8e9
Code: 28 00 00 00 75 05 48 83 c4 28 c3 e8 a1 1a 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 b8 ff ff ff f7 d8 64 89 01 48
RSP: 002b:00007fff0dcc5958 EFLAGS: 00000246 ORIG_RAX: 0000000000000008
RAX: ffffffffffffffda RBX: 00007fff0dcc5970 RCX: 00007fcabab5d8e9
RDX: 0000000000000000 RSI: 0000000000010000 RDI: 0000000000000004
RBP: 0000000000000001 R08: 00007fff0dcc56f7 R09: 00000000000000a0
R10: 0000000000000001 R11: 0000000000000246 R12: 0000000000000001
R13: 431bde82d7b634db R14: 0000000000000001 R15: 0000000000000001
 </TASK>
Modules linked in:
---[ end trace 0000000000000000 ]---
RIP: 0010:ovl_llseek+0x2a4/0x3f0 fs/overlayfs/file.c:214
Code: 8d 7c 24 60 e8 ad db e0 fe 48 8b 44 24 60 48 89 44 24 30 48 83 e0 fc 48 89 44 24 20 4c 8d b0 20 01 00 00 4d 89 f7 49 c1 ef 03 <43> 80 3c 27 00 74 08 4c 89 f7 e8 6d dc e0 fe 49 89 1e 48 8b 1c 24
RSP: 0018:ffffc90002d6fe00 EFLAGS: 00010207
RAX: fffffffffffffff4 RBX: 0000000000000000 RCX: 0000000000000001
RDX: 0000000000000000 RSI: ffffffff8c610080 RDI: 0000000000000001
RBP: ffffc90002d6fec8 R08: ffffffff901ce56f R09: 1ffffffff2039cad
R10: dffffc0000000000 R11: fffffbfff2039cae R12: dffffc0000000000
R13: 1ffff11007b2e7ac R14: 0000000000000114 R15: 0000000000000022
FS:  0000555590dd5380(0000) GS:ffff88801fc00000(0000) knlGS:0000000000000000
CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 00007fcababb3226 CR3: 000000004186a000 CR4: 0000000000350ef0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
----------------
Code disassembly (best guess):
   0:	8d 7c 24 60          	lea    0x60(%rsp),%edi
   4:	e8 ad db e0 fe       	call   0xfee0dbb6
   9:	48 8b 44 24 60       	mov    0x60(%rsp),%rax
   e:	48 89 44 24 30       	mov    %rax,0x30(%rsp)
  13:	48 83 e0 fc          	and    $0xfffffffffffffffc,%rax
  17:	48 89 44 24 20       	mov    %rax,0x20(%rsp)
  1c:	4c 8d b0 20 01 00 00 	lea    0x120(%rax),%r14
  23:	4d 89 f7             	mov    %r14,%r15
  26:	49 c1 ef 03          	shr    $0x3,%r15
* 2a:	43 80 3c 27 00       	cmpb   $0x0,(%r15,%r12,1) <-- trapping instruction
  2f:	74 08                	je     0x39
  31:	4c 89 f7             	mov    %r14,%rdi
  34:	e8 6d dc e0 fe       	call   0xfee0dca6
  39:	49 89 1e             	mov    %rbx,(%r14)
  3c:	48 8b 1c 24          	mov    (%rsp),%rbx


---
This report is generated by a bot. It may contain errors.
See https://goo.gl/tpsmEJ for more information about syzbot.
syzbot engineers can be reached at syzkaller@googlegroups.com.

syzbot will keep track of this issue. See:
https://goo.gl/tpsmEJ#status for how to communicate with syzbot.

If the report is already addressed, let syzbot know by replying with:
#syz fix: exact-commit-title

If you want syzbot to run the reproducer, reply with:
#syz test: git://repo/address.git branch-or-commit-hash
If you attach or paste a git patch, syzbot will apply it before testing.

If you want to overwrite report's subsystems, reply with:
#syz set subsystems: new-subsystem
(See the list of subsystem names on the web dashboard)

If the report is a duplicate of another one, reply with:
#syz dup: exact-subject-of-another-report

If you want to undo deduplication, reply with:
#syz undup

Re: [syzbot] [overlayfs?] general protection fault in ovl_llseek

[Leo Stone]

Add a check to avoid using an invalid pointer if ovl_open_realfile fails.

#syz test: git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master

diff --git a/fs/overlayfs/file.c b/fs/overlayfs/file.c
index 2b7a5a3a7a2f..67f75eeb1e51 100644
--- a/fs/overlayfs/file.c
+++ b/fs/overlayfs/file.c
@@ -117,7 +117,11 @@ static int ovl_real_fdget_meta(const struct file *file, struct fd *real,
                struct file *f = ovl_open_realfile(file, &realpath);
                if (IS_ERR(f))
                        return PTR_ERR(f);
-               real->word = (unsigned long)ovl_open_realfile(file, &realpath) | FDPUT_FPUT;
+               f = ovl_open_realfile(file, &realpath);
+               if (IS_ERR(f))
+                       return PTR_ERR(f);
+               real->word = (unsigned long)f;
+               real->word |= FDPUT_FPUT;
                return 0;
        }

Re: [syzbot] [overlayfs?] general protection fault in ovl_llseek

[syzbot]

Hello,

syzbot has tested the proposed patch and the reproducer did not trigger any issue:

Reported-by: syzbot+d9efec94dcbfa0de1c07@syzkaller.appspotmail.com
Tested-by: syzbot+d9efec94dcbfa0de1c07@syzkaller.appspotmail.com

Tested on:

commit:         075dbe9f Merge tag 'soc-ep93xx-dt-6.12' of git://git.k..
git tree:       upstream
console output: https://syzkaller.appspot.com/x/log.txt?x=13ce159f980000
kernel config:  https://syzkaller.appspot.com/x/.config?x=b2d4fdf18a83ec0b
dashboard link: https://syzkaller.appspot.com/bug?extid=d9efec94dcbfa0de1c07
compiler:       Debian clang version 15.0.6, GNU ld (GNU Binutils for Debian) 2.40
patch:          https://syzkaller.appspot.com/x/patch.diff?x=112c6aa9980000

Note: testing is done by a robot and is best-effort only.

Re: [syzbot] [overlayfs?] general protection fault in ovl_llseek

[Amir Goldstein]

[-- Attachment #1: Type: text/plain, Size: 1204 bytes --]

On Fri, Sep 27, 2024 at 9:10 AM Leo Stone <leocstone@gmail.com> wrote:
>
> Add a check to avoid using an invalid pointer if ovl_open_realfile fails.
>
> #syz test: git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
>
> diff --git a/fs/overlayfs/file.c b/fs/overlayfs/file.c
> index 2b7a5a3a7a2f..67f75eeb1e51 100644
> --- a/fs/overlayfs/file.c
> +++ b/fs/overlayfs/file.c
> @@ -117,7 +117,11 @@ static int ovl_real_fdget_meta(const struct file *file, struct fd *real,
>                 struct file *f = ovl_open_realfile(file, &realpath);
>                 if (IS_ERR(f))
>                         return PTR_ERR(f);
> -               real->word = (unsigned long)ovl_open_realfile(file, &realpath) | FDPUT_FPUT;
> +               f = ovl_open_realfile(file, &realpath);
> +               if (IS_ERR(f))
> +                       return PTR_ERR(f);
> +               real->word = (unsigned long)f;
> +               real->word |= FDPUT_FPUT;
>                 return 0;
>         }
>
>

No, that's the wrong fix.
There is a braino and a file leak in this code.

Linus,

Could you apply this braino fix manually before releasing rc1.

Thanks,
Amir.

[-- Attachment #2: 0001-ovl-fix-file-leak-in-ovl_real_fdget_meta.patch --]
[-- Type: text/x-patch, Size: 1043 bytes --]

From 994d5a61855da275292780af72948d7207025ec8 Mon Sep 17 00:00:00 2001
From: Amir Goldstein <amir73il@gmail.com>
Date: Fri, 27 Sep 2024 13:54:23 +0200
Subject: [PATCH] ovl: fix file leak in ovl_real_fdget_meta()

ovl_open_realfile() is wrongly called twice after conversion to
new struct fd.

Fixes: ("88a2f6468d01 struct fd: representation change")
Reported-by: syzbot+d9efec94dcbfa0de1c07@syzkaller.appspotmail.com
Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---
 fs/overlayfs/file.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/overlayfs/file.c b/fs/overlayfs/file.c
index 2b7a5a3a7a2f..4504493b20be 100644
--- a/fs/overlayfs/file.c
+++ b/fs/overlayfs/file.c
@@ -117,7 +117,7 @@ static int ovl_real_fdget_meta(const struct file *file, struct fd *real,
 		struct file *f = ovl_open_realfile(file, &realpath);
 		if (IS_ERR(f))
 			return PTR_ERR(f);
-		real->word = (unsigned long)ovl_open_realfile(file, &realpath) | FDPUT_FPUT;
+		real->word = (unsigned long)f | FDPUT_FPUT;
 		return 0;
 	}
 
-- 
2.34.1

Re: [syzbot] [overlayfs?] general protection fault in ovl_llseek

[Amir Goldstein]

[-- Attachment #1: Type: text/plain, Size: 1409 bytes --]

On Fri, Sep 27, 2024 at 2:03 PM Amir Goldstein <amir73il@gmail.com> wrote:
>
> On Fri, Sep 27, 2024 at 9:10 AM Leo Stone <leocstone@gmail.com> wrote:
> >
> > Add a check to avoid using an invalid pointer if ovl_open_realfile fails.
> >
> > #syz test: git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
> >
> > diff --git a/fs/overlayfs/file.c b/fs/overlayfs/file.c
> > index 2b7a5a3a7a2f..67f75eeb1e51 100644
> > --- a/fs/overlayfs/file.c
> > +++ b/fs/overlayfs/file.c
> > @@ -117,7 +117,11 @@ static int ovl_real_fdget_meta(const struct file *file, struct fd *real,
> >                 struct file *f = ovl_open_realfile(file, &realpath);
> >                 if (IS_ERR(f))
> >                         return PTR_ERR(f);
> > -               real->word = (unsigned long)ovl_open_realfile(file, &realpath) | FDPUT_FPUT;
> > +               f = ovl_open_realfile(file, &realpath);
> > +               if (IS_ERR(f))
> > +                       return PTR_ERR(f);
> > +               real->word = (unsigned long)f;
> > +               real->word |= FDPUT_FPUT;
> >                 return 0;
> >         }
> >
> >
>
> No, that's the wrong fix.
> There is a braino and a file leak in this code.
>
> Linus,
>
> Could you apply this braino fix manually before releasing rc1.
>

Too quick to send. I messed up the Fixes: tag.
Now fixed.

Thanks,
Amir.

[-- Attachment #2: 0001-ovl-fix-file-leak-in-ovl_real_fdget_meta.patch --]
[-- Type: text/x-patch, Size: 1043 bytes --]

From 994d5a61855da275292780af72948d7207025ec8 Mon Sep 17 00:00:00 2001
From: Amir Goldstein <amir73il@gmail.com>
Date: Fri, 27 Sep 2024 13:54:23 +0200
Subject: [PATCH] ovl: fix file leak in ovl_real_fdget_meta()

ovl_open_realfile() is wrongly called twice after conversion to
new struct fd.

Fixes: 88a2f6468d01 ("struct fd: representation change")
Reported-by: syzbot+d9efec94dcbfa0de1c07@syzkaller.appspotmail.com
Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---
 fs/overlayfs/file.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/overlayfs/file.c b/fs/overlayfs/file.c
index 2b7a5a3a7a2f..4504493b20be 100644
--- a/fs/overlayfs/file.c
+++ b/fs/overlayfs/file.c
@@ -117,7 +117,7 @@ static int ovl_real_fdget_meta(const struct file *file, struct fd *real,
 		struct file *f = ovl_open_realfile(file, &realpath);
 		if (IS_ERR(f))
 			return PTR_ERR(f);
-		real->word = (unsigned long)ovl_open_realfile(file, &realpath) | FDPUT_FPUT;
+		real->word = (unsigned long)f | FDPUT_FPUT;
 		return 0;
 	}
 
-- 
2.34.1

Re: [syzbot] [overlayfs?] general protection fault in ovl_llseek

[Linus Torvalds]

On Fri, 27 Sept 2024 at 05:41, Amir Goldstein <amir73il@gmail.com> wrote:
>
> Too quick to send. I messed up the Fixes: tag.
> Now fixed.

Applied.

However, just for the future: please send patches that you expect me
to apply with a very explicit subject line to that effect.

I get too much email, and hey, I do try to read it all (even if I
don't answer), but I'm really really good at scanning my emails
quickly.

In other words: sometimes I'm a bit *too* good at the "quickly" part,
and end up missing the fact that "oh, there was a patch there that I
need to actually react to and apply".

That has become more true over the years as the individual patch count
has gone down, and *most* of what I do is git pulls, and most of the
emailed patches I see tend to be things that are for review, not
application.

Yes, I picked it up this time. And maybe I even pick up on these
things *most* of the time.

But I still strongly suspect that to make it more likely that I don't
miss anything, you make the subject line some big clue-bat to my head
like having "[PATCH-for-linus]" header.

Because even just a "[PATCH]" is likely to trigger my "patch review"
logic rather than something I'm actually expected to apply, just
because I see *so* many patches fly by.

This was your daily "Linus is all kinds of disorganized and
incompetent" notification. Making things obvious to me is always a
good thing. It's why those "[GIT PULL]" subject lines help not just
pr-tracker-bot, but also me.

Thanks,
                    Linus

Re: [syzbot] [overlayfs?] general protection fault in ovl_llseek

[Al Viro]

On Fri, Sep 27, 2024 at 12:47:16PM -0700, Linus Torvalds wrote:

> But I still strongly suspect that to make it more likely that I don't
> miss anything, you make the subject line some big clue-bat to my head
> like having "[PATCH-for-linus]" header.

BTW, what do you prefer for "please run this script with this explanation
just before -rc1" kind of stuff?

Example: https://lore.kernel.org/all/20240927015611.GT3550746@ZenIV/

I've used "[tree-wide]" as a prefix, but I've no idea what your preferences
would be in that respect...

Re: [syzbot] [overlayfs?] general protection fault in ovl_llseek

[Linus Torvalds]

On Fri, 27 Sept 2024 at 13:15, Al Viro <viro@zeniv.linux.org.uk> wrote:
>
> BTW, what do you prefer for "please run this script with this explanation
> just before -rc1" kind of stuff?

Well, I didn't miss that one either (see commit cb787f4ac0c2:
"[tree-wide] finally take no_llseek out") but yes, it's probably
better to mark those kinds of things very obviously too (and a
'[PATCH-for-linus]' like subject line is probably good there too).

And hey, people will forget, and I'll - mostly - figure it out anyway,
so it's not like this is a big worry. Maybe if you notice that I
missed something, you can make sure that the re-send has a big neon
sign for me.

               Linus