[syzbot] [alsa?] memory leak in snd_seq_create_port

[syzbot]

Hello,

syzbot found the following issue on:

HEAD commit: 3f01e9fed845 Merge tag 'linux-watchdog-6.5-rc2' of git://w..
git tree: upstream
console output: https://syzkaller.appspot.com/x/log.txt?x=14b07344a80000
kernel config: https://syzkaller.appspot.com/x/.config?x=75da4f0a455bdbd3
dashboard link: https://syzkaller.appspot.com/bug?extid=cf8e7fa4eeec59b3d485
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=15877dc2a80000
C reproducer: https://syzkaller.appspot.com/x/repro.c?x=12905004a80000

Downloadable assets:
disk image: https://storage.googleapis.com/syzbot-assets/441fb7ea58b8/disk-3f01e9fe.raw.xz
vmlinux: https://storage.googleapis.com/syzbot-assets/8fa7790ba0c3/vmlinux-3f01e9fe.xz
kernel image: https://storage.googleapis.com/syzbot-assets/5e7a6471dadf/bzImage-3f01e9fe.xz

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

Warning: Permanently added '10.128.1.1' (ED25519) to the list of known hosts.
executing program
executing program
BUG: memory leak
unreferenced object 0xffff888100877000 (size 512):
comm "syz-executor257", pid 5012, jiffies 4294941742 (age 12.790s)
hex dump (first 32 bytes):
80 ff 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
backtrace:
[<ffffffff8154bf94>] kmalloc_trace+0x24/0x90 mm/slab_common.c:1076
[<ffffffff83d29e28>] kmalloc include/linux/slab.h:582 [inline]
[<ffffffff83d29e28>] kzalloc include/linux/slab.h:703 [inline]
[<ffffffff83d29e28>] snd_seq_create_port+0x78/0x300 sound/core/seq/seq_ports.c:135
[<ffffffff83d1f681>] snd_seq_ioctl_create_port+0xe1/0x2a0 sound/core/seq/seq_clientmgr.c:1324
[<ffffffff83d20e5e>] snd_seq_ioctl+0x13e/0x290 sound/core/seq/seq_clientmgr.c:2327
[<ffffffff81685173>] vfs_ioctl fs/ioctl.c:51 [inline]
[<ffffffff81685173>] __do_sys_ioctl fs/ioctl.c:870 [inline]
[<ffffffff81685173>] __se_sys_ioctl fs/ioctl.c:856 [inline]
[<ffffffff81685173>] __x64_sys_ioctl+0x103/0x140 fs/ioctl.c:856
[<ffffffff84a77ff9>] do_syscall_x64 arch/x86/entry/common.c:50 [inline]
[<ffffffff84a77ff9>] do_syscall_64+0x39/0xb0 arch/x86/entry/common.c:80
[<ffffffff84c0008b>] entry_SYSCALL_64_after_hwframe+0x63/0xcd

BUG: memory leak
unreferenced object 0xffff888106742c00 (size 512):
comm "syz-executor257", pid 5013, jiffies 4294942276 (age 7.450s)
hex dump (first 32 bytes):
80 ff 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
backtrace:
[<ffffffff8154bf94>] kmalloc_trace+0x24/0x90 mm/slab_common.c:1076
[<ffffffff83d29e28>] kmalloc include/linux/slab.h:582 [inline]
[<ffffffff83d29e28>] kzalloc include/linux/slab.h:703 [inline]
[<ffffffff83d29e28>] snd_seq_create_port+0x78/0x300 sound/core/seq/seq_ports.c:135
[<ffffffff83d1f681>] snd_seq_ioctl_create_port+0xe1/0x2a0 sound/core/seq/seq_clientmgr.c:1324
[<ffffffff83d20e5e>] snd_seq_ioctl+0x13e/0x290 sound/core/seq/seq_clientmgr.c:2327
[<ffffffff81685173>] vfs_ioctl fs/ioctl.c:51 [inline]
[<ffffffff81685173>] __do_sys_ioctl fs/ioctl.c:870 [inline]
[<ffffffff81685173>] __se_sys_ioctl fs/ioctl.c:856 [inline]
[<ffffffff81685173>] __x64_sys_ioctl+0x103/0x140 fs/ioctl.c:856
[<ffffffff84a77ff9>] do_syscall_x64 arch/x86/entry/common.c:50 [inline]
[<ffffffff84a77ff9>] do_syscall_64+0x39/0xb0 arch/x86/entry/common.c:80
[<ffffffff84c0008b>] entry_SYSCALL_64_after_hwframe+0x63/0xcd



