[syzbot] WARNING: refcount bug in sys_memfd_secret

[syzbot]

Hello,

syzbot found the following issue on:

HEAD commit: 64222515138e Merge tag 'drm-fixes-2021-10-22' of git://ano..
git tree: upstream
console output: https://syzkaller.appspot.com/x/log.txt?x=178e86c4b00000
kernel config: https://syzkaller.appspot.com/x/.config?x=be398dd7862f4b36
dashboard link: https://syzkaller.appspot.com/bug?extid=b904a1de3ec43711eba5
compiler: gcc (Debian 10.2.1-6) 10.2.1 20210110, GNU ld (GNU Binutils for Debian) 2.35.2

Unfortunately, I don't have any reproducer for this issue yet.

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

------------[ cut here ]------------
refcount_t: addition on 0; use-after-free.
WARNING: CPU: 2 PID: 32193 at lib/refcount.c:25 refcount_warn_saturate+0x169/0x1e0 lib/refcount.c:25
Modules linked in:
CPU: 2 PID: 32193 Comm: syz-executor.0 Not tainted 5.15.0-rc6-syzkaller #0
Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.14.0-2 04/01/2014
RIP: 0010:refcount_warn_saturate+0x169/0x1e0 lib/refcount.c:25
Code: 09 31 ff 89 de e8 f7 b9 9b fd 84 db 0f 85 36 ff ff ff e8 3a b2 9b fd 48 c7 c7 c0 65 e3 89 c6 05 6f 6c 7f 09 01 e8 7e 4a 19 05 <0f> 0b e9 17 ff ff ff e8 1b b2 9b fd 0f b6 1d 54 6c 7f 09 31 ff 89
RSP: 0018:ffffc90009f9ff10 EFLAGS: 00010286
RAX: 0000000000000000 RBX: 0000000000000000 RCX: 0000000000000000
RDX: 0000000000040000 RSI: ffffffff815dbf58 RDI: fffff520013f3fd4
RBP: 0000000000000002 R08: 0000000000000000 R09: 0000000000000001
R10: ffffffff815d5cce R11: 0000000000000000 R12: 0000000000000000
R13: 0000000000000000 R14: 0000000000000000 R15: 0000000000000000
FS: 00007f476a1e7700(0000) GS:ffff88802cd00000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 0000001b32c24000 CR3: 00000000494c4000 CR4: 0000000000150ee0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
Call Trace:
__refcount_add include/linux/refcount.h:199 [inline]
__refcount_inc include/linux/refcount.h:250 [inline]
refcount_inc include/linux/refcount.h:267 [inline]
__do_sys_memfd_secret mm/secretmem.c:221 [inline]
__se_sys_memfd_secret mm/secretmem.c:194 [inline]
__x64_sys_memfd_secret+0x182/0x1e0 mm/secretmem.c:194
do_syscall_x64 arch/x86/entry/common.c:50 [inline]
do_syscall_64+0x35/0xb0 arch/x86/entry/common.c:80
entry_SYSCALL_64_after_hwframe+0x44/0xae
RIP: 0033:0x7f476cc71a39
Code: ff ff c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 40 00 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 bc ff ff ff f7 d8 64 89 01 48
RSP: 002b:00007f476a1e7188 EFLAGS: 00000246 ORIG_RAX: 00000000000001bf
RAX: ffffffffffffffda RBX: 00007f476cd74f60 RCX: 00007f476cc71a39
RDX: 0000000000000000 RSI: 0000000000000000 RDI: 0000000000000000
RBP: 00007f476cccbe8f R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000000000
R13: 00007ffc5b5eb85f R14: 00007f476a1e7300 R15: 0000000000022000


---
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 syzk...@googlegroups.com.

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

[Dmitry Vyukov]

