linux-kernel - Suggestion on how to fix "memory leak in snd_ctl_led

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAD-N9QWYOE5vW=Q4wP74vV-TkrZUDFZxLn5OACX4jUC2ysEACQ@mail.gmail.com>
Date:   Thu, 27 May 2021 10:49:22 +0800
From:   慕冬亮 <mudongliangabcd@...il.com>
To:     Greg KH <gregkh@...uxfoundation.org>, rafael@...nel.org,
        linux-kernel <linux-kernel@...r.kernel.org>
Subject: Suggestion on how to fix "memory leak in snd_ctl_led_register"

Hi kernel developers,

The root cause of "memory leak in snd_ctl_led_register" [1] is
unbalanced refcount operations in the device_add function [2]. This
refcount issue causes struct device_private allocated in
device_private_init unable to be deallocated, leading to memory leak.
The details are as follows:

First, the get_device and put_device are balanced to keep the refcount
unchanged. And there seem no other refcount operations in the
following code. However, in my debugging, the klist_add_tail would
increase the refcount of dev with the get_device in the
klist_chldren_get. This is kind of surprising. Let's go deeper.

int device_add(struct device *dev)
{
......
dev = get_device(dev);
if (!dev)
goto done;

if (!dev->p) {
error = device_private_init(dev);
if (error)
goto done;
}
......
if (parent)
klist_add_tail(&dev->p->knode_parent,
       &parent->p->klist_children);
......
done:
put_device(dev);
return error;
......
}

In the klist_add_tail [3], n is &dev->p->knode_parent, k is
&parent->p->klist_children. Then it goes to klist_node_init. Here, in
my debugging, k->get is klist_children_get. In this function, it calls
get_device to increase the refcount.

void klist_add_tail(struct klist_node *n, struct klist *k)
{
klist_node_init(k, n);
add_tail(k, n);
}

static void klist_node_init(struct klist *k, struct klist_node *n)
{
INIT_LIST_HEAD(&n->n_node);
kref_init(&n->n_ref);
knode_set_klist(n, k);
if (k->get)
k->get(n);
}

static void klist_children_get(struct klist_node *n)
{
struct device_private *p = to_device_private_parent(n);
struct device *dev = p->device;

get_device(dev);
}

It seems that klist_children_get is initialized in device_private_init function.

Finally, I tried some patches, but they are not working. Now I have no
idea how to fix this bug. I will really appreciate if anyone can give
some suggestion on how to fix this bug.

BTW, if there are any issues in the root cause analysis, please let me know.

[1] https://syzkaller.appspot.com/bug?id=6d9e1e89003c894e7a1855c92dfa558ebcb8f218
[2] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/base/core.c?id=e48661230cc35b3d0f4367eddfc19f86463ab917#n3198
[3] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/lib/klist.c?id=e48661230cc35b3d0f4367eddfc19f86463ab917#n134
--
My best regards to you.

     No System Is Safe!
     Dongliang Mu