Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20221110082348.4105476-1-ruanjinjie@huawei.com>
In-Reply-To: <20221110082348.4105476-1-ruanjinjie@huawei.com>
From:   Jason Wang <jasowang@redhat.com>
Date:   Thu, 10 Nov 2022 17:14:12 +0800
Message-ID: <CACGkMEvMuBge8LnQbdQHS86RVx8CkwSyMjR4md7Qs0uranTPuA@mail.gmail.com>
Subject: Re: [PATCH v3] vdpa_sim: fix possible memory leak in
 vdpasim_net_init() and vdpasim_blk_init()
To:     ruanjinjie <ruanjinjie@huawei.com>
Cc:     mst@redhat.com, sgarzare@redhat.com, eperezma@redhat.com,
        gautam.dawar@xilinx.com, elic@nvidia.com,
        virtualization@lists.linux-foundation.org,
        linux-kernel@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Thu, Nov 10, 2022 at 4:27 PM ruanjinjie <ruanjinjie@huawei.com> wrote:
>
> Inject fault while probing module, if device_register() fails in
> vdpasim_net_init() or vdpasim_blk_init(), but the refcount of kobject is
> not decreased to 0, the name allocated in dev_set_name() is leaked.
> Fix this by calling put_device(), so that name can be freed in
> callback function kobject_cleanup().
>
> (vdpa_sim_net)
> unreferenced object 0xffff88807eebc370 (size 16):
>   comm "modprobe", pid 3848, jiffies 4362982860 (age 18.153s)
>   hex dump (first 16 bytes):
>     76 64 70 61 73 69 6d 5f 6e 65 74 00 6b 6b 6b a5  vdpasim_net.kkk.
>   backtrace:
>     [<ffffffff8174f19e>] __kmalloc_node_track_caller+0x4e/0x150
>     [<ffffffff81731d53>] kstrdup+0x33/0x60
>     [<ffffffff83a5d421>] kobject_set_name_vargs+0x41/0x110
>     [<ffffffff82d87aab>] dev_set_name+0xab/0xe0
>     [<ffffffff82d91a23>] device_add+0xe3/0x1a80
>     [<ffffffffa0270013>] 0xffffffffa0270013
>     [<ffffffff81001c27>] do_one_initcall+0x87/0x2e0
>     [<ffffffff813739cb>] do_init_module+0x1ab/0x640
>     [<ffffffff81379d20>] load_module+0x5d00/0x77f0
>     [<ffffffff8137bc40>] __do_sys_finit_module+0x110/0x1b0
>     [<ffffffff83c4d505>] do_syscall_64+0x35/0x80
>     [<ffffffff83e0006a>] entry_SYSCALL_64_after_hwframe+0x46/0xb0
>
> (vdpa_sim_blk)
> unreferenced object 0xffff8881070c1250 (size 16):
>   comm "modprobe", pid 6844, jiffies 4364069319 (age 17.572s)
>   hex dump (first 16 bytes):
>     76 64 70 61 73 69 6d 5f 62 6c 6b 00 6b 6b 6b a5  vdpasim_blk.kkk.
>   backtrace:
>     [<ffffffff8174f19e>] __kmalloc_node_track_caller+0x4e/0x150
>     [<ffffffff81731d53>] kstrdup+0x33/0x60
>     [<ffffffff83a5d421>] kobject_set_name_vargs+0x41/0x110
>     [<ffffffff82d87aab>] dev_set_name+0xab/0xe0
>     [<ffffffff82d91a23>] device_add+0xe3/0x1a80
>     [<ffffffffa0220013>] 0xffffffffa0220013
>     [<ffffffff81001c27>] do_one_initcall+0x87/0x2e0
>     [<ffffffff813739cb>] do_init_module+0x1ab/0x640
>     [<ffffffff81379d20>] load_module+0x5d00/0x77f0
>     [<ffffffff8137bc40>] __do_sys_finit_module+0x110/0x1b0
>     [<ffffffff83c4d505>] do_syscall_64+0x35/0x80
>     [<ffffffff83e0006a>] entry_SYSCALL_64_after_hwframe+0x46/0xb0
>
> Fixes: 899c4d187f6a ("vdpa_sim_blk: add support for vdpa management tool")
> Fixes: a3c06ae158dd ("vdpa_sim_net: Add support for user supported devices")
>
> Signed-off-by: ruanjinjie <ruanjinjie@huawei.com>
> Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>

Acked-by: Jason Wang <jasowang@redhat.com>

Thanks

> ---
> v3:
> - add fix tag
> v2:
> - add fault inject message
> ---
>  drivers/vdpa/vdpa_sim/vdpa_sim_blk.c | 4 +++-
>  drivers/vdpa/vdpa_sim/vdpa_sim_net.c | 4 +++-
>  2 files changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/vdpa/vdpa_sim/vdpa_sim_blk.c b/drivers/vdpa/vdpa_sim/vdpa_sim_blk.c
> index c6db1a1baf76..f745926237a8 100644
> --- a/drivers/vdpa/vdpa_sim/vdpa_sim_blk.c
> +++ b/drivers/vdpa/vdpa_sim/vdpa_sim_blk.c
> @@ -427,8 +427,10 @@ static int __init vdpasim_blk_init(void)
>         int ret;
>
>         ret = device_register(&vdpasim_blk_mgmtdev);
> -       if (ret)
> +       if (ret) {
> +               put_device(&vdpasim_blk_mgmtdev);
>                 return ret;
> +       }
>
>         ret = vdpa_mgmtdev_register(&mgmt_dev);
>         if (ret)
> diff --git a/drivers/vdpa/vdpa_sim/vdpa_sim_net.c b/drivers/vdpa/vdpa_sim/vdpa_sim_net.c
> index c3cb225ea469..11f5a121df24 100644
> --- a/drivers/vdpa/vdpa_sim/vdpa_sim_net.c
> +++ b/drivers/vdpa/vdpa_sim/vdpa_sim_net.c
> @@ -305,8 +305,10 @@ static int __init vdpasim_net_init(void)
>         int ret;
>
>         ret = device_register(&vdpasim_net_mgmtdev);
> -       if (ret)
> +       if (ret) {
> +               put_device(&vdpasim_net_mgmtdev);
>                 return ret;
> +       }
>
>         ret = vdpa_mgmtdev_register(&mgmt_dev);
>         if (ret)
> --
> 2.25.1
>