On Fri, 22 Oct 2021 at 17:02, syzbot
<syzbot+b904a1...@syzkaller.appspotmail.com> wrote:
>
> Hello,
>
> syzbot found the following issue on:
>
> HEAD commit: 64222515138e Merge tag 'drm-fixes-2021-10-22' of git://ano..
> git tree: upstream
> console output: https://syzkaller.appspot.com/x/log.txt?x=178e86c4b00000
> kernel config: https://syzkaller.appspot.com/x/.config?x=be398dd7862f4b36
> dashboard link: https://syzkaller.appspot.com/bug?extid=b904a1de3ec43711eba5
> compiler: gcc (Debian 10.2.1-6) 10.2.1 20210110, GNU ld (GNU Binutils for Debian) 2.35.2
>
> Unfortunately, I don't have any reproducer for this issue yet.
>
> IMPORTANT: if you fix the issue, please add the following tag to the commit:
> Reported-by: syzbot+b904a1...@syzkaller.appspotmail.com

+Mike, Jordy for secretmem.c

> --
> You received this message because you are subscribed to the Google Groups "syzkaller-bugs" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to syzkaller-bug...@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/syzkaller-bugs/00000000000062d0fc05cef24c57%40google.com.

[Jordy Zomer]

After a quick scan, it appears to be a false-positive.

This because nothing appears to be being freed.

In any case, you probably don't want warnings everywhere.

I believe we should probably do something along the lines of:

if (refcount_read(&secretmem_users) == 0) {
refcount_set(&secretmem_users, 1);
} else {
refcount_inc(&secretmem_users);
}

Does this appear to be a feasible patch? :)

Best Regards,

Jordy

[Dmitry Vyukov]

On Fri, 22 Oct 2021 at 18:25, Jordy Zomer <jo...@pwning.systems> wrote:
>
> After a quick scan, it appears to be a false-positive.
>
> This because nothing appears to be being freed.
>
> In any case, you probably don't want warnings everywhere.
>
> I believe we should probably do something along the lines of:
>
> if (refcount_read(&secretmem_users) == 0) {
> refcount_set(&secretmem_users, 1);
> } else {
> refcount_inc(&secretmem_users);
> }
>
> Does this appear to be a feasible patch? :)

I don't think multithreading work this way :)
Imagine 2 threads reading refcount_read(&secretmem_users) == 0 and
then both doing refcount_set(&secretmem_users, 1).

[Jordy Zomer]

Good point, it appears that we'll have to implement some locking at this stage as well.

Best Regards,

Jordy

[syzbot]

syzbot has found a reproducer for the following issue on:

HEAD commit: cf6c9d12750c Add linux-next specific files for 20211022
git tree: linux-next
console output: https://syzkaller.appspot.com/x/log.txt?x=10bdd272b00000
kernel config: https://syzkaller.appspot.com/x/.config?x=dd1cd3d631599df5

dashboard link: https://syzkaller.appspot.com/bug?extid=b904a1de3ec43711eba5
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=12790a72b00000
C reproducer: https://syzkaller.appspot.com/x/repro.c?x=13eb76dcb00000

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

------------[ cut here ]------------
refcount_t: addition on 0; use-after-free.

WARNING: CPU: 1 PID: 6528 at lib/refcount.c:25 refcount_warn_saturate+0x169/0x1e0 lib/refcount.c:25
Modules linked in:
CPU: 1 PID: 6528 Comm: syz-executor149 Not tainted 5.15.0-rc6-next-20211022-syzkaller #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011

RIP: 0010:refcount_warn_saturate+0x169/0x1e0 lib/refcount.c:25

Code: 09 31 ff 89 de e8 27 1f 9f fd 84 db 0f 85 36 ff ff ff e8 3a 1b 9f fd 48 c7 c7 00 2e 04 8a c6 05 c7 25 a3 09 01 e8 92 ce 31 05 <0f> 0b e9 17 ff ff ff e8 1b 1b 9f fd 0f b6 1d ac 25 a3 09 31 ff 89
RSP: 0018:ffffc90001a4ff10 EFLAGS: 00010286

RAX: 0000000000000000 RBX: 0000000000000000 RCX: 0000000000000000

RDX: ffff88801d369d40 RSI: ffffffff815f06f8 RDI: fffff52000349fd4
RBP: 0000000000000002 R08: 0000000000000000 R09: 0000000000000000
R10: ffffffff815ea4ce R11: 0000000000000000 R12: 0000000000000000