---
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.

If the bug is already fixed, 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 change bug's subsystems, reply with:
#syz set subsystems: new-subsystem
(See the list of subsystem names on the web dashboard)

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

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

[Takashi Iwai]

Likely a forgotten kfree() at the error path.
The patch below should fix it.


Takashi

-- 8< --
From: Takashi Iwai <ti...@suse.de>
Subject: [PATCH] ALSA: seq: Fix memory leak at error path in
snd_seq_create_port()

We forgot to release a newly allocated item at the error path in
snd_seq_create_port(). This patch fixes it.

Reported-by: syzbot+cf8e7f...@syzkaller.appspotmail.com
Closes: https://lore.kernel.org/r/00000000000098...@google.com
Cc: <sta...@vger.kernel.org>
Signed-off-by: Takashi Iwai <ti...@suse.de>
---
sound/core/seq/seq_ports.c | 1 +
1 file changed, 1 insertion(+)

diff --git a/sound/core/seq/seq_ports.c b/sound/core/seq/seq_ports.c
index 9b80f8275026..f3f14ff0f80f 100644
--- a/sound/core/seq/seq_ports.c
+++ b/sound/core/seq/seq_ports.c
@@ -149,6 +149,7 @@ int snd_seq_create_port(struct snd_seq_client *client, int port,
write_lock_irq(&client->ports_lock);
list_for_each_entry(p, &client->ports_list_head, list) {
if (p->addr.port == port) {
+ kfree(new_port);
num = -EBUSY;
goto unlock;
}
--
2.35.3

[Geraldo Nascimento]

Thanks for the clarification and quick proposed resolution Takashi. As
an ALSA novice these bots always stunt me, personally. I understand how
helpful they are however, even if cryptic.

But shouldn't this be reported to security? It's always prone to bad
stuff when we forget a kfree()

Thanks,
Geraldo Nascimento

[Takashi Iwai]

It's a bug that happened only on 6.5-rc1, so no need to bother too
much with security issue fiasco for distros.


Takashi

[Dmitry Vyukov]

Hi Geraldo,

What exactly is cryptic in the report? Is there anything that can be
done to make it less cryptic?

[Geraldo Nascimento]

On Mon, Jul 17, 2023 at 08:27:48AM +0200, Takashi Iwai wrote:
>
> It's a bug that happened only on 6.5-rc1, so no need to bother too
> much with security issue fiasco for distros.

Thanks Takashi. I tried to create a DoS to exhaust all memory through
this bug but ran on other unrelated issues with 6.5-rc1. Glad syzbot
caught this. Thanks!

Geraldo Nascimento

>
>
> Takashi

[Geraldo Nascimento]

On Mon, Jul 17, 2023 at 09:02:07AM +0200, Dmitry Vyukov wrote:
>
> Hi Geraldo,
>
> What exactly is cryptic in the report? Is there anything that can be
> done to make it less cryptic?
>

Hi Dmitry,

It's cryptic for a novice only, of course, in the same sense that kernel
stack traces are a pain for a novice do decode. Unfortunately I believe
it's only AI/LLMs that will make it easier to abstract the low-level
details and give a high-level explanation of the bug.

Thanks,
Geraldo Nascimento

[Geraldo Nascimento]

On Mon, Jul 17, 2023 at 09:02:07AM +0200, Dmitry Vyukov wrote:
>

> Hi Geraldo,
>
> What exactly is cryptic in the report? Is there anything that can be
> done to make it less cryptic?

Hi again, Dmitry.

Perhaps also a bad choice of words. Cryptic borders on the undecipharable
while esoteric is the more proper word here. Those kernel hackers with
esoteric C and assembly skills like Takashi Iwai or you will quickly
infer that a kfree() is missing in such and such scope.

In my other message, I meant to say that such esoteric knowledge is
barely possessed by a novice kernel hacker, and they end up adding noise
to the lists specially if they are involved in the patch acceptance
process, specially as author of the patch, which I'm neither in this
case.

Now, if somebody were to apply LLMs to the build and checker bots and
actually get to a point where they were getting good patch propositions
from the machine rather than a bunch of hallucinations, that would be
quite the feat. It's only a faint dream right now, but you did
specifically ask for the "vision" :)

Thank you,
Geraldo Nascimento