[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <20221110075142.f6yr7wagqa33rjjn@sgarzare-redhat>
Date: Thu, 10 Nov 2022 08:51:42 +0100
From: Stefano Garzarella <sgarzare@...hat.com>
To: "Michael S. Tsirkin" <mst@...hat.com>
Cc: virtualization@...ts.linux-foundation.org, eperezma@...hat.com,
netdev@...r.kernel.org, kvm@...r.kernel.org,
Gautam Dawar <gautam.dawar@...inx.com>,
Jason Wang <jasowang@...hat.com>, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] vhost-vdpa: fix potential memory leak during the release
On Wed, Nov 09, 2022 at 12:47:19PM -0500, Michael S. Tsirkin wrote:
>On Wed, Nov 09, 2022 at 04:42:13PM +0100, Stefano Garzarella wrote:
>> Before commit 3d5698793897 ("vhost-vdpa: introduce asid based IOTLB")
>> we call vhost_vdpa_iotlb_unmap(v, iotlb, 0ULL, 0ULL - 1) during the
>> release to free all the resources allocated when processing user IOTLB
>> messages through vhost_vdpa_process_iotlb_update().
>> That commit changed the handling of IOTLB a bit, and we accidentally
>> removed some code called during the release.
>>
>> We partially fixed with commit 037d4305569a ("vhost-vdpa: call
>> vhost_vdpa_cleanup during the release") but a potential memory leak is
>> still there as showed by kmemleak if the application does not send
>> VHOST_IOTLB_INVALIDATE or crashes:
>>
>> unreferenced object 0xffff888007fbaa30 (size 16):
>> comm "blkio-bench", pid 914, jiffies 4294993521 (age 885.500s)
>> hex dump (first 16 bytes):
>> 40 73 41 07 80 88 ff ff 00 00 00 00 00 00 00 00 @sA.............
>> backtrace:
>> [<0000000087736d2a>] kmem_cache_alloc_trace+0x142/0x1c0
>> [<0000000060740f50>] vhost_vdpa_process_iotlb_msg+0x68c/0x901 [vhost_vdpa]
>> [<0000000083e8e205>] vhost_chr_write_iter+0xc0/0x4a0 [vhost]
>> [<000000008f2f414a>] vhost_vdpa_chr_write_iter+0x18/0x20 [vhost_vdpa]
>> [<00000000de1cd4a0>] vfs_write+0x216/0x4b0
>> [<00000000a2850200>] ksys_write+0x71/0xf0
>> [<00000000de8e720b>] __x64_sys_write+0x19/0x20
>> [<0000000018b12cbb>] do_syscall_64+0x3f/0x90
>> [<00000000986ec465>] entry_SYSCALL_64_after_hwframe+0x63/0xcd
>>
>> Let's fix calling vhost_vdpa_iotlb_unmap() on the whole range in
>> vhost_vdpa_remove_as(). We move that call before vhost_dev_cleanup()
>> since we need a valid v->vdev.mm in vhost_vdpa_pa_unmap().
>> vhost_iotlb_reset() call can be removed, since vhost_vdpa_iotlb_unmap()
>> on the whole range removes all the entries.
>>
>> The kmemleak log reported was observed with a vDPA device that has `use_va`
>> set to true (e.g. VDUSE). This patch has been tested with both types of
>> devices.
>>
>> Fixes: 037d4305569a ("vhost-vdpa: call vhost_vdpa_cleanup during the release")
>> Fixes: 3d5698793897 ("vhost-vdpa: introduce asid based IOTLB")
>> Signed-off-by: Stefano Garzarella <sgarzare@...hat.com>
>
>It's fine, just pls don't say "potential" here in the subject, let's
>avoid pleonasms
Got it, I'll avoid in the future.
>- it's a memory leak, yes it triggers under some coditions
>but little is unconditional in this world :)
Nothing could be more true :-)
>
>No need to repost.
>
Thanks,
Stefano
Powered by blists - more mailing lists