R13: 0000000000000000 R14: 0000000000000000 R15: 0000000000000000

FS: 00005555565e9300(0000) GS:ffff8880b9d00000(0000) knlGS:0000000000000000

CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033

CR2: 00007f842b6f56c0 CR3: 000000001bc33000 CR4: 00000000003506e0

DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
Call Trace:

<TASK>

__refcount_add include/linux/refcount.h:199 [inline]
__refcount_inc include/linux/refcount.h:250 [inline]
refcount_inc include/linux/refcount.h:267 [inline]
__do_sys_memfd_secret mm/secretmem.c:221 [inline]
__se_sys_memfd_secret mm/secretmem.c:194 [inline]
__x64_sys_memfd_secret+0x182/0x1e0 mm/secretmem.c:194
do_syscall_x64 arch/x86/entry/common.c:50 [inline]
do_syscall_64+0x35/0xb0 arch/x86/entry/common.c:80
entry_SYSCALL_64_after_hwframe+0x44/0xae

RIP: 0033:0x7fbeb6a4cf89
Code: 28 c3 e8 2a 14 00 00 66 2e 0f 1f 84 00 00 00 00 00 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 c0 ff ff ff f7 d8 64 89 01 48
RSP: 002b:00007ffde5076be8 EFLAGS: 00000246 ORIG_RAX: 00000000000001bf
RAX: ffffffffffffffda RBX: 0000000000000000 RCX: 00007fbeb6a4cf89
RDX: 00007fbeb6a0fe93 RSI: 0000000000000012 RDI: 0000000000080000
RBP: 00007fbeb6a10f70 R08: 0000000000000000 R09: 0000000000000000
R10: 00000000ffffffff R11: 0000000000000246 R12: 00007fbeb6a11000

R13: 0000000000000000 R14: 0000000000000000 R15: 0000000000000000

</TASK>

[syzbot]

Hello,

syzbot has tested the proposed patch but the reproducer is still triggering an issue:
WARNING: refcount bug in sys_memfd_secret

------------[ cut here ]------------
refcount_t: addition on 0; use-after-free.

WARNING: CPU: 1 PID: 9083 at lib/refcount.c:25 refcount_warn_saturate+0x169/0x1e0 lib/refcount.c:25
Modules linked in:
CPU: 1 PID: 9083 Comm: syz-executor.1 Not tainted 5.15.0-rc6-syzkaller #0

Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
RIP: 0010:refcount_warn_saturate+0x169/0x1e0 lib/refcount.c:25

Code: 09 31 ff 89 de e8 87 b4 9b fd 84 db 0f 85 36 ff ff ff e8 ca ac 9b fd 48 c7 c7 c0 65 e3 89 c6 05 ff 56 7f 09 01 e8 0d 45 19 05 <0f> 0b e9 17 ff ff ff e8 ab ac 9b fd 0f b6 1d e4 56 7f 09 31 ff 89
RSP: 0018:ffffc9000d23ff10 EFLAGS: 00010286

RAX: 0000000000000000 RBX: 0000000000000000 RCX: 0000000000000000

RDX: ffff888074b04300 RSI: ffffffff815dcf58 RDI: fffff52001a47fd4

RBP: 0000000000000002 R08: 0000000000000000 R09: 0000000000000000

R10: ffffffff815d6cce R11: 0000000000000000 R12: 0000000000000000

R13: 0000000000000000 R14: 0000000000000000 R15: 0000000000000000

FS: 00007ff50d6ec700(0000) GS:ffff8880b9d00000(0000) knlGS:0000000000000000

CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033

CR2: 00007ff757827000 CR3: 000000005b5d1000 CR4: 00000000003506e0

DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
Call Trace:

__refcount_add include/linux/refcount.h:199 [inline]
__refcount_inc include/linux/refcount.h:250 [inline]
refcount_inc include/linux/refcount.h:267 [inline]
__do_sys_memfd_secret mm/secretmem.c:221 [inline]
__se_sys_memfd_secret mm/secretmem.c:194 [inline]
__x64_sys_memfd_secret+0x182/0x1e0 mm/secretmem.c:194
do_syscall_x64 arch/x86/entry/common.c:50 [inline]
do_syscall_64+0x35/0xb0 arch/x86/entry/common.c:80
entry_SYSCALL_64_after_hwframe+0x44/0xae

