lkml.org 
[lkml]   [2017]   [Aug]   [22]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
From
SubjectRe:Re: [PATCH] fix memory leak on kvm_vm_ioctl_create_spapr_tce
Date
>On 22.08.2017 17:15, David Hildenbrand wrote:
>> On 22.08.2017 16:28, nixiaoming wrote:
>>> miss kfree(stt) when anon_inode_getfd return fail so add check
>>> anon_inode_getfd return val, and kfree stt
>>>
>>> Signed-off-by: nixiaoming <nixiaoming@huawei.com>
>>> ---
>>> arch/powerpc/kvm/book3s_64_vio.c | 5 ++++-
>>> 1 file changed, 4 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/arch/powerpc/kvm/book3s_64_vio.c
>>> b/arch/powerpc/kvm/book3s_64_vio.c
>>> index a160c14..a0b4459 100644
>>> --- a/arch/powerpc/kvm/book3s_64_vio.c
>>> +++ b/arch/powerpc/kvm/book3s_64_vio.c
>>> @@ -341,8 +341,11 @@ long kvm_vm_ioctl_create_spapr_tce(struct kvm
>>> *kvm,
>>>
>>> mutex_unlock(&kvm->lock);
>>>
>>> - return anon_inode_getfd("kvm-spapr-tce", &kvm_spapr_tce_fops,
>>> + ret = anon_inode_getfd("kvm-spapr-tce", &kvm_spapr_tce_fops,
>>> stt, O_RDWR | O_CLOEXEC);
>>> + if (ret < 0)
>>> + goto fail;
>>> + return ret;
>>>
>>> fail:
>>> if (stt) {
>>>
>>
>>
>> stt has already been added to kvm->arch.spapr_tce_tables, so freeing
>> it is evil IMHO. I don't know that code, so I don't know if there is
>> some other place that will make sure that everything in
>> kvm->arch.spapr_tce_tables will properly get freed, even when no
>> kvm->release
>> function has been called (kvm_spapr_tce_release).
>>
>
>If it is really not freed, than also kvm_put_kvm(stt->kvm) is missing.
>
>--
>
>Thanks,
>
>David
>

if (!stt) return -ENOMEM;
kvm_get_kvm(kvm);
if anon_inode_getfd return -ENOMEM
The user can not determine whether kvm_get_kvm has been called
so need add kvm_pet_kvm when anon_inode_getfd fail

stt has already been added to kvm->arch.spapr_tce_tables,
but if anon_inode_getfd fail, stt is unused val,
so call list_del_rcu, and free as quickly as possible

new patch:

---
arch/powerpc/kvm/book3s_64_vio.c | 10 +++++++++-
1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/kvm/book3s_64_vio.c b/arch/powerpc/kvm/book3s_64_vio.c
index a160c14..e2228f1 100644
--- a/arch/powerpc/kvm/book3s_64_vio.c
+++ b/arch/powerpc/kvm/book3s_64_vio.c
@@ -341,8 +341,16 @@ long kvm_vm_ioctl_create_spapr_tce(struct kvm *kvm,

mutex_unlock(&kvm->lock);

- return anon_inode_getfd("kvm-spapr-tce", &kvm_spapr_tce_fops,
+ ret = anon_inode_getfd("kvm-spapr-tce", &kvm_spapr_tce_fops,
stt, O_RDWR | O_CLOEXEC);
+ if (ret < 0) {
+ mutex_lock(&kvm->lock);
+ list_del_rcu(&stt->list);
+ mutex_unlock(&kvm->lock);
+ kvm_put_kvm(kvm);
+ goto fail;
+ }
+ return ret;

fail:
if (stt) {
--
2.11.0.1



\
 
 \ /
  Last update: 2017-08-23 03:48    [W:0.075 / U:13.444 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site