RIP: 0033:0x7ff50df76a39
Code: ff ff c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 40 00 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 bc ff ff ff f7 d8 64 89 01 48
RSP: 002b:00007ff50d6ec188 EFLAGS: 00000246 ORIG_RAX: 00000000000001bf
RAX: ffffffffffffffda RBX: 00007ff50e079f60 RCX: 00007ff50df76a39

RDX: 0000000000000000 RSI: 0000000000000000 RDI: 0000000000000000

RBP: 00007ff50dfd0e8f R08: 0000000000000000 R09: 0000000000000000

R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000000000

R13: 00007fff653dc29f R14: 00007ff50d6ec300 R15: 0000000000022000


Tested on:

commit: 9c0c4d24 Merge tag 'block-5.15-2021-10-22' of git://gi..
git tree: upstream
console output: https://syzkaller.appspot.com/x/log.txt?x=149555c4b00000
kernel config: https://syzkaller.appspot.com/x/.config?x=be398dd7862f4b36

dashboard link: https://syzkaller.appspot.com/bug?extid=b904a1de3ec43711eba5
compiler: gcc (Debian 10.2.1-6) 10.2.1 20210110, GNU ld (GNU Binutils for Debian) 2.35.2

patch: https://syzkaller.appspot.com/x/patch.diff?x=13a70dc2b00000

[Mike Rapoport]

On Fri, Oct 22, 2021 at 05:07:40PM +0200, Dmitry Vyukov wrote:
> On Fri, 22 Oct 2021 at 17:02, syzbot
> <syzbot+b904a1...@syzkaller.appspotmail.com> wrote:
> >
> > Hello,
> >
> > syzbot found the following issue on:
> >
> > HEAD commit: 64222515138e Merge tag 'drm-fixes-2021-10-22' of git://ano..
> > git tree: upstream
> > console output: https://syzkaller.appspot.com/x/log.txt?x=178e86c4b00000
> > kernel config: https://syzkaller.appspot.com/x/.config?x=be398dd7862f4b36
> > dashboard link: https://syzkaller.appspot.com/bug?extid=b904a1de3ec43711eba5
> > compiler: gcc (Debian 10.2.1-6) 10.2.1 20210110, GNU ld (GNU Binutils for Debian) 2.35.2
> >
> > Unfortunately, I don't have any reproducer for this issue yet.
> >
> > IMPORTANT: if you fix the issue, please add the following tag to the commit:
> > Reported-by: syzbot+b904a1...@syzkaller.appspotmail.com
>
> +Mike, Jordy for secretmem.c

I was actually the first to report it ;-)

https://lore.kernel.org/all/YXJjuWyY...@kernel.org/

and my first reaction was to send a revert the untested commit 110860541f44
("mm/secretmem: use refcount_t instead of atomic_t").

Anyway, this should fix it:

From c22a588fab3a0762f0a8c0dbab99343c48b3e27b Mon Sep 17 00:00:00 2001
From: Mike Rapoport <rp...@linux.ibm.com>
Date: Sat, 23 Oct 2021 18:13:16 +0300
Subject: [PATCH] secretmem: bump initial refcount to fix refcount woes

Commit 110860541f44 ("mm/secretmem: use refcount_t instead of atomic_t")
replaced atomic_t with refcount_t but it dind't take into account that
unlike atomic_inc(), refcount_inc() presumes that "the caller already has a
reference on the object". With that, using 0 as initial count caused
warnings in the refcount code:

[ 20.957833] ------------[ cut here ]------------
[ 20.957844] refcount_t: addition on 0; use-after-free.
[ 20.957897] WARNING: CPU: 3 PID: 598 at /home/rppt/git/linux/lib/refcount.c:25 refcount_warn_saturate+0xcf/0xf0
[ 20.957919] Modules linked in:
[ 20.957930] CPU: 3 PID: 598 Comm: secretmemfd Not tainted 5.15.0-rc6+ #432
[ 20.957944] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.10.2-1ubuntu1 04/01/2014
[ 20.957948] RIP: 0010:refcount_warn_saturate+0xcf/0xf0
[ 20.957957] Code: 01 01 e8 d4 db c3 ff 0f 0b c3 80 3d 39 32 43 01 00 0f 85 6b ff ff ff 48 c7 c7 00 bc c5 af c6 05 25 32 43 01 01 e8 b1 db c3 ff <0f> 0b c3 48 c7 c7 b0 bb c5 af c6 05 10 32 43 01 01 e8 9b db c3 ff
[ 20.957962] RSP: 0018:ffffb188c0583f20 EFLAGS: 00010282
[ 20.957967] RAX: 0000000000000000 RBX: 0000000000000003 RCX: 0000000000000027
[ 20.957971] RDX: 0000000000000000 RSI: ffff8bfefbb975b0 RDI: ffff8bfefbb975b8
[ 20.957974] RBP: ffffb188c0583f48 R08: 0000000000000000 R09: 0000000000000001
[ 20.957977] R10: 0000000000000003 R11: ffffb188c0583d38 R12: 0000000000000000
[ 20.957980] R13: 0000000000000000 R14: 0000000000000000 R15: 0000000000000000
[ 20.957983] FS: 00007f9467b9c740(0000) GS:ffff8bfefbb80000(0000) knlGS:0000000000000000
[ 20.957993] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 20.957997] CR2: 00007ffe83be8084 CR3: 00000001100cc003 CR4: 0000000000060ee0
[ 20.958001] Call Trace:
[ 20.959285] __x64_sys_memfd_secret+0xa9/0xc0
[ 20.959308] do_syscall_64+0x3a/0x80
[ 20.959331] entry_SYSCALL_64_after_hwframe+0x44/0xae
[ 20.959352] RIP: 0033:0x7f9467cba89d
[ 20.959358] Code: 00 c3 66 2e 0f 1f 84 00 00 00 00 00 90 f3 0f 1e fa 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 8b 0d c3 f5 0c 00 f7 d8 64 89 01 48
[ 20.959362] RSP: 002b:00007ffe83bb8148 EFLAGS: 00000206 ORIG_RAX: 00000000000001bf
[ 20.959368] RAX: ffffffffffffffda RBX: 0000561f62400d50 RCX: 00007f9467cba89d
[ 20.959372] RDX: 0000000000000e11 RSI: 0000000000008000 RDI: 0000000000000000
[ 20.959375] RBP: 00007ffe83bb8160 R08: 000000002c06910a R09: 0000000000000000
[ 20.959378] R10: 00007f9467d8a1c4 R11: 0000000000000206 R12: 0000561f624008d0
[ 20.959381] R13: 00007ffe83bb82b0 R14: 0000000000000000 R15: 0000000000000000
[ 20.959386] ---[ end trace 9368244c7159e4de ]---
[ 20.960666] ------------[ cut here ]------------
[ 20.960675] refcount_t: decrement hit 0; leaking memory.
[ 20.960717] WARNING: CPU: 1 PID: 598 at /home/rppt/git/linux/lib/refcount.c:31 refcount_warn_saturate+0x4f/0xf0
[ 20.960737] Modules linked in:
[ 20.960742] CPU: 1 PID: 598 Comm: secretmemfd Tainted: G W 5.15.0-rc6+ #432
[ 20.960748] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.10.2-1ubuntu1 04/01/2014
[ 20.960751] RIP: 0010:refcount_warn_saturate+0x4f/0xf0
[ 20.960759] Code: 00 00 f3 c3 83 fe 03 74 43 83 fe 04 75 1f 80 3d b3 32 43 01 00 75 eb 48 c7 c7 58 bc c5 af c6 05 a3 32 43 01 01 e8 31 dc c3 ff <0f> 0b c3 80 3d 93 32 43 01 00 75 cc 48 c7 c7 88 bc c5 af c6 05 83
[ 20.960764] RSP: 0018:ffffb188c0583e40 EFLAGS: 00010286
[ 20.960769] RAX: 0000000000000000 RBX: ffff8bfec1f51900 RCX: 0000000000000027
[ 20.960772] RDX: 0000000000000000 RSI: ffff8bfefba975b0 RDI: ffff8bfefba975b8
[ 20.960775] RBP: 0000000000080003 R08: 0000000000000000 R09: 0000000000000001
[ 20.960778] R10: ffff8bfec439da80 R11: ffffb188c0583c58 R12: ffff8bfec4e576a0
[ 20.960781] R13: ffff8bfec01a8ca0 R14: ffff8bfecd314300 R15: 0000000000000000
[ 20.960784] FS: 0000000000000000(0000) GS:ffff8bfefba80000(0000) knlGS:0000000000000000
[ 20.960835] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 20.960840] CR2: 00007f9467c85290 CR3: 0000000080a0c004 CR4: 0000000000060ee0
[ 20.960843] Call Trace:
[ 20.960849] secretmem_release+0x26/0x30
[ 20.960862] __fput+0x85/0x240
[ 20.960868] task_work_run+0x67/0xa0
[ 20.960890] do_exit+0x363/0xbb0
[ 20.960902] do_group_exit+0x35/0x90
[ 20.960908] __x64_sys_exit_group+0xf/0x10
[ 20.960913] do_syscall_64+0x3a/0x80
[ 20.960922] entry_SYSCALL_64_after_hwframe+0x44/0xae
[ 20.960928] RIP: 0033:0x7f9467c852c6
[ 20.960933] Code: Unable to access opcode bytes at RIP 0x7f9467c8529c.
[ 20.960936] RSP: 002b:00007ffe83bb8168 EFLAGS: 00000246 ORIG_RAX: 00000000000000e7
[ 20.960941] RAX: ffffffffffffffda RBX: 00007f9467d8c610 RCX: 00007f9467c852c6
[ 20.960944] RDX: 0000000000000000 RSI: 000000000000003c RDI: 0000000000000000
[ 20.960947] RBP: 0000000000000000 R08: 00000000000000e7 R09: ffffffffffffff80
[ 20.960950] R10: 0000000000000003 R11: 0000000000000246 R12: 00007f9467d8c610
[ 20.960953] R13: 0000000000000001 R14: 00007f9467d8ffc8 R15: 0000000000000000
[ 20.960957] ---[ end trace 9368244c7159e4df ]---

Bump the initial reference count value to 1 to fix this.

Fixes: 110860541f44 ("mm/secretmem: use refcount_t instead of atomic_t")
Reported-by: syzbot+b904a1...@syzkaller.appspotmail.com
Signed-off-by: Mike Rapoport <rp...@linux.ibm.com>
---
mm/secretmem.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/mm/secretmem.c b/mm/secretmem.c
index 1fea68b8d5a6..06fd6407ed03 100644
--- a/mm/secretmem.c
+++ b/mm/secretmem.c
@@ -41,11 +41,11 @@ module_param_named(enable, secretmem_enable, bool, 0400);
MODULE_PARM_DESC(secretmem_enable,
"Enable secretmem and memfd_secret(2) system call");

-static refcount_t secretmem_users;
+static refcount_t secretmem_users = REFCOUNT_INIT(1);

bool secretmem_active(void)
{
- return !!refcount_read(&secretmem_users);
+ return refcount_read(&secretmem_users) > 1;
}

static vm_fault_t secretmem_fault(struct vm_fault *vmf)
--
2.28.0


--
Sincerely yours,
Mike.

[Kees Cook]

Excellent, thanks!

Reviewed-by: Kees Cook <kees...@chromium.org>

-Kees

>---
> mm/secretmem.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
>diff --git a/mm/secretmem.c b/mm/secretmem.c
>index 1fea68b8d5a6..06fd6407ed03 100644
>--- a/mm/secretmem.c
>+++ b/mm/secretmem.c
>@@ -41,11 +41,11 @@ module_param_named(enable, secretmem_enable, bool, 0400);
> MODULE_PARM_DESC(secretmem_enable,
> "Enable secretmem and memfd_secret(2) system call");
>
>-static refcount_t secretmem_users;
>+static refcount_t secretmem_users = REFCOUNT_INIT(1);
>
> bool secretmem_active(void)
> {
>- return !!refcount_read(&secretmem_users);
>+ return refcount_read(&secretmem_users) > 1;
> }
>
> static vm_fault_t secretmem_fault(struct vm_fault *vmf)

--

Kees Cook

[syzbot]

syzbot has bisected this issue to:

commit 110860541f443f950c1274f217a1a3e298670a33
Author: Jordy Zomer <jo...@jordyzomer.github.io>
Date: Wed Sep 8 02:56:18 2021 +0000

mm/secretmem: use refcount_t instead of atomic_t

bisection log: https://syzkaller.appspot.com/x/bisect.txt?x=141a071cb00000
start commit: 9c0c4d24ac00 Merge tag 'block-5.15-2021-10-22' of git://gi..
git tree: upstream
final oops: https://syzkaller.appspot.com/x/report.txt?x=161a071cb00000
console output: https://syzkaller.appspot.com/x/log.txt?x=121a071cb00000

kernel config: https://syzkaller.appspot.com/x/.config?x=be398dd7862f4b36
dashboard link: https://syzkaller.appspot.com/bug?extid=b904a1de3ec43711eba5

syz repro: https://syzkaller.appspot.com/x/repro.syz?x=130cabdcb00000
C reproducer: https://syzkaller.appspot.com/x/repro.c?x=175b91acb00000

Reported-by: syzbot+b904a1...@syzkaller.appspotmail.com

Fixes: 110860541f44 ("mm/secretmem: use refcount_t instead of atomic_t")

For information about bisection process see: https://goo.gl/tpsmEJ#bisection

[Matthew Wilcox]

On Sat, Oct 23, 2021 at 10:03:11AM -0700, Kees Cook wrote:
> On October 23, 2021 8:27:28 AM PDT, Mike Rapoport <rp...@kernel.org> wrote:
> >and my first reaction was to send a revert the untested commit 110860541f44
> >("mm/secretmem: use refcount_t instead of atomic_t").

I think you should. This isn't a real problem. And it abuses the
refcount_t interface. Your hack is clever, but it's fundamentally
wrong.

[Mike Rapoport]

On Sat, Oct 23, 2021 at 11:46:18PM +0100, Matthew Wilcox wrote:
> On Sat, Oct 23, 2021 at 10:03:11AM -0700, Kees Cook wrote:
> > On October 23, 2021 8:27:28 AM PDT, Mike Rapoport <rp...@kernel.org> wrote:
> > >and my first reaction was to send a revert the untested commit 110860541f44
> > >("mm/secretmem: use refcount_t instead of atomic_t").
>
> I think you should. This isn't a real problem.

Do you mean that creation of 4 billion of file descriptors is not feasible?

--
Sincerely yours,
Mike.

[Dmitry Vyukov]

FWIW I think refcount is at least capable of catching the issue I
described with the counter temporarily going below its true value.
With refcount it can be caught during fuzzing as refcount reaching 0
and then being incremented again. Basically this warning, but a true
positive.

[Matthew Wilcox]

On a sufficiently large machine, it is. But then we have the same
problem with other atomic_t. If you really care, just check whether
secretmem_users has gone negative, and return -ENFILE. It doesn't
even have to be all that exact; you've got 2 billion values of slop
to use before you hit the wrap from negative to 0 which is the actual
problem.

ie this:

+++ b/mm/secretmem.c
@@ -203,6 +203,8 @@ SYSCALL_DEFINE1(memfd_secret, unsigned int, flags)

if (flags & ~(SECRETMEM_FLAGS_MASK | O_CLOEXEC))
return -EINVAL;
+ if (atomic_read(&secretmem_users) < 0)
+ return -ENFILE;

fd = get_unused_fd_flags(flags & O_CLOEXEC);
if (fd < 0)


Also, why does secretmem depend on !EMBEDDED?

config EMBEDDED
bool "Embedded system"
select EXPERT
help
This option should be enabled if compiling the kernel for
an embedded system so certain expert options are available
for configuration.

This is the only Kconfig option that depends on !EMBEDDED. It's usually
used to avoid showing questions. It means that my allmodconfig build
*doesn't* build secretmem, which is surely not what you wanted.

+++ b/mm/Kconfig
@@ -892,7 +892,7 @@ config IO_MAPPING
bool

config SECRETMEM
- def_bool ARCH_HAS_SET_DIRECT_MAP && !EMBEDDED
+ def_bool ARCH_HAS_SET_DIRECT_MAP

source "mm/damon/Kconfig"

[Mike Rapoport]

On Sun, Oct 24, 2021 at 11:57:02AM +0100, Matthew Wilcox wrote:
> On Sun, Oct 24, 2021 at 08:37:59AM +0300, Mike Rapoport wrote:
> > On Sat, Oct 23, 2021 at 11:46:18PM +0100, Matthew Wilcox wrote:
> > > On Sat, Oct 23, 2021 at 10:03:11AM -0700, Kees Cook wrote:
> > > > On October 23, 2021 8:27:28 AM PDT, Mike Rapoport <rp...@kernel.org> wrote:
> > > > >and my first reaction was to send a revert the untested commit 110860541f44
> > > > >("mm/secretmem: use refcount_t instead of atomic_t").
> > >
> > > I think you should. This isn't a real problem.
> >
> > Do you mean that creation of 4 billion of file descriptors is not feasible?
>
> On a sufficiently large machine, it is. But then we have the same
> problem with other atomic_t. If you really care, just check whether
> secretmem_users has gone negative, and return -ENFILE. It doesn't
> even have to be all that exact; you've got 2 billion values of slop
> to use before you hit the wrap from negative to 0 which is the actual
> problem.
>
> ie this:
>
> +++ b/mm/secretmem.c
> @@ -203,6 +203,8 @@ SYSCALL_DEFINE1(memfd_secret, unsigned int, flags)
>
> if (flags & ~(SECRETMEM_FLAGS_MASK | O_CLOEXEC))
> return -EINVAL;
> + if (atomic_read(&secretmem_users) < 0)
> + return -ENFILE;

So you suggest to prevent creation of the file descriptor to ensure there
is no overflow of secretmem_users. I don't feel it's a clean and elegant
solution.

>
> fd = get_unused_fd_flags(flags & O_CLOEXEC);
> if (fd < 0)
>
>
> Also, why does secretmem depend on !EMBEDDED?

There was a request from tiny-config maintainers to keep this code outside
tiny-config and the best option I could find to make secretmem depend on
!EMBEDDED.

--
Sincerely yours,
Mike.

[syzbot]

syzbot suspects this issue was fixed by commit:

commit 87066fdd2e30fe9dd531125d95257c118a74617e
Author: Linus Torvalds <torv...@linux-foundation.org>
Date: Sun Oct 24 19:48:33 2021 +0000

Revert "mm/secretmem: use refcount_t instead of atomic_t"

bisection log: https://syzkaller.appspot.com/x/bisect.txt?x=11cccf06b00000

start commit: 9c0c4d24ac00 Merge tag 'block-5.15-2021-10-22' of git://gi..
git tree: upstream

kernel config: https://syzkaller.appspot.com/x/.config?x=be398dd7862f4b36
dashboard link: https://syzkaller.appspot.com/bug?extid=b904a1de3ec43711eba5

syz repro: https://syzkaller.appspot.com/x/repro.syz?x=130cabdcb00000
C reproducer: https://syzkaller.appspot.com/x/repro.c?x=175b91acb00000

If the result looks correct, please mark the issue as fixed by replying with:

#syz fix: Revert "mm/secretmem: use refcount_t instead of atomic_t"

[syzbot]

> #syz fix: Revert "mm/secretmem: use refcount_t instead of atomic_t"

Your 'fix:' command is accepted, but please keep syzkall...@googlegroups.com mailing list in CC next time. It serves as a history of what happened with each bug report. Thank you.