* [PATCH] fix memory leak on kvm_vm_ioctl_get_htab_fd
@ 2017-08-22 14:18 ` nixiaoming
0 siblings, 0 replies; 35+ messages in thread
From: nixiaoming @ 2017-08-22 14:18 UTC (permalink / raw)
To: agraf, pbonzini, rkrcmar, benh, paulus, mpe
Cc: kvm-ppc, kvm, linuxppc-dev, linux-kernel
ctx = kzalloc(sizeof(*ctx), GFP_KERNEL);
but no free when anon_inode_getfd return fail
so, add kfree(ctx) to fix memory leak
Signed-off-by: nixiaoming <nixiaoming@huawei.com>
---
arch/powerpc/kvm/book3s_64_mmu_hv.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/powerpc/kvm/book3s_64_mmu_hv.c b/arch/powerpc/kvm/book3s_64_mmu_hv.c
index b42812e..be3d08f 100644
--- a/arch/powerpc/kvm/book3s_64_mmu_hv.c
+++ b/arch/powerpc/kvm/book3s_64_mmu_hv.c
@@ -1940,6 +1940,7 @@ int kvm_vm_ioctl_get_htab_fd(struct kvm *kvm, struct kvm_get_htab_fd *ghf)
rwflag = (ghf->flags & KVM_GET_HTAB_WRITE) ? O_WRONLY : O_RDONLY;
ret = anon_inode_getfd("kvm-htab", &kvm_htab_fops, ctx, rwflag | O_CLOEXEC);
if (ret < 0) {
+ kfree(ctx);
kvm_put_kvm(kvm);
return ret;
}
--
2.11.0.1
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [PATCH] fix memory leak on kvm_vm_ioctl_get_htab_fd
@ 2017-08-22 14:18 ` nixiaoming
0 siblings, 0 replies; 35+ messages in thread
From: nixiaoming @ 2017-08-22 14:18 UTC (permalink / raw)
To: agraf, pbonzini, rkrcmar, benh, paulus, mpe
Cc: kvm-ppc, kvm, linuxppc-dev, linux-kernel
ctx = kzalloc(sizeof(*ctx), GFP_KERNEL);
but no free when anon_inode_getfd return fail
so, add kfree(ctx) to fix memory leak
Signed-off-by: nixiaoming <nixiaoming@huawei.com>
---
arch/powerpc/kvm/book3s_64_mmu_hv.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/powerpc/kvm/book3s_64_mmu_hv.c b/arch/powerpc/kvm/book3s_64_mmu_hv.c
index b42812e..be3d08f 100644
--- a/arch/powerpc/kvm/book3s_64_mmu_hv.c
+++ b/arch/powerpc/kvm/book3s_64_mmu_hv.c
@@ -1940,6 +1940,7 @@ int kvm_vm_ioctl_get_htab_fd(struct kvm *kvm, struct kvm_get_htab_fd *ghf)
rwflag = (ghf->flags & KVM_GET_HTAB_WRITE) ? O_WRONLY : O_RDONLY;
ret = anon_inode_getfd("kvm-htab", &kvm_htab_fops, ctx, rwflag | O_CLOEXEC);
if (ret < 0) {
+ kfree(ctx);
kvm_put_kvm(kvm);
return ret;
}
--
2.11.0.1
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [PATCH] fix memory leak on kvm_vm_ioctl_create_spapr_tce
2017-08-22 14:18 ` nixiaoming
@ 2017-08-22 14:28 ` nixiaoming
-1 siblings, 0 replies; 35+ messages in thread
From: nixiaoming @ 2017-08-22 14:28 UTC (permalink / raw)
To: agraf, pbonzini, rkrcmar, benh, paulus, mpe
Cc: kvm-ppc, kvm, linuxppc-dev, linux-kernel
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) {
--
2.11.0.1
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [PATCH] fix memory leak on kvm_vm_ioctl_create_spapr_tce
@ 2017-08-22 14:28 ` nixiaoming
0 siblings, 0 replies; 35+ messages in thread
From: nixiaoming @ 2017-08-22 14:28 UTC (permalink / raw)
To: agraf, pbonzini, rkrcmar, benh, paulus, mpe
Cc: kvm-ppc, kvm, linuxppc-dev, linux-kernel
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) {
--
2.11.0.1
^ permalink raw reply related [flat|nested] 35+ messages in thread
* Re: [PATCH] fix memory leak on kvm_vm_ioctl_create_spapr_tce
2017-08-22 14:28 ` nixiaoming
@ 2017-08-22 15:15 ` David Hildenbrand
-1 siblings, 0 replies; 35+ messages in thread
From: David Hildenbrand @ 2017-08-22 15:15 UTC (permalink / raw)
To: nixiaoming, agraf, pbonzini, rkrcmar, benh, paulus, mpe
Cc: kvm-ppc, kvm, linuxppc-dev, linux-kernel
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 release
function has been called (kvm_spapr_tce_release).
--
Thanks,
David
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH] fix memory leak on kvm_vm_ioctl_create_spapr_tce
@ 2017-08-22 15:15 ` David Hildenbrand
0 siblings, 0 replies; 35+ messages in thread
From: David Hildenbrand @ 2017-08-22 15:15 UTC (permalink / raw)
To: nixiaoming, agraf, pbonzini, rkrcmar, benh, paulus, mpe
Cc: kvm-ppc, kvm, linuxppc-dev, linux-kernel
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 release
function has been called (kvm_spapr_tce_release).
--
Thanks,
David
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH] fix memory leak on kvm_vm_ioctl_create_spapr_tce
2017-08-22 15:15 ` David Hildenbrand
@ 2017-08-22 15:23 ` David Hildenbrand
-1 siblings, 0 replies; 35+ messages in thread
From: David Hildenbrand @ 2017-08-22 15:23 UTC (permalink / raw)
To: nixiaoming, agraf, pbonzini, rkrcmar, benh, paulus, mpe
Cc: kvm-ppc, kvm, linuxppc-dev, linux-kernel
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 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
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH] fix memory leak on kvm_vm_ioctl_create_spapr_tce
@ 2017-08-22 15:23 ` David Hildenbrand
0 siblings, 0 replies; 35+ messages in thread
From: David Hildenbrand @ 2017-08-22 15:23 UTC (permalink / raw)
To: nixiaoming, agraf, pbonzini, rkrcmar, benh, paulus, mpe
Cc: kvm-ppc, kvm, linuxppc-dev, linux-kernel
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 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
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH] fix memory leak on kvm_vm_ioctl_get_htab_fd
2017-08-22 14:18 ` nixiaoming
@ 2017-08-22 15:51 ` Paolo Bonzini
-1 siblings, 0 replies; 35+ messages in thread
From: Paolo Bonzini @ 2017-08-22 15:51 UTC (permalink / raw)
To: nixiaoming, agraf, rkrcmar, benh, paulus, mpe
Cc: kvm-ppc, kvm, linuxppc-dev, linux-kernel
On 22/08/2017 16:18, nixiaoming wrote:
> ctx = kzalloc(sizeof(*ctx), GFP_KERNEL);
> but no free when anon_inode_getfd return fail
> so, add kfree(ctx) to fix memory leak
>
> Signed-off-by: nixiaoming <nixiaoming@huawei.com>
> ---
> arch/powerpc/kvm/book3s_64_mmu_hv.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/arch/powerpc/kvm/book3s_64_mmu_hv.c b/arch/powerpc/kvm/book3s_64_mmu_hv.c
> index b42812e..be3d08f 100644
> --- a/arch/powerpc/kvm/book3s_64_mmu_hv.c
> +++ b/arch/powerpc/kvm/book3s_64_mmu_hv.c
> @@ -1940,6 +1940,7 @@ int kvm_vm_ioctl_get_htab_fd(struct kvm *kvm, struct kvm_get_htab_fd *ghf)
> rwflag = (ghf->flags & KVM_GET_HTAB_WRITE) ? O_WRONLY : O_RDONLY;
> ret = anon_inode_getfd("kvm-htab", &kvm_htab_fops, ctx, rwflag | O_CLOEXEC);
> if (ret < 0) {
> + kfree(ctx);
> kvm_put_kvm(kvm);
> return ret;
> }
>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH] fix memory leak on kvm_vm_ioctl_get_htab_fd
@ 2017-08-22 15:51 ` Paolo Bonzini
0 siblings, 0 replies; 35+ messages in thread
From: Paolo Bonzini @ 2017-08-22 15:51 UTC (permalink / raw)
To: nixiaoming, agraf, rkrcmar, benh, paulus, mpe
Cc: kvm-ppc, kvm, linuxppc-dev, linux-kernel
On 22/08/2017 16:18, nixiaoming wrote:
> ctx = kzalloc(sizeof(*ctx), GFP_KERNEL);
> but no free when anon_inode_getfd return fail
> so, add kfree(ctx) to fix memory leak
>
> Signed-off-by: nixiaoming <nixiaoming@huawei.com>
> ---
> arch/powerpc/kvm/book3s_64_mmu_hv.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/arch/powerpc/kvm/book3s_64_mmu_hv.c b/arch/powerpc/kvm/book3s_64_mmu_hv.c
> index b42812e..be3d08f 100644
> --- a/arch/powerpc/kvm/book3s_64_mmu_hv.c
> +++ b/arch/powerpc/kvm/book3s_64_mmu_hv.c
> @@ -1940,6 +1940,7 @@ int kvm_vm_ioctl_get_htab_fd(struct kvm *kvm, struct kvm_get_htab_fd *ghf)
> rwflag = (ghf->flags & KVM_GET_HTAB_WRITE) ? O_WRONLY : O_RDONLY;
> ret = anon_inode_getfd("kvm-htab", &kvm_htab_fops, ctx, rwflag | O_CLOEXEC);
> if (ret < 0) {
> + kfree(ctx);
> kvm_put_kvm(kvm);
> return ret;
> }
>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re:Re: [PATCH] fix memory leak on kvm_vm_ioctl_create_spapr_tce
2017-08-22 15:23 ` David Hildenbrand
(?)
@ 2017-08-23 1:43 ` Nixiaoming
-1 siblings, 0 replies; 35+ messages in thread
From: Nixiaoming @ 2017-08-23 1:43 UTC (permalink / raw)
To: David Hildenbrand, agraf, pbonzini, rkrcmar, benh, paulus, mpe
Cc: kvm-ppc, kvm, linuxppc-dev, linux-kernel
>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
^ permalink raw reply related [flat|nested] 35+ messages in thread
* Re:Re: [PATCH] fix memory leak on kvm_vm_ioctl_create_spapr_tce
@ 2017-08-23 1:43 ` Nixiaoming
0 siblings, 0 replies; 35+ messages in thread
From: Nixiaoming @ 2017-08-23 1:43 UTC (permalink / raw)
To: David Hildenbrand, agraf, pbonzini, rkrcmar, benh, paulus, mpe
Cc: kvm-ppc, kvm, linuxppc-dev, linux-kernel
Pk9uIDIyLjA4LjIwMTcgMTc6MTUsIERhdmlkIEhpbGRlbmJyYW5kIHdyb3RlOg0KPj4gT24gMjIu
MDguMjAxNyAxNjoyOCwgbml4aWFvbWluZyB3cm90ZToNCj4+PiBtaXNzIGtmcmVlKHN0dCkgd2hl
biBhbm9uX2lub2RlX2dldGZkIHJldHVybiBmYWlsIHNvIGFkZCBjaGVjayANCj4+PiBhbm9uX2lu
b2RlX2dldGZkIHJldHVybiB2YWwsIGFuZCBrZnJlZSBzdHQNCj4+Pg0KPj4+IFNpZ25lZC1vZmYt
Ynk6IG5peGlhb21pbmcgPG5peGlhb21pbmdAaHVhd2VpLmNvbT4NCj4+PiAtLS0NCj4+PiAgYXJj
aC9wb3dlcnBjL2t2bS9ib29rM3NfNjRfdmlvLmMgfCA1ICsrKystDQo+Pj4gIDEgZmlsZSBjaGFu
Z2VkLCA0IGluc2VydGlvbnMoKyksIDEgZGVsZXRpb24oLSkNCj4+Pg0KPj4+IGRpZmYgLS1naXQg
YS9hcmNoL3Bvd2VycGMva3ZtL2Jvb2szc182NF92aW8uYyANCj4+PiBiL2FyY2gvcG93ZXJwYy9r
dm0vYm9vazNzXzY0X3Zpby5jDQo+Pj4gaW5kZXggYTE2MGMxNC4uYTBiNDQ1OSAxMDA2NDQNCj4+
PiAtLS0gYS9hcmNoL3Bvd2VycGMva3ZtL2Jvb2szc182NF92aW8uYw0KPj4+ICsrKyBiL2FyY2gv
cG93ZXJwYy9rdm0vYm9vazNzXzY0X3Zpby5jDQo+Pj4gQEAgLTM0MSw4ICszNDEsMTEgQEAgbG9u
ZyBrdm1fdm1faW9jdGxfY3JlYXRlX3NwYXByX3RjZShzdHJ1Y3Qga3ZtIA0KPj4+ICprdm0sDQo+
Pj4gIA0KPj4+ICAJbXV0ZXhfdW5sb2NrKCZrdm0tPmxvY2spOw0KPj4+ICANCj4+PiAtCXJldHVy
biBhbm9uX2lub2RlX2dldGZkKCJrdm0tc3BhcHItdGNlIiwgJmt2bV9zcGFwcl90Y2VfZm9wcywN
Cj4+PiArCXJldCA9IGFub25faW5vZGVfZ2V0ZmQoImt2bS1zcGFwci10Y2UiLCAma3ZtX3NwYXBy
X3RjZV9mb3BzLA0KPj4+ICAJCQkJc3R0LCBPX1JEV1IgfCBPX0NMT0VYRUMpOw0KPj4+ICsJaWYg
KHJldCA8IDApDQo+Pj4gKwkJZ290byBmYWlsOw0KPj4+ICsJcmV0dXJuIHJldDsNCj4+PiAgDQo+
Pj4gIGZhaWw6DQo+Pj4gIAlpZiAoc3R0KSB7DQo+Pj4NCj4+IA0KPj4gDQo+PiBzdHQgaGFzIGFs
cmVhZHkgYmVlbiBhZGRlZCB0byBrdm0tPmFyY2guc3BhcHJfdGNlX3RhYmxlcywgc28gZnJlZWlu
ZyANCj4+IGl0IGlzIGV2aWwgSU1ITy4gSSBkb24ndCBrbm93IHRoYXQgY29kZSwgc28gSSBkb24n
dCBrbm93IGlmIHRoZXJlIGlzIA0KPj4gc29tZSBvdGhlciBwbGFjZSB0aGF0IHdpbGwgbWFrZSBz
dXJlIHRoYXQgZXZlcnl0aGluZyBpbg0KPj4ga3ZtLT5hcmNoLnNwYXByX3RjZV90YWJsZXMgd2ls
bCBwcm9wZXJseSBnZXQgZnJlZWQsIGV2ZW4gd2hlbiBubyANCj4+IGt2bS0+cmVsZWFzZQ0KPj4g
ZnVuY3Rpb24gaGFzIGJlZW4gY2FsbGVkIChrdm1fc3BhcHJfdGNlX3JlbGVhc2UpLg0KPj4gDQo+
DQo+SWYgaXQgaXMgcmVhbGx5IG5vdCBmcmVlZCwgdGhhbiBhbHNvIGt2bV9wdXRfa3ZtKHN0dC0+
a3ZtKSBpcyBtaXNzaW5nLg0KPg0KPi0tIA0KPg0KPlRoYW5rcywNCj4NCj5EYXZpZA0KPg0KDQpp
ZiAoIXN0dCkgcmV0dXJuIC1FTk9NRU07DQprdm1fZ2V0X2t2bShrdm0pOw0KaWYgYW5vbl9pbm9k
ZV9nZXRmZCByZXR1cm4gLUVOT01FTQ0KVGhlIHVzZXIgY2FuIG5vdCBkZXRlcm1pbmUgd2hldGhl
ciBrdm1fZ2V0X2t2bSBoYXMgYmVlbiBjYWxsZWQNCnNvIG5lZWQgYWRkIGt2bV9wZXRfa3ZtIHdo
ZW4gYW5vbl9pbm9kZV9nZXRmZCBmYWlsDQoNCnN0dCBoYXMgYWxyZWFkeSBiZWVuIGFkZGVkIHRv
IGt2bS0+YXJjaC5zcGFwcl90Y2VfdGFibGVzLA0KYnV0IGlmIGFub25faW5vZGVfZ2V0ZmQgZmFp
bCwgc3R0IGlzIHVudXNlZCB2YWwsIA0Kc28gY2FsbCBsaXN0X2RlbF9yY3UsIGFuZCAgZnJlZSBh
cyBxdWlja2x5IGFzIHBvc3NpYmxlDQoNCm5ldyBwYXRjaDoNCg0KLS0tDQogYXJjaC9wb3dlcnBj
L2t2bS9ib29rM3NfNjRfdmlvLmMgfCAxMCArKysrKysrKystDQogMSBmaWxlIGNoYW5nZWQsIDkg
aW5zZXJ0aW9ucygrKSwgMSBkZWxldGlvbigtKQ0KDQpkaWZmIC0tZ2l0IGEvYXJjaC9wb3dlcnBj
L2t2bS9ib29rM3NfNjRfdmlvLmMgYi9hcmNoL3Bvd2VycGMva3ZtL2Jvb2szc182NF92aW8uYw0K
aW5kZXggYTE2MGMxNC4uZTIyMjhmMSAxMDA2NDQNCi0tLSBhL2FyY2gvcG93ZXJwYy9rdm0vYm9v
azNzXzY0X3Zpby5jDQorKysgYi9hcmNoL3Bvd2VycGMva3ZtL2Jvb2szc182NF92aW8uYw0KQEAg
LTM0MSw4ICszNDEsMTYgQEAgbG9uZyBrdm1fdm1faW9jdGxfY3JlYXRlX3NwYXByX3RjZShzdHJ1
Y3Qga3ZtICprdm0sDQoNCiAgICAgICAgbXV0ZXhfdW5sb2NrKCZrdm0tPmxvY2spOw0KDQotICAg
ICAgIHJldHVybiBhbm9uX2lub2RlX2dldGZkKCJrdm0tc3BhcHItdGNlIiwgJmt2bV9zcGFwcl90
Y2VfZm9wcywNCisgICAgICAgcmV0ID0gYW5vbl9pbm9kZV9nZXRmZCgia3ZtLXNwYXByLXRjZSIs
ICZrdm1fc3BhcHJfdGNlX2ZvcHMsDQogICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgIHN0
dCwgT19SRFdSIHwgT19DTE9FWEVDKTsNCisgICAgICAgaWYgKHJldCA8IDApIHsNCisgICAgICAg
ICAgICAgICBtdXRleF9sb2NrKCZrdm0tPmxvY2spOw0KKyAgICAgICAgICAgICAgIGxpc3RfZGVs
X3JjdSgmc3R0LT5saXN0KTsNCisgICAgICAgICAgICAgICBtdXRleF91bmxvY2soJmt2bS0+bG9j
ayk7DQorICAgICAgICAgICAgICAga3ZtX3B1dF9rdm0oa3ZtKTsNCisgICAgICAgICAgICAgICBn
b3RvIGZhaWw7DQorICAgICAgIH0NCisgICAgICAgcmV0dXJuIHJldDsNCg0KIGZhaWw6DQogICAg
ICAgIGlmIChzdHQpIHsNCi0tIA0KMi4xMS4wLjENCg0KDQoNCg==
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re:Re: [PATCH] fix memory leak on kvm_vm_ioctl_create_spapr_tce
@ 2017-08-23 1:43 ` Nixiaoming
0 siblings, 0 replies; 35+ messages in thread
From: Nixiaoming @ 2017-08-23 1:43 UTC (permalink / raw)
To: David Hildenbrand, agraf, pbonzini, rkrcmar, benh, paulus, mpe
Cc: kvm-ppc, kvm, linuxppc-dev, linux-kernel
Pk9uIDIyLjA4LjIwMTcgMTc6MTUsIERhdmlkIEhpbGRlbmJyYW5kIHdyb3RlOg0KPj4gT24gMjIu
MDguMjAxNyAxNjoyOCwgbml4aWFvbWluZyB3cm90ZToNCj4+PiBtaXNzIGtmcmVlKHN0dCkgd2hl
biBhbm9uX2lub2RlX2dldGZkIHJldHVybiBmYWlsIHNvIGFkZCBjaGVjayANCj4+PiBhbm9uX2lu
b2RlX2dldGZkIHJldHVybiB2YWwsIGFuZCBrZnJlZSBzdHQNCj4+Pg0KPj4+IFNpZ25lZC1vZmYt
Ynk6IG5peGlhb21pbmcgPG5peGlhb21pbmdAaHVhd2VpLmNvbT4NCj4+PiAtLS0NCj4+PiAgYXJj
aC9wb3dlcnBjL2t2bS9ib29rM3NfNjRfdmlvLmMgfCA1ICsrKystDQo+Pj4gIDEgZmlsZSBjaGFu
Z2VkLCA0IGluc2VydGlvbnMoKyksIDEgZGVsZXRpb24oLSkNCj4+Pg0KPj4+IGRpZmYgLS1naXQg
YS9hcmNoL3Bvd2VycGMva3ZtL2Jvb2szc182NF92aW8uYyANCj4+PiBiL2FyY2gvcG93ZXJwYy9r
dm0vYm9vazNzXzY0X3Zpby5jDQo+Pj4gaW5kZXggYTE2MGMxNC4uYTBiNDQ1OSAxMDA2NDQNCj4+
PiAtLS0gYS9hcmNoL3Bvd2VycGMva3ZtL2Jvb2szc182NF92aW8uYw0KPj4+ICsrKyBiL2FyY2gv
cG93ZXJwYy9rdm0vYm9vazNzXzY0X3Zpby5jDQo+Pj4gQEAgLTM0MSw4ICszNDEsMTEgQEAgbG9u
ZyBrdm1fdm1faW9jdGxfY3JlYXRlX3NwYXByX3RjZShzdHJ1Y3Qga3ZtIA0KPj4+ICprdm0sDQo+
Pj4gIA0KPj4+ICAJbXV0ZXhfdW5sb2NrKCZrdm0tPmxvY2spOw0KPj4+ICANCj4+PiAtCXJldHVy
biBhbm9uX2lub2RlX2dldGZkKCJrdm0tc3BhcHItdGNlIiwgJmt2bV9zcGFwcl90Y2VfZm9wcywN
Cj4+PiArCXJldCA9IGFub25faW5vZGVfZ2V0ZmQoImt2bS1zcGFwci10Y2UiLCAma3ZtX3NwYXBy
X3RjZV9mb3BzLA0KPj4+ICAJCQkJc3R0LCBPX1JEV1IgfCBPX0NMT0VYRUMpOw0KPj4+ICsJaWYg
KHJldCA8IDApDQo+Pj4gKwkJZ290byBmYWlsOw0KPj4+ICsJcmV0dXJuIHJldDsNCj4+PiAgDQo+
Pj4gIGZhaWw6DQo+Pj4gIAlpZiAoc3R0KSB7DQo+Pj4NCj4+IA0KPj4gDQo+PiBzdHQgaGFzIGFs
cmVhZHkgYmVlbiBhZGRlZCB0byBrdm0tPmFyY2guc3BhcHJfdGNlX3RhYmxlcywgc28gZnJlZWlu
ZyANCj4+IGl0IGlzIGV2aWwgSU1ITy4gSSBkb24ndCBrbm93IHRoYXQgY29kZSwgc28gSSBkb24n
dCBrbm93IGlmIHRoZXJlIGlzIA0KPj4gc29tZSBvdGhlciBwbGFjZSB0aGF0IHdpbGwgbWFrZSBz
dXJlIHRoYXQgZXZlcnl0aGluZyBpbg0KPj4ga3ZtLT5hcmNoLnNwYXByX3RjZV90YWJsZXMgd2ls
bCBwcm9wZXJseSBnZXQgZnJlZWQsIGV2ZW4gd2hlbiBubyANCj4+IGt2bS0+cmVsZWFzZQ0KPj4g
ZnVuY3Rpb24gaGFzIGJlZW4gY2FsbGVkIChrdm1fc3BhcHJfdGNlX3JlbGVhc2UpLg0KPj4gDQo+
DQo+SWYgaXQgaXMgcmVhbGx5IG5vdCBmcmVlZCwgdGhhbiBhbHNvIGt2bV9wdXRfa3ZtKHN0dC0+
a3ZtKSBpcyBtaXNzaW5nLg0KPg0KPi0tIA0KPg0KPlRoYW5rcywNCj4NCj5EYXZpZA0KPg0KDQpp
ZiAoIXN0dCkgcmV0dXJuIC1FTk9NRU07DQprdm1fZ2V0X2t2bShrdm0pOw0KaWYgYW5vbl9pbm9k
ZV9nZXRmZCByZXR1cm4gLUVOT01FTQ0KVGhlIHVzZXIgY2FuIG5vdCBkZXRlcm1pbmUgd2hldGhl
ciBrdm1fZ2V0X2t2bSBoYXMgYmVlbiBjYWxsZWQNCnNvIG5lZWQgYWRkIGt2bV9wZXRfa3ZtIHdo
ZW4gYW5vbl9pbm9kZV9nZXRmZCBmYWlsDQoNCnN0dCBoYXMgYWxyZWFkeSBiZWVuIGFkZGVkIHRv
IGt2bS0+YXJjaC5zcGFwcl90Y2VfdGFibGVzLA0KYnV0IGlmIGFub25faW5vZGVfZ2V0ZmQgZmFp
bCwgc3R0IGlzIHVudXNlZCB2YWwsIA0Kc28gY2FsbCBsaXN0X2RlbF9yY3UsIGFuZCAgZnJlZSBh
cyBxdWlja2x5IGFzIHBvc3NpYmxlDQoNCm5ldyBwYXRjaDoNCg0KLS0tDQogYXJjaC9wb3dlcnBj
L2t2bS9ib29rM3NfNjRfdmlvLmMgfCAxMCArKysrKysrKystDQogMSBmaWxlIGNoYW5nZWQsIDkg
aW5zZXJ0aW9ucygrKSwgMSBkZWxldGlvbigtKQ0KDQpkaWZmIC0tZ2l0IGEvYXJjaC9wb3dlcnBj
L2t2bS9ib29rM3NfNjRfdmlvLmMgYi9hcmNoL3Bvd2VycGMva3ZtL2Jvb2szc182NF92aW8uYw0K
aW5kZXggYTE2MGMxNC4uZTIyMjhmMSAxMDA2NDQNCi0tLSBhL2FyY2gvcG93ZXJwYy9rdm0vYm9v
azNzXzY0X3Zpby5jDQorKysgYi9hcmNoL3Bvd2VycGMva3ZtL2Jvb2szc182NF92aW8uYw0KQEAg
LTM0MSw4ICszNDEsMTYgQEAgbG9uZyBrdm1fdm1faW9jdGxfY3JlYXRlX3NwYXByX3RjZShzdHJ1
Y3Qga3ZtICprdm0sDQoNCiAgICAgICAgbXV0ZXhfdW5sb2NrKCZrdm0tPmxvY2spOw0KDQotICAg
ICAgIHJldHVybiBhbm9uX2lub2RlX2dldGZkKCJrdm0tc3BhcHItdGNlIiwgJmt2bV9zcGFwcl90
Y2VfZm9wcywNCisgICAgICAgcmV0ID0gYW5vbl9pbm9kZV9nZXRmZCgia3ZtLXNwYXByLXRjZSIs
ICZrdm1fc3BhcHJfdGNlX2ZvcHMsDQogICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgIHN0
dCwgT19SRFdSIHwgT19DTE9FWEVDKTsNCisgICAgICAgaWYgKHJldCA8IDApIHsNCisgICAgICAg
ICAgICAgICBtdXRleF9sb2NrKCZrdm0tPmxvY2spOw0KKyAgICAgICAgICAgICAgIGxpc3RfZGVs
X3JjdSgmc3R0LT5saXN0KTsNCisgICAgICAgICAgICAgICBtdXRleF91bmxvY2soJmt2bS0+bG9j
ayk7DQorICAgICAgICAgICAgICAga3ZtX3B1dF9rdm0oa3ZtKTsNCisgICAgICAgICAgICAgICBn
b3RvIGZhaWw7DQorICAgICAgIH0NCisgICAgICAgcmV0dXJuIHJldDsNCg0KIGZhaWw6DQogICAg
ICAgIGlmIChzdHQpIHsNCi0tIA0KMi4xMS4wLjENCg0KDQoNCg=
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: Re: [PATCH] fix memory leak on kvm_vm_ioctl_create_spapr_tce
2017-08-23 1:43 ` Nixiaoming
@ 2017-08-23 6:06 ` Paul Mackerras
-1 siblings, 0 replies; 35+ messages in thread
From: Paul Mackerras @ 2017-08-23 6:06 UTC (permalink / raw)
To: Nixiaoming
Cc: David Hildenbrand, agraf, pbonzini, rkrcmar, benh, mpe, kvm-ppc,
kvm, linuxppc-dev, linux-kernel
On Wed, Aug 23, 2017 at 01:43:08AM +0000, Nixiaoming wrote:
> >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;
It seems to me that it would be better to do the anon_inode_getfd()
call before the kvm_get_kvm() call, and go to the fail label if it
fails.
Paul.
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: Re: [PATCH] fix memory leak on kvm_vm_ioctl_create_spapr_tce
@ 2017-08-23 6:06 ` Paul Mackerras
0 siblings, 0 replies; 35+ messages in thread
From: Paul Mackerras @ 2017-08-23 6:06 UTC (permalink / raw)
To: Nixiaoming
Cc: David Hildenbrand, agraf, pbonzini, rkrcmar, benh, mpe, kvm-ppc,
kvm, linuxppc-dev, linux-kernel
On Wed, Aug 23, 2017 at 01:43:08AM +0000, Nixiaoming wrote:
> >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;
It seems to me that it would be better to do the anon_inode_getfd()
call before the kvm_get_kvm() call, and go to the fail label if it
fails.
Paul.
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH] fix memory leak on kvm_vm_ioctl_create_spapr_tce
2017-08-23 6:06 ` Paul Mackerras
@ 2017-08-23 8:25 ` David Hildenbrand
-1 siblings, 0 replies; 35+ messages in thread
From: David Hildenbrand @ 2017-08-23 8:25 UTC (permalink / raw)
To: Paul Mackerras, Nixiaoming
Cc: agraf, pbonzini, rkrcmar, benh, mpe, kvm-ppc, kvm, linuxppc-dev,
linux-kernel
On 23.08.2017 08:06, Paul Mackerras wrote:
> On Wed, Aug 23, 2017 at 01:43:08AM +0000, Nixiaoming wrote:
>>> 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);
... don't we have to take care of rcu synchronization before freeing it?
>> + mutex_unlock(&kvm->lock);
>> + kvm_put_kvm(kvm);
>> + goto fail;
>> + }
>> + return ret;
of simply
if (!ret)
return 0;
mutex_lock(&kvm->lock);
list_del_rcu(&stt->list);
mutex_unlock(&kvm->lock);
kvm_put_kvm(kvm);
>
> It seems to me that it would be better to do the anon_inode_getfd()
> call before the kvm_get_kvm() call, and go to the fail label if it
> fails.
I would have suggested to not add it to the list before it has been
fully created (so nobody can have access to it). But I guess than we
need another level of protection(e.g. kvm->lock).
Am I missing something, or is kvm_vm_ioctl_create_spapr_tce() racy?
The -EBUSY check is done without any locking, so two parallel creators
could create an inconsistency, no? Shouldn't this all be protected by
kvm->lock?
>
> Paul.
>
Independent of the fix, I'd suggest the following cleanup.
>From 979f55083ee965e25827a8743e8a9fdb85231a6f Mon Sep 17 00:00:00 2001
From: David Hildenbrand <david@redhat.com>
Date: Wed, 23 Aug 2017 10:08:58 +0200
Subject: [PATCH v1 1/1] KVM: PPC: cleanup kvm_vm_ioctl_create_spapr_tce
Let's simplify error handling.
Signed-off-by: David Hildenbrand <david@redhat.com>
---
arch/powerpc/kvm/book3s_64_vio.c | 29 +++++++++++------------------
1 file changed, 11 insertions(+), 18 deletions(-)
diff --git a/arch/powerpc/kvm/book3s_64_vio.c
b/arch/powerpc/kvm/book3s_64_vio.c
index a160c14304eb..6bac49292296 100644
--- a/arch/powerpc/kvm/book3s_64_vio.c
+++ b/arch/powerpc/kvm/book3s_64_vio.c
@@ -295,8 +295,7 @@ long kvm_vm_ioctl_create_spapr_tce(struct kvm *kvm,
{
struct kvmppc_spapr_tce_table *stt = NULL;
unsigned long npages, size;
- int ret = -ENOMEM;
- int i;
+ int i, ret;
if (!args->size)
return -EINVAL;
@@ -310,16 +309,13 @@ long kvm_vm_ioctl_create_spapr_tce(struct kvm *kvm,
size = _ALIGN_UP(args->size, PAGE_SIZE >> 3);
npages = kvmppc_tce_pages(size);
ret = kvmppc_account_memlimit(kvmppc_stt_pages(npages), true);
- if (ret) {
- stt = NULL;
- goto fail;
- }
+ if (ret)
+ return ret;
- ret = -ENOMEM;
stt = kzalloc(sizeof(*stt) + npages * sizeof(struct page *),
GFP_KERNEL);
if (!stt)
- goto fail;
+ return -ENOMEM;
stt->liobn = args->liobn;
stt->page_shift = args->page_shift;
@@ -331,7 +327,7 @@ long kvm_vm_ioctl_create_spapr_tce(struct kvm *kvm,
for (i = 0; i < npages; i++) {
stt->pages[i] = alloc_page(GFP_KERNEL | __GFP_ZERO);
if (!stt->pages[i])
- goto fail;
+ goto fail_free;
}
kvm_get_kvm(kvm);
@@ -344,15 +340,12 @@ long kvm_vm_ioctl_create_spapr_tce(struct kvm *kvm,
return anon_inode_getfd("kvm-spapr-tce", &kvm_spapr_tce_fops,
stt, O_RDWR | O_CLOEXEC);
-fail:
- if (stt) {
- for (i = 0; i < npages; i++)
- if (stt->pages[i])
- __free_page(stt->pages[i]);
-
- kfree(stt);
- }
- return ret;
+fail_free:
+ for (i = 0; i < npages; i++)
+ if (stt->pages[i])
+ __free_page(stt->pages[i]);
+ kfree(stt);
+ return -ENOMEM;
}
static void kvmppc_clear_tce(struct iommu_table *tbl, unsigned long entry)
--
2.13.5
--
Thanks,
David
^ permalink raw reply related [flat|nested] 35+ messages in thread
* Re: [PATCH] fix memory leak on kvm_vm_ioctl_create_spapr_tce
@ 2017-08-23 8:25 ` David Hildenbrand
0 siblings, 0 replies; 35+ messages in thread
From: David Hildenbrand @ 2017-08-23 8:25 UTC (permalink / raw)
To: Paul Mackerras, Nixiaoming
Cc: agraf, pbonzini, rkrcmar, benh, mpe, kvm-ppc, kvm, linuxppc-dev,
linux-kernel
On 23.08.2017 08:06, Paul Mackerras wrote:
> On Wed, Aug 23, 2017 at 01:43:08AM +0000, Nixiaoming wrote:
>>> 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);
... don't we have to take care of rcu synchronization before freeing it?
>> + mutex_unlock(&kvm->lock);
>> + kvm_put_kvm(kvm);
>> + goto fail;
>> + }
>> + return ret;
of simply
if (!ret)
return 0;
mutex_lock(&kvm->lock);
list_del_rcu(&stt->list);
mutex_unlock(&kvm->lock);
kvm_put_kvm(kvm);
>
> It seems to me that it would be better to do the anon_inode_getfd()
> call before the kvm_get_kvm() call, and go to the fail label if it
> fails.
I would have suggested to not add it to the list before it has been
fully created (so nobody can have access to it). But I guess than we
need another level of protection(e.g. kvm->lock).
Am I missing something, or is kvm_vm_ioctl_create_spapr_tce() racy?
The -EBUSY check is done without any locking, so two parallel creators
could create an inconsistency, no? Shouldn't this all be protected by
kvm->lock?
>
> Paul.
>
Independent of the fix, I'd suggest the following cleanup.
From 979f55083ee965e25827a8743e8a9fdb85231a6f Mon Sep 17 00:00:00 2001
From: David Hildenbrand <david@redhat.com>
Date: Wed, 23 Aug 2017 10:08:58 +0200
Subject: [PATCH v1 1/1] KVM: PPC: cleanup kvm_vm_ioctl_create_spapr_tce
Let's simplify error handling.
Signed-off-by: David Hildenbrand <david@redhat.com>
---
arch/powerpc/kvm/book3s_64_vio.c | 29 +++++++++++------------------
1 file changed, 11 insertions(+), 18 deletions(-)
diff --git a/arch/powerpc/kvm/book3s_64_vio.c
b/arch/powerpc/kvm/book3s_64_vio.c
index a160c14304eb..6bac49292296 100644
--- a/arch/powerpc/kvm/book3s_64_vio.c
+++ b/arch/powerpc/kvm/book3s_64_vio.c
@@ -295,8 +295,7 @@ long kvm_vm_ioctl_create_spapr_tce(struct kvm *kvm,
{
struct kvmppc_spapr_tce_table *stt = NULL;
unsigned long npages, size;
- int ret = -ENOMEM;
- int i;
+ int i, ret;
if (!args->size)
return -EINVAL;
@@ -310,16 +309,13 @@ long kvm_vm_ioctl_create_spapr_tce(struct kvm *kvm,
size = _ALIGN_UP(args->size, PAGE_SIZE >> 3);
npages = kvmppc_tce_pages(size);
ret = kvmppc_account_memlimit(kvmppc_stt_pages(npages), true);
- if (ret) {
- stt = NULL;
- goto fail;
- }
+ if (ret)
+ return ret;
- ret = -ENOMEM;
stt = kzalloc(sizeof(*stt) + npages * sizeof(struct page *),
GFP_KERNEL);
if (!stt)
- goto fail;
+ return -ENOMEM;
stt->liobn = args->liobn;
stt->page_shift = args->page_shift;
@@ -331,7 +327,7 @@ long kvm_vm_ioctl_create_spapr_tce(struct kvm *kvm,
for (i = 0; i < npages; i++) {
stt->pages[i] = alloc_page(GFP_KERNEL | __GFP_ZERO);
if (!stt->pages[i])
- goto fail;
+ goto fail_free;
}
kvm_get_kvm(kvm);
@@ -344,15 +340,12 @@ long kvm_vm_ioctl_create_spapr_tce(struct kvm *kvm,
return anon_inode_getfd("kvm-spapr-tce", &kvm_spapr_tce_fops,
stt, O_RDWR | O_CLOEXEC);
-fail:
- if (stt) {
- for (i = 0; i < npages; i++)
- if (stt->pages[i])
- __free_page(stt->pages[i]);
-
- kfree(stt);
- }
- return ret;
+fail_free:
+ for (i = 0; i < npages; i++)
+ if (stt->pages[i])
+ __free_page(stt->pages[i]);
+ kfree(stt);
+ return -ENOMEM;
}
static void kvmppc_clear_tce(struct iommu_table *tbl, unsigned long entry)
--
2.13.5
--
Thanks,
David
^ permalink raw reply related [flat|nested] 35+ messages in thread
* Re: [PATCH] fix memory leak on kvm_vm_ioctl_create_spapr_tce
2017-08-23 8:25 ` David Hildenbrand
@ 2017-08-23 9:16 ` David Hildenbrand
-1 siblings, 0 replies; 35+ messages in thread
From: David Hildenbrand @ 2017-08-23 9:16 UTC (permalink / raw)
To: Paul Mackerras, Nixiaoming
Cc: agraf, pbonzini, rkrcmar, benh, mpe, kvm-ppc, kvm, linuxppc-dev,
linux-kernel
>>> + mutex_unlock(&kvm->lock);
>>> + kvm_put_kvm(kvm);
>>> + goto fail;
>>> + }
>>> + return ret;
>
> of simply
>
> if (!ret)
if (ret >= 0)
return ret;
is of course what I meant :)
> return 0;
>
> mutex_lock(&kvm->lock);
> list_del_rcu(&stt->list);
> mutex_unlock(&kvm->lock);
> kvm_put_kvm(kvm);
--
Thanks,
David
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH] fix memory leak on kvm_vm_ioctl_create_spapr_tce
@ 2017-08-23 9:16 ` David Hildenbrand
0 siblings, 0 replies; 35+ messages in thread
From: David Hildenbrand @ 2017-08-23 9:16 UTC (permalink / raw)
To: Paul Mackerras, Nixiaoming
Cc: agraf, pbonzini, rkrcmar, benh, mpe, kvm-ppc, kvm, linuxppc-dev,
linux-kernel
>>> + mutex_unlock(&kvm->lock);
>>> + kvm_put_kvm(kvm);
>>> + goto fail;
>>> + }
>>> + return ret;
>
> of simply
>
> if (!ret)
if (ret >= 0)
return ret;
is of course what I meant :)
> return 0;
>
> mutex_lock(&kvm->lock);
> list_del_rcu(&stt->list);
> mutex_unlock(&kvm->lock);
> kvm_put_kvm(kvm);
--
Thanks,
David
^ permalink raw reply [flat|nested] 35+ messages in thread
* 答复: [PATCH] fix memory leak on kvm_vm_ioctl_create_spapr_tce
2017-08-23 8:25 ` David Hildenbrand
(?)
@ 2017-08-23 10:17 ` Nixiaoming
-1 siblings, 0 replies; 35+ messages in thread
From: Nixiaoming @ 2017-08-23 10:17 UTC (permalink / raw)
To: David Hildenbrand, Paul Mackerras
Cc: agraf, pbonzini, rkrcmar, benh, mpe, kvm-ppc, kvm, linuxppc-dev,
linux-kernel
>On 23.08.2017 08:06, Paul Mackerras wrote:
>> On Wed, Aug 23, 2017 at 01:43:08AM +0000, Nixiaoming wrote:
>>>> 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);
>
>... don't we have to take care of rcu synchronization before freeing it?
>
>>> + mutex_unlock(&kvm->lock);
>>> + kvm_put_kvm(kvm);
>>> + goto fail;
>>> + }
>>> + return ret;
>
>of simply
>
>if (!ret)
> return 0;
>
>mutex_lock(&kvm->lock);
>list_del_rcu(&stt->list);
>mutex_unlock(&kvm->lock);
>kvm_put_kvm(kvm);
>
>
>>
>> It seems to me that it would be better to do the anon_inode_getfd()
>> call before the kvm_get_kvm() call, and go to the fail label if it
>> fails.
>
>I would have suggested to not add it to the list before it has been
>fully created (so nobody can have access to it). But I guess than we
>need another level of protection(e.g. kvm->lock).
>
>Am I missing something, or is kvm_vm_ioctl_create_spapr_tce() racy?
>
>The -EBUSY check is done without any locking, so two parallel creators
>could create an inconsistency, no? Shouldn't this all be protected by
>kvm->lock?
>
>>
>> Paul.
>>
>
>Independent of the fix, I'd suggest the following cleanup.
>
>
>From 979f55083ee965e25827a8743e8a9fdb85231a6f Mon Sep 17 00:00:00 2001
>From: David Hildenbrand <david@redhat.com>
>Date: Wed, 23 Aug 2017 10:08:58 +0200
>Subject: [PATCH v1 1/1] KVM: PPC: cleanup kvm_vm_ioctl_create_spapr_tce
>
>Let's simplify error handling.
>
>Signed-off-by: David Hildenbrand <david@redhat.com>
>---
> arch/powerpc/kvm/book3s_64_vio.c | 29 +++++++++++------------------
> 1 file changed, 11 insertions(+), 18 deletions(-)
>
>diff --git a/arch/powerpc/kvm/book3s_64_vio.c
>b/arch/powerpc/kvm/book3s_64_vio.c
>index a160c14304eb..6bac49292296 100644
>--- a/arch/powerpc/kvm/book3s_64_vio.c
>+++ b/arch/powerpc/kvm/book3s_64_vio.c
>@@ -295,8 +295,7 @@ long kvm_vm_ioctl_create_spapr_tce(struct kvm *kvm,
> {
> struct kvmppc_spapr_tce_table *stt = NULL;
> unsigned long npages, size;
>- int ret = -ENOMEM;
>- int i;
>+ int i, ret;
>
> if (!args->size)
> return -EINVAL;
>@@ -310,16 +309,13 @@ long kvm_vm_ioctl_create_spapr_tce(struct kvm *kvm,
> size = _ALIGN_UP(args->size, PAGE_SIZE >> 3);
> npages = kvmppc_tce_pages(size);
> ret = kvmppc_account_memlimit(kvmppc_stt_pages(npages), true);
>- if (ret) {
>- stt = NULL;
>- goto fail;
>- }
>+ if (ret)
>+ return ret;
>
>- ret = -ENOMEM;
> stt = kzalloc(sizeof(*stt) + npages * sizeof(struct page *),
> GFP_KERNEL);
> if (!stt)
>- goto fail;
>+ return -ENOMEM;
>
> stt->liobn = args->liobn;
> stt->page_shift = args->page_shift;
>@@ -331,7 +327,7 @@ long kvm_vm_ioctl_create_spapr_tce(struct kvm *kvm,
> for (i = 0; i < npages; i++) {
> stt->pages[i] = alloc_page(GFP_KERNEL | __GFP_ZERO);
> if (!stt->pages[i])
>- goto fail;
>+ goto fail_free;
> }
>
> kvm_get_kvm(kvm);
>@@ -344,15 +340,12 @@ long kvm_vm_ioctl_create_spapr_tce(struct kvm *kvm,
> return anon_inode_getfd("kvm-spapr-tce", &kvm_spapr_tce_fops,
> stt, O_RDWR | O_CLOEXEC);
>
>-fail:
>- if (stt) {
>- for (i = 0; i < npages; i++)
>- if (stt->pages[i])
>- __free_page(stt->pages[i]);
>-
>- kfree(stt);
>- }
>- return ret;
>+fail_free:
>+ for (i = 0; i < npages; i++)
>+ if (stt->pages[i])
>+ __free_page(stt->pages[i]);
>+ kfree(stt);
>+ return -ENOMEM;
> }
>
> static void kvmppc_clear_tce(struct iommu_table *tbl, unsigned long entry)
>--
>2.13.5
>
>
>--
>
>Thanks,
>
>David
>
Update patch based on advice from David Hildenbrand and Paul Mackerras
---
arch/powerpc/kvm/book3s_64_vio.c | 13 +++++++++----
1 file changed, 9 insertions(+), 4 deletions(-)
diff --git a/arch/powerpc/kvm/book3s_64_vio.c b/arch/powerpc/kvm/book3s_64_vio.c
index a160c14..517594a 100644
--- a/arch/powerpc/kvm/book3s_64_vio.c
+++ b/arch/powerpc/kvm/book3s_64_vio.c
@@ -334,16 +334,21 @@ long kvm_vm_ioctl_create_spapr_tce(struct kvm *kvm,
goto fail;
}
- kvm_get_kvm(kvm);
-
mutex_lock(&kvm->lock);
list_add_rcu(&stt->list, &kvm->arch.spapr_tce_tables);
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) {
+ kvm_get_kvm(kvm);
+ return ret;
+ }
+ mutex_lock(&kvm->lock);
+ list_del_rcu(&stt->list);
+ mutex_unlock(&kvm->lock);
+ synchronize_rcu();
fail:
if (stt) {
for (i = 0; i < npages; i++)
--
^ permalink raw reply related [flat|nested] 35+ messages in thread
* 答复: [PATCH] fix memory leak on kvm_vm_ioctl_create_spapr_tce
@ 2017-08-23 10:17 ` Nixiaoming
0 siblings, 0 replies; 35+ messages in thread
From: Nixiaoming @ 2017-08-23 10:17 UTC (permalink / raw)
To: David Hildenbrand, Paul Mackerras
Cc: agraf, pbonzini, rkrcmar, benh, mpe, kvm-ppc, kvm, linuxppc-dev,
linux-kernel
Pk9uIDIzLjA4LjIwMTcgMDg6MDYsIFBhdWwgTWFja2VycmFzIHdyb3RlOg0KPj4gT24gV2VkLCBB
dWcgMjMsIDIwMTcgYXQgMDE6NDM6MDhBTSArMDAwMCwgTml4aWFvbWluZyB3cm90ZToNCj4+Pj4g
T24gMjIuMDguMjAxNyAxNzoxNSwgRGF2aWQgSGlsZGVuYnJhbmQgd3JvdGU6DQo+Pj4+PiBPbiAy
Mi4wOC4yMDE3IDE2OjI4LCBuaXhpYW9taW5nIHdyb3RlOg0KPj4+Pj4+IG1pc3Mga2ZyZWUoc3R0
KSB3aGVuIGFub25faW5vZGVfZ2V0ZmQgcmV0dXJuIGZhaWwgc28gYWRkIGNoZWNrIA0KPj4+Pj4+
IGFub25faW5vZGVfZ2V0ZmQgcmV0dXJuIHZhbCwgYW5kIGtmcmVlIHN0dA0KPj4+Pj4+DQo+Pj4+
Pj4gU2lnbmVkLW9mZi1ieTogbml4aWFvbWluZyA8bml4aWFvbWluZ0BodWF3ZWkuY29tPg0KPj4+
Pj4+IC0tLQ0KPj4+Pj4+ICBhcmNoL3Bvd2VycGMva3ZtL2Jvb2szc182NF92aW8uYyB8IDUgKysr
Ky0NCj4+Pj4+PiAgMSBmaWxlIGNoYW5nZWQsIDQgaW5zZXJ0aW9ucygrKSwgMSBkZWxldGlvbigt
KQ0KPj4+Pj4+DQo+Pj4+Pj4gZGlmZiAtLWdpdCBhL2FyY2gvcG93ZXJwYy9rdm0vYm9vazNzXzY0
X3Zpby5jIA0KPj4+Pj4+IGIvYXJjaC9wb3dlcnBjL2t2bS9ib29rM3NfNjRfdmlvLmMNCj4+Pj4+
PiBpbmRleCBhMTYwYzE0Li5hMGI0NDU5IDEwMDY0NA0KPj4+Pj4+IC0tLSBhL2FyY2gvcG93ZXJw
Yy9rdm0vYm9vazNzXzY0X3Zpby5jDQo+Pj4+Pj4gKysrIGIvYXJjaC9wb3dlcnBjL2t2bS9ib29r
M3NfNjRfdmlvLmMNCj4+Pj4+PiBAQCAtMzQxLDggKzM0MSwxMSBAQCBsb25nIGt2bV92bV9pb2N0
bF9jcmVhdGVfc3BhcHJfdGNlKHN0cnVjdCBrdm0gDQo+Pj4+Pj4gKmt2bSwNCj4+Pj4+PiAgDQo+
Pj4+Pj4gIAltdXRleF91bmxvY2soJmt2bS0+bG9jayk7DQo+Pj4+Pj4gIA0KPj4+Pj4+IC0JcmV0
dXJuIGFub25faW5vZGVfZ2V0ZmQoImt2bS1zcGFwci10Y2UiLCAma3ZtX3NwYXByX3RjZV9mb3Bz
LA0KPj4+Pj4+ICsJcmV0ID0gYW5vbl9pbm9kZV9nZXRmZCgia3ZtLXNwYXByLXRjZSIsICZrdm1f
c3BhcHJfdGNlX2ZvcHMsDQo+Pj4+Pj4gIAkJCQlzdHQsIE9fUkRXUiB8IE9fQ0xPRVhFQyk7DQo+
Pj4+Pj4gKwlpZiAocmV0IDwgMCkNCj4+Pj4+PiArCQlnb3RvIGZhaWw7DQo+Pj4+Pj4gKwlyZXR1
cm4gcmV0Ow0KPj4+Pj4+ICANCj4+Pj4+PiAgZmFpbDoNCj4+Pj4+PiAgCWlmIChzdHQpIHsNCj4+
Pj4+Pg0KPj4+Pj4NCj4+Pj4+DQo+Pj4+PiBzdHQgaGFzIGFscmVhZHkgYmVlbiBhZGRlZCB0byBr
dm0tPmFyY2guc3BhcHJfdGNlX3RhYmxlcywgc28gZnJlZWluZyANCj4+Pj4+IGl0IGlzIGV2aWwg
SU1ITy4gSSBkb24ndCBrbm93IHRoYXQgY29kZSwgc28gSSBkb24ndCBrbm93IGlmIHRoZXJlIGlz
IA0KPj4+Pj4gc29tZSBvdGhlciBwbGFjZSB0aGF0IHdpbGwgbWFrZSBzdXJlIHRoYXQgZXZlcnl0
aGluZyBpbg0KPj4+Pj4ga3ZtLT5hcmNoLnNwYXByX3RjZV90YWJsZXMgd2lsbCBwcm9wZXJseSBn
ZXQgZnJlZWQsIGV2ZW4gd2hlbiBubyANCj4+Pj4+IGt2bS0+cmVsZWFzZQ0KPj4+Pj4gZnVuY3Rp
b24gaGFzIGJlZW4gY2FsbGVkIChrdm1fc3BhcHJfdGNlX3JlbGVhc2UpLg0KPj4+Pj4NCj4+Pj4N
Cj4+Pj4gSWYgaXQgaXMgcmVhbGx5IG5vdCBmcmVlZCwgdGhhbiBhbHNvIGt2bV9wdXRfa3ZtKHN0
dC0+a3ZtKSBpcyBtaXNzaW5nLg0KPj4+Pg0KPj4+PiAtLSANCj4+Pj4NCj4+Pj4gVGhhbmtzLA0K
Pj4+Pg0KPj4+PiBEYXZpZA0KPj4+Pg0KPj4+DQo+Pj4gaWYgKCFzdHQpIHJldHVybiAtRU5PTUVN
Ow0KPj4+IGt2bV9nZXRfa3ZtKGt2bSk7DQo+Pj4gaWYgYW5vbl9pbm9kZV9nZXRmZCByZXR1cm4g
LUVOT01FTQ0KPj4+IFRoZSB1c2VyIGNhbiBub3QgZGV0ZXJtaW5lIHdoZXRoZXIga3ZtX2dldF9r
dm0gaGFzIGJlZW4gY2FsbGVkDQo+Pj4gc28gbmVlZCBhZGQga3ZtX3BldF9rdm0gd2hlbiBhbm9u
X2lub2RlX2dldGZkIGZhaWwNCj4+Pg0KPj4+IHN0dCBoYXMgYWxyZWFkeSBiZWVuIGFkZGVkIHRv
IGt2bS0+YXJjaC5zcGFwcl90Y2VfdGFibGVzLA0KPj4+IGJ1dCBpZiBhbm9uX2lub2RlX2dldGZk
IGZhaWwsIHN0dCBpcyB1bnVzZWQgdmFsLCANCj4+PiBzbyBjYWxsIGxpc3RfZGVsX3JjdSwgYW5k
ICBmcmVlIGFzIHF1aWNrbHkgYXMgcG9zc2libGUNCj4+Pg0KPj4+IG5ldyBwYXRjaDoNCj4+Pg0K
Pj4+IC0tLQ0KPj4+ICBhcmNoL3Bvd2VycGMva3ZtL2Jvb2szc182NF92aW8uYyB8IDEwICsrKysr
KysrKy0NCj4+PiAgMSBmaWxlIGNoYW5nZWQsIDkgaW5zZXJ0aW9ucygrKSwgMSBkZWxldGlvbigt
KQ0KPj4+DQo+Pj4gZGlmZiAtLWdpdCBhL2FyY2gvcG93ZXJwYy9rdm0vYm9vazNzXzY0X3Zpby5j
IGIvYXJjaC9wb3dlcnBjL2t2bS9ib29rM3NfNjRfdmlvLmMNCj4+PiBpbmRleCBhMTYwYzE0Li5l
MjIyOGYxIDEwMDY0NA0KPj4+IC0tLSBhL2FyY2gvcG93ZXJwYy9rdm0vYm9vazNzXzY0X3Zpby5j
DQo+Pj4gKysrIGIvYXJjaC9wb3dlcnBjL2t2bS9ib29rM3NfNjRfdmlvLmMNCj4+PiBAQCAtMzQx
LDggKzM0MSwxNiBAQCBsb25nIGt2bV92bV9pb2N0bF9jcmVhdGVfc3BhcHJfdGNlKHN0cnVjdCBr
dm0gKmt2bSwNCj4+Pg0KPj4+ICAgICAgICAgbXV0ZXhfdW5sb2NrKCZrdm0tPmxvY2spOw0KPj4+
DQo+Pj4gLSAgICAgICByZXR1cm4gYW5vbl9pbm9kZV9nZXRmZCgia3ZtLXNwYXByLXRjZSIsICZr
dm1fc3BhcHJfdGNlX2ZvcHMsDQo+Pj4gKyAgICAgICByZXQgPSBhbm9uX2lub2RlX2dldGZkKCJr
dm0tc3BhcHItdGNlIiwgJmt2bV9zcGFwcl90Y2VfZm9wcywNCj4+PiAgICAgICAgICAgICAgICAg
ICAgICAgICAgICAgICAgIHN0dCwgT19SRFdSIHwgT19DTE9FWEVDKTsNCj4+PiArICAgICAgIGlm
IChyZXQgPCAwKSB7DQo+Pj4gKyAgICAgICAgICAgICAgIG11dGV4X2xvY2soJmt2bS0+bG9jayk7
DQo+Pj4gKyAgICAgICAgICAgICAgIGxpc3RfZGVsX3JjdSgmc3R0LT5saXN0KTsNCj4NCj4uLi4g
ZG9uJ3Qgd2UgaGF2ZSB0byB0YWtlIGNhcmUgb2YgcmN1IHN5bmNocm9uaXphdGlvbiBiZWZvcmUg
ZnJlZWluZyBpdD8NCj4NCj4+PiArICAgICAgICAgICAgICAgbXV0ZXhfdW5sb2NrKCZrdm0tPmxv
Y2spOw0KPj4+ICsgICAgICAgICAgICAgICBrdm1fcHV0X2t2bShrdm0pOw0KPj4+ICsgICAgICAg
ICAgICAgICBnb3RvIGZhaWw7DQo+Pj4gKyAgICAgICB9DQo+Pj4gKyAgICAgICByZXR1cm4gcmV0
Ow0KPg0KPm9mIHNpbXBseQ0KPg0KPmlmICghcmV0KQ0KPglyZXR1cm4gMDsNCj4NCj5tdXRleF9s
b2NrKCZrdm0tPmxvY2spOw0KPmxpc3RfZGVsX3JjdSgmc3R0LT5saXN0KTsNCj5tdXRleF91bmxv
Y2soJmt2bS0+bG9jayk7DQo+a3ZtX3B1dF9rdm0oa3ZtKTsNCj4NCj4NCj4+IA0KPj4gSXQgc2Vl
bXMgdG8gbWUgdGhhdCBpdCB3b3VsZCBiZSBiZXR0ZXIgdG8gZG8gdGhlIGFub25faW5vZGVfZ2V0
ZmQoKQ0KPj4gY2FsbCBiZWZvcmUgdGhlIGt2bV9nZXRfa3ZtKCkgY2FsbCwgYW5kIGdvIHRvIHRo
ZSBmYWlsIGxhYmVsIGlmIGl0DQo+PiBmYWlscy4NCj4NCj5JIHdvdWxkIGhhdmUgc3VnZ2VzdGVk
IHRvIG5vdCBhZGQgaXQgdG8gdGhlIGxpc3QgYmVmb3JlIGl0IGhhcyBiZWVuDQo+ZnVsbHkgY3Jl
YXRlZCAoc28gbm9ib2R5IGNhbiBoYXZlIGFjY2VzcyB0byBpdCkuIEJ1dCBJIGd1ZXNzIHRoYW4g
d2UNCj5uZWVkIGFub3RoZXIgbGV2ZWwgb2YgcHJvdGVjdGlvbihlLmcuIGt2bS0+bG9jaykuDQo+
DQo+QW0gSSBtaXNzaW5nIHNvbWV0aGluZywgb3IgaXMga3ZtX3ZtX2lvY3RsX2NyZWF0ZV9zcGFw
cl90Y2UoKSByYWN5Pw0KPg0KPlRoZSAtRUJVU1kgY2hlY2sgaXMgZG9uZSB3aXRob3V0IGFueSBs
b2NraW5nLCBzbyB0d28gcGFyYWxsZWwgY3JlYXRvcnMNCj5jb3VsZCBjcmVhdGUgYW4gaW5jb25z
aXN0ZW5jeSwgbm8/IFNob3VsZG4ndCB0aGlzIGFsbCBiZSBwcm90ZWN0ZWQgYnkNCj5rdm0tPmxv
Y2s/DQo+DQo+PiANCj4+IFBhdWwuDQo+PiANCj4NCj5JbmRlcGVuZGVudCBvZiB0aGUgZml4LCBJ
J2Qgc3VnZ2VzdCB0aGUgZm9sbG93aW5nIGNsZWFudXAuDQo+DQo+DQo+RnJvbSA5NzlmNTUwODNl
ZTk2NWUyNTgyN2E4NzQzZThhOWZkYjg1MjMxYTZmIE1vbiBTZXAgMTcgMDA6MDA6MDAgMjAwMQ0K
PkZyb206IERhdmlkIEhpbGRlbmJyYW5kIDxkYXZpZEByZWRoYXQuY29tPg0KPkRhdGU6IFdlZCwg
MjMgQXVnIDIwMTcgMTA6MDg6NTggKzAyMDANCj5TdWJqZWN0OiBbUEFUQ0ggdjEgMS8xXSBLVk06
IFBQQzogY2xlYW51cCBrdm1fdm1faW9jdGxfY3JlYXRlX3NwYXByX3RjZQ0KPg0KPkxldCdzIHNp
bXBsaWZ5IGVycm9yIGhhbmRsaW5nLg0KPg0KPlNpZ25lZC1vZmYtYnk6IERhdmlkIEhpbGRlbmJy
YW5kIDxkYXZpZEByZWRoYXQuY29tPg0KPi0tLQ0KPiBhcmNoL3Bvd2VycGMva3ZtL2Jvb2szc182
NF92aW8uYyB8IDI5ICsrKysrKysrKysrLS0tLS0tLS0tLS0tLS0tLS0tDQo+IDEgZmlsZSBjaGFu
Z2VkLCAxMSBpbnNlcnRpb25zKCspLCAxOCBkZWxldGlvbnMoLSkNCj4NCj5kaWZmIC0tZ2l0IGEv
YXJjaC9wb3dlcnBjL2t2bS9ib29rM3NfNjRfdmlvLmMNCj5iL2FyY2gvcG93ZXJwYy9rdm0vYm9v
azNzXzY0X3Zpby5jDQo+aW5kZXggYTE2MGMxNDMwNGViLi42YmFjNDkyOTIyOTYgMTAwNjQ0DQo+
LS0tIGEvYXJjaC9wb3dlcnBjL2t2bS9ib29rM3NfNjRfdmlvLmMNCj4rKysgYi9hcmNoL3Bvd2Vy
cGMva3ZtL2Jvb2szc182NF92aW8uYw0KPkBAIC0yOTUsOCArMjk1LDcgQEAgbG9uZyBrdm1fdm1f
aW9jdGxfY3JlYXRlX3NwYXByX3RjZShzdHJ1Y3Qga3ZtICprdm0sDQo+IHsNCj4gCXN0cnVjdCBr
dm1wcGNfc3BhcHJfdGNlX3RhYmxlICpzdHQgPSBOVUxMOw0KPiAJdW5zaWduZWQgbG9uZyBucGFn
ZXMsIHNpemU7DQo+LQlpbnQgcmV0ID0gLUVOT01FTTsNCj4tCWludCBpOw0KPisJaW50IGksIHJl
dDsNCj4NCj4gCWlmICghYXJncy0+c2l6ZSkNCj4gCQlyZXR1cm4gLUVJTlZBTDsNCj5AQCAtMzEw
LDE2ICszMDksMTMgQEAgbG9uZyBrdm1fdm1faW9jdGxfY3JlYXRlX3NwYXByX3RjZShzdHJ1Y3Qg
a3ZtICprdm0sDQo+IAlzaXplID0gX0FMSUdOX1VQKGFyZ3MtPnNpemUsIFBBR0VfU0laRSA+PiAz
KTsNCj4gCW5wYWdlcyA9IGt2bXBwY190Y2VfcGFnZXMoc2l6ZSk7DQo+IAlyZXQgPSBrdm1wcGNf
YWNjb3VudF9tZW1saW1pdChrdm1wcGNfc3R0X3BhZ2VzKG5wYWdlcyksIHRydWUpOw0KPi0JaWYg
KHJldCkgew0KPi0JCXN0dCA9IE5VTEw7DQo+LQkJZ290byBmYWlsOw0KPi0JfQ0KPisJaWYgKHJl
dCkNCj4rCQlyZXR1cm4gcmV0Ow0KPg0KPi0JcmV0ID0gLUVOT01FTTsNCj4gCXN0dCA9IGt6YWxs
b2Moc2l6ZW9mKCpzdHQpICsgbnBhZ2VzICogc2l6ZW9mKHN0cnVjdCBwYWdlICopLA0KPiAJCSAg
ICAgIEdGUF9LRVJORUwpOw0KPiAJaWYgKCFzdHQpDQo+LQkJZ290byBmYWlsOw0KPisJCXJldHVy
biAtRU5PTUVNOw0KPg0KPiAJc3R0LT5saW9ibiA9IGFyZ3MtPmxpb2JuOw0KPiAJc3R0LT5wYWdl
X3NoaWZ0ID0gYXJncy0+cGFnZV9zaGlmdDsNCj5AQCAtMzMxLDcgKzMyNyw3IEBAIGxvbmcga3Zt
X3ZtX2lvY3RsX2NyZWF0ZV9zcGFwcl90Y2Uoc3RydWN0IGt2bSAqa3ZtLA0KPiAJZm9yIChpID0g
MDsgaSA8IG5wYWdlczsgaSsrKSB7DQo+IAkJc3R0LT5wYWdlc1tpXSA9IGFsbG9jX3BhZ2UoR0ZQ
X0tFUk5FTCB8IF9fR0ZQX1pFUk8pOw0KPiAJCWlmICghc3R0LT5wYWdlc1tpXSkNCj4tCQkJZ290
byBmYWlsOw0KPisJCQlnb3RvIGZhaWxfZnJlZTsNCj4gCX0NCj4NCj4gCWt2bV9nZXRfa3ZtKGt2
bSk7DQo+QEAgLTM0NCwxNSArMzQwLDEyIEBAIGxvbmcga3ZtX3ZtX2lvY3RsX2NyZWF0ZV9zcGFw
cl90Y2Uoc3RydWN0IGt2bSAqa3ZtLA0KPiAJcmV0dXJuIGFub25faW5vZGVfZ2V0ZmQoImt2bS1z
cGFwci10Y2UiLCAma3ZtX3NwYXByX3RjZV9mb3BzLA0KPiAJCQkJc3R0LCBPX1JEV1IgfCBPX0NM
T0VYRUMpOw0KPg0KPi1mYWlsOg0KPi0JaWYgKHN0dCkgew0KPi0JCWZvciAoaSA9IDA7IGkgPCBu
cGFnZXM7IGkrKykNCj4tCQkJaWYgKHN0dC0+cGFnZXNbaV0pDQo+LQkJCQlfX2ZyZWVfcGFnZShz
dHQtPnBhZ2VzW2ldKTsNCj4tDQo+LQkJa2ZyZWUoc3R0KTsNCj4tCX0NCj4tCXJldHVybiByZXQ7
DQo+K2ZhaWxfZnJlZToNCj4rCWZvciAoaSA9IDA7IGkgPCBucGFnZXM7IGkrKykNCj4rCQlpZiAo
c3R0LT5wYWdlc1tpXSkNCj4rCQkJX19mcmVlX3BhZ2Uoc3R0LT5wYWdlc1tpXSk7DQo+KwlrZnJl
ZShzdHQpOw0KPisJcmV0dXJuIC1FTk9NRU07DQo+IH0NCj4NCj4gc3RhdGljIHZvaWQga3ZtcHBj
X2NsZWFyX3RjZShzdHJ1Y3QgaW9tbXVfdGFibGUgKnRibCwgdW5zaWduZWQgbG9uZyBlbnRyeSkN
Cj4tLSANCj4yLjEzLjUNCj4NCj4NCj4tLSANCj4NCj5UaGFua3MsDQo+DQo+RGF2aWQNCj4NCg0K
VXBkYXRlIHBhdGNoIGJhc2VkIG9uIGFkdmljZSBmcm9tIERhdmlkIEhpbGRlbmJyYW5kIGFuZCBQ
YXVsIE1hY2tlcnJhcw0KDQotLS0NCiBhcmNoL3Bvd2VycGMva3ZtL2Jvb2szc182NF92aW8uYyB8
IDEzICsrKysrKysrKy0tLS0NCiAxIGZpbGUgY2hhbmdlZCwgOSBpbnNlcnRpb25zKCspLCA0IGRl
bGV0aW9ucygtKQ0KDQpkaWZmIC0tZ2l0IGEvYXJjaC9wb3dlcnBjL2t2bS9ib29rM3NfNjRfdmlv
LmMgYi9hcmNoL3Bvd2VycGMva3ZtL2Jvb2szc182NF92aW8uYw0KaW5kZXggYTE2MGMxNC4uNTE3
NTk0YSAxMDA2NDQNCi0tLSBhL2FyY2gvcG93ZXJwYy9rdm0vYm9vazNzXzY0X3Zpby5jDQorKysg
Yi9hcmNoL3Bvd2VycGMva3ZtL2Jvb2szc182NF92aW8uYw0KQEAgLTMzNCwxNiArMzM0LDIxIEBA
IGxvbmcga3ZtX3ZtX2lvY3RsX2NyZWF0ZV9zcGFwcl90Y2Uoc3RydWN0IGt2bSAqa3ZtLA0KIAkJ
CWdvdG8gZmFpbDsNCiAJfQ0KIA0KLQlrdm1fZ2V0X2t2bShrdm0pOw0KLQ0KIAltdXRleF9sb2Nr
KCZrdm0tPmxvY2spOw0KIAlsaXN0X2FkZF9yY3UoJnN0dC0+bGlzdCwgJmt2bS0+YXJjaC5zcGFw
cl90Y2VfdGFibGVzKTsNCiANCiAJbXV0ZXhfdW5sb2NrKCZrdm0tPmxvY2spOw0KIA0KLQlyZXR1
cm4gYW5vbl9pbm9kZV9nZXRmZCgia3ZtLXNwYXByLXRjZSIsICZrdm1fc3BhcHJfdGNlX2ZvcHMs
DQorCXJldCA9IGFub25faW5vZGVfZ2V0ZmQoImt2bS1zcGFwci10Y2UiLCAma3ZtX3NwYXByX3Rj
ZV9mb3BzLA0KIAkJCQlzdHQsIE9fUkRXUiB8IE9fQ0xPRVhFQyk7DQotDQorCWlmIChyZXQgPj0g
MCkgew0KKwkJa3ZtX2dldF9rdm0oa3ZtKTsNCisJCXJldHVybiByZXQ7DQorCX0NCisJbXV0ZXhf
bG9jaygma3ZtLT5sb2NrKTsNCisJbGlzdF9kZWxfcmN1KCZzdHQtPmxpc3QpOw0KKwltdXRleF91
bmxvY2soJmt2bS0+bG9jayk7DQorCXN5bmNocm9uaXplX3JjdSgpOw0KIGZhaWw6DQogCWlmIChz
dHQpIHsNCiAJCWZvciAoaSA9IDA7IGkgPCBucGFnZXM7IGkrKykNCi0tIA0KDQoNCg==
^ permalink raw reply [flat|nested] 35+ messages in thread
* 答复: [PATCH] fix memory leak on kvm_vm_ioc =?utf-8?B?dGxfY3JlYXRlX3NwY
@ 2017-08-23 10:17 ` Nixiaoming
0 siblings, 0 replies; 35+ messages in thread
From: Nixiaoming @ 2017-08-23 10:17 UTC (permalink / raw)
To: David Hildenbrand, Paul Mackerras
Cc: agraf, pbonzini, rkrcmar, benh, mpe, kvm-ppc, kvm, linuxppc-dev,
linux-kernel
Pk9uIDIzLjA4LjIwMTcgMDg6MDYsIFBhdWwgTWFja2VycmFzIHdyb3RlOg0KPj4gT24gV2VkLCBB
dWcgMjMsIDIwMTcgYXQgMDE6NDM6MDhBTSArMDAwMCwgTml4aWFvbWluZyB3cm90ZToNCj4+Pj4g
T24gMjIuMDguMjAxNyAxNzoxNSwgRGF2aWQgSGlsZGVuYnJhbmQgd3JvdGU6DQo+Pj4+PiBPbiAy
Mi4wOC4yMDE3IDE2OjI4LCBuaXhpYW9taW5nIHdyb3RlOg0KPj4+Pj4+IG1pc3Mga2ZyZWUoc3R0
KSB3aGVuIGFub25faW5vZGVfZ2V0ZmQgcmV0dXJuIGZhaWwgc28gYWRkIGNoZWNrIA0KPj4+Pj4+
IGFub25faW5vZGVfZ2V0ZmQgcmV0dXJuIHZhbCwgYW5kIGtmcmVlIHN0dA0KPj4+Pj4+DQo+Pj4+
Pj4gU2lnbmVkLW9mZi1ieTogbml4aWFvbWluZyA8bml4aWFvbWluZ0BodWF3ZWkuY29tPg0KPj4+
Pj4+IC0tLQ0KPj4+Pj4+ICBhcmNoL3Bvd2VycGMva3ZtL2Jvb2szc182NF92aW8uYyB8IDUgKysr
Ky0NCj4+Pj4+PiAgMSBmaWxlIGNoYW5nZWQsIDQgaW5zZXJ0aW9ucygrKSwgMSBkZWxldGlvbigt
KQ0KPj4+Pj4+DQo+Pj4+Pj4gZGlmZiAtLWdpdCBhL2FyY2gvcG93ZXJwYy9rdm0vYm9vazNzXzY0
X3Zpby5jIA0KPj4+Pj4+IGIvYXJjaC9wb3dlcnBjL2t2bS9ib29rM3NfNjRfdmlvLmMNCj4+Pj4+
PiBpbmRleCBhMTYwYzE0Li5hMGI0NDU5IDEwMDY0NA0KPj4+Pj4+IC0tLSBhL2FyY2gvcG93ZXJw
Yy9rdm0vYm9vazNzXzY0X3Zpby5jDQo+Pj4+Pj4gKysrIGIvYXJjaC9wb3dlcnBjL2t2bS9ib29r
M3NfNjRfdmlvLmMNCj4+Pj4+PiBAQCAtMzQxLDggKzM0MSwxMSBAQCBsb25nIGt2bV92bV9pb2N0
bF9jcmVhdGVfc3BhcHJfdGNlKHN0cnVjdCBrdm0gDQo+Pj4+Pj4gKmt2bSwNCj4+Pj4+PiAgDQo+
Pj4+Pj4gIAltdXRleF91bmxvY2soJmt2bS0+bG9jayk7DQo+Pj4+Pj4gIA0KPj4+Pj4+IC0JcmV0
dXJuIGFub25faW5vZGVfZ2V0ZmQoImt2bS1zcGFwci10Y2UiLCAma3ZtX3NwYXByX3RjZV9mb3Bz
LA0KPj4+Pj4+ICsJcmV0ID0gYW5vbl9pbm9kZV9nZXRmZCgia3ZtLXNwYXByLXRjZSIsICZrdm1f
c3BhcHJfdGNlX2ZvcHMsDQo+Pj4+Pj4gIAkJCQlzdHQsIE9fUkRXUiB8IE9fQ0xPRVhFQyk7DQo+
Pj4+Pj4gKwlpZiAocmV0IDwgMCkNCj4+Pj4+PiArCQlnb3RvIGZhaWw7DQo+Pj4+Pj4gKwlyZXR1
cm4gcmV0Ow0KPj4+Pj4+ICANCj4+Pj4+PiAgZmFpbDoNCj4+Pj4+PiAgCWlmIChzdHQpIHsNCj4+
Pj4+Pg0KPj4+Pj4NCj4+Pj4+DQo+Pj4+PiBzdHQgaGFzIGFscmVhZHkgYmVlbiBhZGRlZCB0byBr
dm0tPmFyY2guc3BhcHJfdGNlX3RhYmxlcywgc28gZnJlZWluZyANCj4+Pj4+IGl0IGlzIGV2aWwg
SU1ITy4gSSBkb24ndCBrbm93IHRoYXQgY29kZSwgc28gSSBkb24ndCBrbm93IGlmIHRoZXJlIGlz
IA0KPj4+Pj4gc29tZSBvdGhlciBwbGFjZSB0aGF0IHdpbGwgbWFrZSBzdXJlIHRoYXQgZXZlcnl0
aGluZyBpbg0KPj4+Pj4ga3ZtLT5hcmNoLnNwYXByX3RjZV90YWJsZXMgd2lsbCBwcm9wZXJseSBn
ZXQgZnJlZWQsIGV2ZW4gd2hlbiBubyANCj4+Pj4+IGt2bS0+cmVsZWFzZQ0KPj4+Pj4gZnVuY3Rp
b24gaGFzIGJlZW4gY2FsbGVkIChrdm1fc3BhcHJfdGNlX3JlbGVhc2UpLg0KPj4+Pj4NCj4+Pj4N
Cj4+Pj4gSWYgaXQgaXMgcmVhbGx5IG5vdCBmcmVlZCwgdGhhbiBhbHNvIGt2bV9wdXRfa3ZtKHN0
dC0+a3ZtKSBpcyBtaXNzaW5nLg0KPj4+Pg0KPj4+PiAtLSANCj4+Pj4NCj4+Pj4gVGhhbmtzLA0K
Pj4+Pg0KPj4+PiBEYXZpZA0KPj4+Pg0KPj4+DQo+Pj4gaWYgKCFzdHQpIHJldHVybiAtRU5PTUVN
Ow0KPj4+IGt2bV9nZXRfa3ZtKGt2bSk7DQo+Pj4gaWYgYW5vbl9pbm9kZV9nZXRmZCByZXR1cm4g
LUVOT01FTQ0KPj4+IFRoZSB1c2VyIGNhbiBub3QgZGV0ZXJtaW5lIHdoZXRoZXIga3ZtX2dldF9r
dm0gaGFzIGJlZW4gY2FsbGVkDQo+Pj4gc28gbmVlZCBhZGQga3ZtX3BldF9rdm0gd2hlbiBhbm9u
X2lub2RlX2dldGZkIGZhaWwNCj4+Pg0KPj4+IHN0dCBoYXMgYWxyZWFkeSBiZWVuIGFkZGVkIHRv
IGt2bS0+YXJjaC5zcGFwcl90Y2VfdGFibGVzLA0KPj4+IGJ1dCBpZiBhbm9uX2lub2RlX2dldGZk
IGZhaWwsIHN0dCBpcyB1bnVzZWQgdmFsLCANCj4+PiBzbyBjYWxsIGxpc3RfZGVsX3JjdSwgYW5k
ICBmcmVlIGFzIHF1aWNrbHkgYXMgcG9zc2libGUNCj4+Pg0KPj4+IG5ldyBwYXRjaDoNCj4+Pg0K
Pj4+IC0tLQ0KPj4+ICBhcmNoL3Bvd2VycGMva3ZtL2Jvb2szc182NF92aW8uYyB8IDEwICsrKysr
KysrKy0NCj4+PiAgMSBmaWxlIGNoYW5nZWQsIDkgaW5zZXJ0aW9ucygrKSwgMSBkZWxldGlvbigt
KQ0KPj4+DQo+Pj4gZGlmZiAtLWdpdCBhL2FyY2gvcG93ZXJwYy9rdm0vYm9vazNzXzY0X3Zpby5j
IGIvYXJjaC9wb3dlcnBjL2t2bS9ib29rM3NfNjRfdmlvLmMNCj4+PiBpbmRleCBhMTYwYzE0Li5l
MjIyOGYxIDEwMDY0NA0KPj4+IC0tLSBhL2FyY2gvcG93ZXJwYy9rdm0vYm9vazNzXzY0X3Zpby5j
DQo+Pj4gKysrIGIvYXJjaC9wb3dlcnBjL2t2bS9ib29rM3NfNjRfdmlvLmMNCj4+PiBAQCAtMzQx
LDggKzM0MSwxNiBAQCBsb25nIGt2bV92bV9pb2N0bF9jcmVhdGVfc3BhcHJfdGNlKHN0cnVjdCBr
dm0gKmt2bSwNCj4+Pg0KPj4+ICAgICAgICAgbXV0ZXhfdW5sb2NrKCZrdm0tPmxvY2spOw0KPj4+
DQo+Pj4gLSAgICAgICByZXR1cm4gYW5vbl9pbm9kZV9nZXRmZCgia3ZtLXNwYXByLXRjZSIsICZr
dm1fc3BhcHJfdGNlX2ZvcHMsDQo+Pj4gKyAgICAgICByZXQgPSBhbm9uX2lub2RlX2dldGZkKCJr
dm0tc3BhcHItdGNlIiwgJmt2bV9zcGFwcl90Y2VfZm9wcywNCj4+PiAgICAgICAgICAgICAgICAg
ICAgICAgICAgICAgICAgIHN0dCwgT19SRFdSIHwgT19DTE9FWEVDKTsNCj4+PiArICAgICAgIGlm
IChyZXQgPCAwKSB7DQo+Pj4gKyAgICAgICAgICAgICAgIG11dGV4X2xvY2soJmt2bS0+bG9jayk7
DQo+Pj4gKyAgICAgICAgICAgICAgIGxpc3RfZGVsX3JjdSgmc3R0LT5saXN0KTsNCj4NCj4uLi4g
ZG9uJ3Qgd2UgaGF2ZSB0byB0YWtlIGNhcmUgb2YgcmN1IHN5bmNocm9uaXphdGlvbiBiZWZvcmUg
ZnJlZWluZyBpdD8NCj4NCj4+PiArICAgICAgICAgICAgICAgbXV0ZXhfdW5sb2NrKCZrdm0tPmxv
Y2spOw0KPj4+ICsgICAgICAgICAgICAgICBrdm1fcHV0X2t2bShrdm0pOw0KPj4+ICsgICAgICAg
ICAgICAgICBnb3RvIGZhaWw7DQo+Pj4gKyAgICAgICB9DQo+Pj4gKyAgICAgICByZXR1cm4gcmV0
Ow0KPg0KPm9mIHNpbXBseQ0KPg0KPmlmICghcmV0KQ0KPglyZXR1cm4gMDsNCj4NCj5tdXRleF9s
b2NrKCZrdm0tPmxvY2spOw0KPmxpc3RfZGVsX3JjdSgmc3R0LT5saXN0KTsNCj5tdXRleF91bmxv
Y2soJmt2bS0+bG9jayk7DQo+a3ZtX3B1dF9rdm0oa3ZtKTsNCj4NCj4NCj4+IA0KPj4gSXQgc2Vl
bXMgdG8gbWUgdGhhdCBpdCB3b3VsZCBiZSBiZXR0ZXIgdG8gZG8gdGhlIGFub25faW5vZGVfZ2V0
ZmQoKQ0KPj4gY2FsbCBiZWZvcmUgdGhlIGt2bV9nZXRfa3ZtKCkgY2FsbCwgYW5kIGdvIHRvIHRo
ZSBmYWlsIGxhYmVsIGlmIGl0DQo+PiBmYWlscy4NCj4NCj5JIHdvdWxkIGhhdmUgc3VnZ2VzdGVk
IHRvIG5vdCBhZGQgaXQgdG8gdGhlIGxpc3QgYmVmb3JlIGl0IGhhcyBiZWVuDQo+ZnVsbHkgY3Jl
YXRlZCAoc28gbm9ib2R5IGNhbiBoYXZlIGFjY2VzcyB0byBpdCkuIEJ1dCBJIGd1ZXNzIHRoYW4g
d2UNCj5uZWVkIGFub3RoZXIgbGV2ZWwgb2YgcHJvdGVjdGlvbihlLmcuIGt2bS0+bG9jaykuDQo+
DQo+QW0gSSBtaXNzaW5nIHNvbWV0aGluZywgb3IgaXMga3ZtX3ZtX2lvY3RsX2NyZWF0ZV9zcGFw
cl90Y2UoKSByYWN5Pw0KPg0KPlRoZSAtRUJVU1kgY2hlY2sgaXMgZG9uZSB3aXRob3V0IGFueSBs
b2NraW5nLCBzbyB0d28gcGFyYWxsZWwgY3JlYXRvcnMNCj5jb3VsZCBjcmVhdGUgYW4gaW5jb25z
aXN0ZW5jeSwgbm8/IFNob3VsZG4ndCB0aGlzIGFsbCBiZSBwcm90ZWN0ZWQgYnkNCj5rdm0tPmxv
Y2s/DQo+DQo+PiANCj4+IFBhdWwuDQo+PiANCj4NCj5JbmRlcGVuZGVudCBvZiB0aGUgZml4LCBJ
J2Qgc3VnZ2VzdCB0aGUgZm9sbG93aW5nIGNsZWFudXAuDQo+DQo+DQo+RnJvbSA5NzlmNTUwODNl
ZTk2NWUyNTgyN2E4NzQzZThhOWZkYjg1MjMxYTZmIE1vbiBTZXAgMTcgMDA6MDA6MDAgMjAwMQ0K
PkZyb206IERhdmlkIEhpbGRlbmJyYW5kIDxkYXZpZEByZWRoYXQuY29tPg0KPkRhdGU6IFdlZCwg
MjMgQXVnIDIwMTcgMTA6MDg6NTggKzAyMDANCj5TdWJqZWN0OiBbUEFUQ0ggdjEgMS8xXSBLVk06
IFBQQzogY2xlYW51cCBrdm1fdm1faW9jdGxfY3JlYXRlX3NwYXByX3RjZQ0KPg0KPkxldCdzIHNp
bXBsaWZ5IGVycm9yIGhhbmRsaW5nLg0KPg0KPlNpZ25lZC1vZmYtYnk6IERhdmlkIEhpbGRlbmJy
YW5kIDxkYXZpZEByZWRoYXQuY29tPg0KPi0tLQ0KPiBhcmNoL3Bvd2VycGMva3ZtL2Jvb2szc182
NF92aW8uYyB8IDI5ICsrKysrKysrKysrLS0tLS0tLS0tLS0tLS0tLS0tDQo+IDEgZmlsZSBjaGFu
Z2VkLCAxMSBpbnNlcnRpb25zKCspLCAxOCBkZWxldGlvbnMoLSkNCj4NCj5kaWZmIC0tZ2l0IGEv
YXJjaC9wb3dlcnBjL2t2bS9ib29rM3NfNjRfdmlvLmMNCj5iL2FyY2gvcG93ZXJwYy9rdm0vYm9v
azNzXzY0X3Zpby5jDQo+aW5kZXggYTE2MGMxNDMwNGViLi42YmFjNDkyOTIyOTYgMTAwNjQ0DQo+
LS0tIGEvYXJjaC9wb3dlcnBjL2t2bS9ib29rM3NfNjRfdmlvLmMNCj4rKysgYi9hcmNoL3Bvd2Vy
cGMva3ZtL2Jvb2szc182NF92aW8uYw0KPkBAIC0yOTUsOCArMjk1LDcgQEAgbG9uZyBrdm1fdm1f
aW9jdGxfY3JlYXRlX3NwYXByX3RjZShzdHJ1Y3Qga3ZtICprdm0sDQo+IHsNCj4gCXN0cnVjdCBr
dm1wcGNfc3BhcHJfdGNlX3RhYmxlICpzdHQgPSBOVUxMOw0KPiAJdW5zaWduZWQgbG9uZyBucGFn
ZXMsIHNpemU7DQo+LQlpbnQgcmV0ID0gLUVOT01FTTsNCj4tCWludCBpOw0KPisJaW50IGksIHJl
dDsNCj4NCj4gCWlmICghYXJncy0+c2l6ZSkNCj4gCQlyZXR1cm4gLUVJTlZBTDsNCj5AQCAtMzEw
LDE2ICszMDksMTMgQEAgbG9uZyBrdm1fdm1faW9jdGxfY3JlYXRlX3NwYXByX3RjZShzdHJ1Y3Qg
a3ZtICprdm0sDQo+IAlzaXplID0gX0FMSUdOX1VQKGFyZ3MtPnNpemUsIFBBR0VfU0laRSA+PiAz
KTsNCj4gCW5wYWdlcyA9IGt2bXBwY190Y2VfcGFnZXMoc2l6ZSk7DQo+IAlyZXQgPSBrdm1wcGNf
YWNjb3VudF9tZW1saW1pdChrdm1wcGNfc3R0X3BhZ2VzKG5wYWdlcyksIHRydWUpOw0KPi0JaWYg
KHJldCkgew0KPi0JCXN0dCA9IE5VTEw7DQo+LQkJZ290byBmYWlsOw0KPi0JfQ0KPisJaWYgKHJl
dCkNCj4rCQlyZXR1cm4gcmV0Ow0KPg0KPi0JcmV0ID0gLUVOT01FTTsNCj4gCXN0dCA9IGt6YWxs
b2Moc2l6ZW9mKCpzdHQpICsgbnBhZ2VzICogc2l6ZW9mKHN0cnVjdCBwYWdlICopLA0KPiAJCSAg
ICAgIEdGUF9LRVJORUwpOw0KPiAJaWYgKCFzdHQpDQo+LQkJZ290byBmYWlsOw0KPisJCXJldHVy
biAtRU5PTUVNOw0KPg0KPiAJc3R0LT5saW9ibiA9IGFyZ3MtPmxpb2JuOw0KPiAJc3R0LT5wYWdl
X3NoaWZ0ID0gYXJncy0+cGFnZV9zaGlmdDsNCj5AQCAtMzMxLDcgKzMyNyw3IEBAIGxvbmcga3Zt
X3ZtX2lvY3RsX2NyZWF0ZV9zcGFwcl90Y2Uoc3RydWN0IGt2bSAqa3ZtLA0KPiAJZm9yIChpID0g
MDsgaSA8IG5wYWdlczsgaSsrKSB7DQo+IAkJc3R0LT5wYWdlc1tpXSA9IGFsbG9jX3BhZ2UoR0ZQ
X0tFUk5FTCB8IF9fR0ZQX1pFUk8pOw0KPiAJCWlmICghc3R0LT5wYWdlc1tpXSkNCj4tCQkJZ290
byBmYWlsOw0KPisJCQlnb3RvIGZhaWxfZnJlZTsNCj4gCX0NCj4NCj4gCWt2bV9nZXRfa3ZtKGt2
bSk7DQo+QEAgLTM0NCwxNSArMzQwLDEyIEBAIGxvbmcga3ZtX3ZtX2lvY3RsX2NyZWF0ZV9zcGFw
cl90Y2Uoc3RydWN0IGt2bSAqa3ZtLA0KPiAJcmV0dXJuIGFub25faW5vZGVfZ2V0ZmQoImt2bS1z
cGFwci10Y2UiLCAma3ZtX3NwYXByX3RjZV9mb3BzLA0KPiAJCQkJc3R0LCBPX1JEV1IgfCBPX0NM
T0VYRUMpOw0KPg0KPi1mYWlsOg0KPi0JaWYgKHN0dCkgew0KPi0JCWZvciAoaSA9IDA7IGkgPCBu
cGFnZXM7IGkrKykNCj4tCQkJaWYgKHN0dC0+cGFnZXNbaV0pDQo+LQkJCQlfX2ZyZWVfcGFnZShz
dHQtPnBhZ2VzW2ldKTsNCj4tDQo+LQkJa2ZyZWUoc3R0KTsNCj4tCX0NCj4tCXJldHVybiByZXQ7
DQo+K2ZhaWxfZnJlZToNCj4rCWZvciAoaSA9IDA7IGkgPCBucGFnZXM7IGkrKykNCj4rCQlpZiAo
c3R0LT5wYWdlc1tpXSkNCj4rCQkJX19mcmVlX3BhZ2Uoc3R0LT5wYWdlc1tpXSk7DQo+KwlrZnJl
ZShzdHQpOw0KPisJcmV0dXJuIC1FTk9NRU07DQo+IH0NCj4NCj4gc3RhdGljIHZvaWQga3ZtcHBj
X2NsZWFyX3RjZShzdHJ1Y3QgaW9tbXVfdGFibGUgKnRibCwgdW5zaWduZWQgbG9uZyBlbnRyeSkN
Cj4tLSANCj4yLjEzLjUNCj4NCj4NCj4tLSANCj4NCj5UaGFua3MsDQo+DQo+RGF2aWQNCj4NCg0K
VXBkYXRlIHBhdGNoIGJhc2VkIG9uIGFkdmljZSBmcm9tIERhdmlkIEhpbGRlbmJyYW5kIGFuZCBQ
YXVsIE1hY2tlcnJhcw0KDQotLS0NCiBhcmNoL3Bvd2VycGMva3ZtL2Jvb2szc182NF92aW8uYyB8
IDEzICsrKysrKysrKy0tLS0NCiAxIGZpbGUgY2hhbmdlZCwgOSBpbnNlcnRpb25zKCspLCA0IGRl
bGV0aW9ucygtKQ0KDQpkaWZmIC0tZ2l0IGEvYXJjaC9wb3dlcnBjL2t2bS9ib29rM3NfNjRfdmlv
LmMgYi9hcmNoL3Bvd2VycGMva3ZtL2Jvb2szc182NF92aW8uYw0KaW5kZXggYTE2MGMxNC4uNTE3
NTk0YSAxMDA2NDQNCi0tLSBhL2FyY2gvcG93ZXJwYy9rdm0vYm9vazNzXzY0X3Zpby5jDQorKysg
Yi9hcmNoL3Bvd2VycGMva3ZtL2Jvb2szc182NF92aW8uYw0KQEAgLTMzNCwxNiArMzM0LDIxIEBA
IGxvbmcga3ZtX3ZtX2lvY3RsX2NyZWF0ZV9zcGFwcl90Y2Uoc3RydWN0IGt2bSAqa3ZtLA0KIAkJ
CWdvdG8gZmFpbDsNCiAJfQ0KIA0KLQlrdm1fZ2V0X2t2bShrdm0pOw0KLQ0KIAltdXRleF9sb2Nr
KCZrdm0tPmxvY2spOw0KIAlsaXN0X2FkZF9yY3UoJnN0dC0+bGlzdCwgJmt2bS0+YXJjaC5zcGFw
cl90Y2VfdGFibGVzKTsNCiANCiAJbXV0ZXhfdW5sb2NrKCZrdm0tPmxvY2spOw0KIA0KLQlyZXR1
cm4gYW5vbl9pbm9kZV9nZXRmZCgia3ZtLXNwYXByLXRjZSIsICZrdm1fc3BhcHJfdGNlX2ZvcHMs
DQorCXJldCA9IGFub25faW5vZGVfZ2V0ZmQoImt2bS1zcGFwci10Y2UiLCAma3ZtX3NwYXByX3Rj
ZV9mb3BzLA0KIAkJCQlzdHQsIE9fUkRXUiB8IE9fQ0xPRVhFQyk7DQotDQorCWlmIChyZXQgPj0g
MCkgew0KKwkJa3ZtX2dldF9rdm0oa3ZtKTsNCisJCXJldHVybiByZXQ7DQorCX0NCisJbXV0ZXhf
bG9jaygma3ZtLT5sb2NrKTsNCisJbGlzdF9kZWxfcmN1KCZzdHQtPmxpc3QpOw0KKwltdXRleF91
bmxvY2soJmt2bS0+bG9jayk7DQorCXN5bmNocm9uaXplX3JjdSgpOw0KIGZhaWw6DQogCWlmIChz
dHQpIHsNCiAJCWZvciAoaSA9IDA7IGkgPCBucGFnZXM7IGkrKykNCi0tIA0KDQoNCg=
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH] fix memory leak on kvm_vm_ioctl_create_spapr_tce
2017-08-23 8:25 ` David Hildenbrand
(?)
@ 2017-08-24 1:06 ` Nixiaoming
-1 siblings, 0 replies; 35+ messages in thread
From: Nixiaoming @ 2017-08-24 1:06 UTC (permalink / raw)
To: David Hildenbrand, Paul Mackerras
Cc: agraf, pbonzini, rkrcmar, benh, mpe, kvm-ppc, kvm, linuxppc-dev,
linux-kernel
>On 23.08.2017 08:06, Paul Mackerras wrote:
>> On Wed, Aug 23, 2017 at 01:43:08AM +0000, Nixiaoming wrote:
>>>> 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);
>
>... don't we have to take care of rcu synchronization before freeing it?
>
>>> + mutex_unlock(&kvm->lock);
>>> + kvm_put_kvm(kvm);
>>> + goto fail;
>>> + }
>>> + return ret;
>
>of simply
>
>if (!ret)
> return 0;
>
>mutex_lock(&kvm->lock);
>list_del_rcu(&stt->list);
>mutex_unlock(&kvm->lock);
>kvm_put_kvm(kvm);
>
>
>>
>> It seems to me that it would be better to do the anon_inode_getfd()
>> call before the kvm_get_kvm() call, and go to the fail label if it
>> fails.
>
>I would have suggested to not add it to the list before it has been
>fully created (so nobody can have access to it). But I guess than we
>need another level of protection(e.g. kvm->lock).
>
>Am I missing something, or is kvm_vm_ioctl_create_spapr_tce() racy?
>
>The -EBUSY check is done without any locking, so two parallel creators
>could create an inconsistency, no? Shouldn't this all be protected by
>kvm->lock?
>
>>
>> Paul.
>>
>
>Independent of the fix, I'd suggest the following cleanup.
>
>
>From 979f55083ee965e25827a8743e8a9fdb85231a6f Mon Sep 17 00:00:00 2001
>From: David Hildenbrand <david@redhat.com>
>Date: Wed, 23 Aug 2017 10:08:58 +0200
>Subject: [PATCH v1 1/1] KVM: PPC: cleanup kvm_vm_ioctl_create_spapr_tce
>
>Let's simplify error handling.
>
>Signed-off-by: David Hildenbrand <david@redhat.com>
>---
> arch/powerpc/kvm/book3s_64_vio.c | 29 +++++++++++------------------
> 1 file changed, 11 insertions(+), 18 deletions(-)
>
>diff --git a/arch/powerpc/kvm/book3s_64_vio.c
>b/arch/powerpc/kvm/book3s_64_vio.c
>index a160c14304eb..6bac49292296 100644
>--- a/arch/powerpc/kvm/book3s_64_vio.c
>+++ b/arch/powerpc/kvm/book3s_64_vio.c
>@@ -295,8 +295,7 @@ long kvm_vm_ioctl_create_spapr_tce(struct kvm *kvm,
>{
> struct kvmppc_spapr_tce_table *stt = NULL;
> unsigned long npages, size;
>- int ret = -ENOMEM;
>- int i;
>+ int i, ret;
>
> if (!args->size)
> return -EINVAL;
>@@ -310,16 +309,13 @@ long kvm_vm_ioctl_create_spapr_tce(struct kvm *kvm,
> size = _ALIGN_UP(args->size, PAGE_SIZE >> 3);
> npages = kvmppc_tce_pages(size);
> ret = kvmppc_account_memlimit(kvmppc_stt_pages(npages), true);
>- if (ret) {
>- stt = NULL;
>- goto fail;
>- }
>+ if (ret)
>+ return ret;
>
>- ret = -ENOMEM;
> stt = kzalloc(sizeof(*stt) + npages * sizeof(struct page *),
> GFP_KERNEL);
> if (!stt)
>- goto fail;
>+ return -ENOMEM;
>
> stt->liobn = args->liobn;
> stt->page_shift = args->page_shift;
>@@ -331,7 +327,7 @@ long kvm_vm_ioctl_create_spapr_tce(struct kvm *kvm,
> for (i = 0; i < npages; i++) {
> stt->pages[i] = alloc_page(GFP_KERNEL | __GFP_ZERO);
> if (!stt->pages[i])
>- goto fail;
>+ goto fail_free;
> }
>
> kvm_get_kvm(kvm);
>@@ -344,15 +340,12 @@ long kvm_vm_ioctl_create_spapr_tce(struct kvm *kvm,
> return anon_inode_getfd("kvm-spapr-tce", &kvm_spapr_tce_fops,
> stt, O_RDWR | O_CLOEXEC);
>
>-fail:
>- if (stt) {
>- for (i = 0; i < npages; i++)
>- if (stt->pages[i])
>- __free_page(stt->pages[i]);
>-
>- kfree(stt);
>- }
>- return ret;
>+fail_free:
>+ for (i = 0; i < npages; i++)
>+ if (stt->pages[i])
>+ __free_page(stt->pages[i]);
>+ kfree(stt);
>+ return -ENOMEM;
> }
>
> static void kvmppc_clear_tce(struct iommu_table *tbl, unsigned long
>entry)
>--
>2.13.5
>
>
>--
>
>Thanks,
>
>David
>
Update patch based on advice from David Hildenbrand and Paul Mackerras
---
arch/powerpc/kvm/book3s_64_vio.c | 13 +++++++++----
1 file changed, 9 insertions(+), 4 deletions(-)
diff --git a/arch/powerpc/kvm/book3s_64_vio.c b/arch/powerpc/kvm/book3s_64_vio.c
index a160c14..517594a 100644
--- a/arch/powerpc/kvm/book3s_64_vio.c
+++ b/arch/powerpc/kvm/book3s_64_vio.c
@@ -334,16 +334,21 @@ long kvm_vm_ioctl_create_spapr_tce(struct kvm *kvm,
goto fail;
}
- kvm_get_kvm(kvm);
-
mutex_lock(&kvm->lock);
list_add_rcu(&stt->list, &kvm->arch.spapr_tce_tables);
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) {
+ kvm_get_kvm(kvm);
+ return ret;
+ }
+ mutex_lock(&kvm->lock);
+ list_del_rcu(&stt->list);
+ mutex_unlock(&kvm->lock);
+ synchronize_rcu();
fail:
if (stt) {
for (i = 0; i < npages; i++)
--
^ permalink raw reply related [flat|nested] 35+ messages in thread
* Re: [PATCH] fix memory leak on kvm_vm_ioctl_create_spapr_tce
@ 2017-08-24 1:06 ` Nixiaoming
0 siblings, 0 replies; 35+ messages in thread
From: Nixiaoming @ 2017-08-24 1:06 UTC (permalink / raw)
To: David Hildenbrand, Paul Mackerras
Cc: agraf, pbonzini, rkrcmar, benh, mpe, kvm-ppc, kvm, linuxppc-dev,
linux-kernel
Pk9uIDIzLjA4LjIwMTcgMDg6MDYsIFBhdWwgTWFja2VycmFzIHdyb3RlOg0KPj4gT24gV2VkLCBB
dWcgMjMsIDIwMTcgYXQgMDE6NDM6MDhBTSArMDAwMCwgTml4aWFvbWluZyB3cm90ZToNCj4+Pj4g
T24gMjIuMDguMjAxNyAxNzoxNSwgRGF2aWQgSGlsZGVuYnJhbmQgd3JvdGU6DQo+Pj4+PiBPbiAy
Mi4wOC4yMDE3IDE2OjI4LCBuaXhpYW9taW5nIHdyb3RlOg0KPj4+Pj4+IG1pc3Mga2ZyZWUoc3R0
KSB3aGVuIGFub25faW5vZGVfZ2V0ZmQgcmV0dXJuIGZhaWwgc28gYWRkIGNoZWNrIA0KPj4+Pj4+
IGFub25faW5vZGVfZ2V0ZmQgcmV0dXJuIHZhbCwgYW5kIGtmcmVlIHN0dA0KPj4+Pj4+DQo+Pj4+
Pj4gU2lnbmVkLW9mZi1ieTogbml4aWFvbWluZyA8bml4aWFvbWluZ0BodWF3ZWkuY29tPg0KPj4+
Pj4+IC0tLQ0KPj4+Pj4+ICBhcmNoL3Bvd2VycGMva3ZtL2Jvb2szc182NF92aW8uYyB8IDUgKysr
Ky0NCj4+Pj4+PiAgMSBmaWxlIGNoYW5nZWQsIDQgaW5zZXJ0aW9ucygrKSwgMSBkZWxldGlvbigt
KQ0KPj4+Pj4+DQo+Pj4+Pj4gZGlmZiAtLWdpdCBhL2FyY2gvcG93ZXJwYy9rdm0vYm9vazNzXzY0
X3Zpby5jDQo+Pj4+Pj4gYi9hcmNoL3Bvd2VycGMva3ZtL2Jvb2szc182NF92aW8uYw0KPj4+Pj4+
IGluZGV4IGExNjBjMTQuLmEwYjQ0NTkgMTAwNjQ0DQo+Pj4+Pj4gLS0tIGEvYXJjaC9wb3dlcnBj
L2t2bS9ib29rM3NfNjRfdmlvLmMNCj4+Pj4+PiArKysgYi9hcmNoL3Bvd2VycGMva3ZtL2Jvb2sz
c182NF92aW8uYw0KPj4+Pj4+IEBAIC0zNDEsOCArMzQxLDExIEBAIGxvbmcga3ZtX3ZtX2lvY3Rs
X2NyZWF0ZV9zcGFwcl90Y2Uoc3RydWN0IA0KPj4+Pj4+IGt2bSAqa3ZtLA0KPj4+Pj4+ICANCj4+
Pj4+PiAgCW11dGV4X3VubG9jaygma3ZtLT5sb2NrKTsNCj4+Pj4+PiAgDQo+Pj4+Pj4gLQlyZXR1
cm4gYW5vbl9pbm9kZV9nZXRmZCgia3ZtLXNwYXByLXRjZSIsICZrdm1fc3BhcHJfdGNlX2ZvcHMs
DQo+Pj4+Pj4gKwlyZXQgPSBhbm9uX2lub2RlX2dldGZkKCJrdm0tc3BhcHItdGNlIiwgJmt2bV9z
cGFwcl90Y2VfZm9wcywNCj4+Pj4+PiAgCQkJCXN0dCwgT19SRFdSIHwgT19DTE9FWEVDKTsNCj4+
Pj4+PiArCWlmIChyZXQgPCAwKQ0KPj4+Pj4+ICsJCWdvdG8gZmFpbDsNCj4+Pj4+PiArCXJldHVy
biByZXQ7DQo+Pj4+Pj4gIA0KPj4+Pj4+ICBmYWlsOg0KPj4+Pj4+ICAJaWYgKHN0dCkgew0KPj4+
Pj4+DQo+Pj4+Pg0KPj4+Pj4NCj4+Pj4+IHN0dCBoYXMgYWxyZWFkeSBiZWVuIGFkZGVkIHRvIGt2
bS0+YXJjaC5zcGFwcl90Y2VfdGFibGVzLCBzbyANCj4+Pj4+IGZyZWVpbmcgaXQgaXMgZXZpbCBJ
TUhPLiBJIGRvbid0IGtub3cgdGhhdCBjb2RlLCBzbyBJIGRvbid0IGtub3cgDQo+Pj4+PiBpZiB0
aGVyZSBpcyBzb21lIG90aGVyIHBsYWNlIHRoYXQgd2lsbCBtYWtlIHN1cmUgdGhhdCBldmVyeXRo
aW5nIA0KPj4+Pj4gaW4NCj4+Pj4+IGt2bS0+YXJjaC5zcGFwcl90Y2VfdGFibGVzIHdpbGwgcHJv
cGVybHkgZ2V0IGZyZWVkLCBldmVuIHdoZW4gbm8gDQo+Pj4+PiBrdm0tPnJlbGVhc2UNCj4+Pj4+
IGZ1bmN0aW9uIGhhcyBiZWVuIGNhbGxlZCAoa3ZtX3NwYXByX3RjZV9yZWxlYXNlKS4NCj4+Pj4+
DQo+Pj4+DQo+Pj4+IElmIGl0IGlzIHJlYWxseSBub3QgZnJlZWQsIHRoYW4gYWxzbyBrdm1fcHV0
X2t2bShzdHQtPmt2bSkgaXMgbWlzc2luZy4NCj4+Pj4NCj4+Pj4gLS0NCj4+Pj4NCj4+Pj4gVGhh
bmtzLA0KPj4+Pg0KPj4+PiBEYXZpZA0KPj4+Pg0KPj4+DQo+Pj4gaWYgKCFzdHQpIHJldHVybiAt
RU5PTUVNOw0KPj4+IGt2bV9nZXRfa3ZtKGt2bSk7DQo+Pj4gaWYgYW5vbl9pbm9kZV9nZXRmZCBy
ZXR1cm4gLUVOT01FTQ0KPj4+IFRoZSB1c2VyIGNhbiBub3QgZGV0ZXJtaW5lIHdoZXRoZXIga3Zt
X2dldF9rdm0gaGFzIGJlZW4gY2FsbGVkIHNvIA0KPj4+IG5lZWQgYWRkIGt2bV9wZXRfa3ZtIHdo
ZW4gYW5vbl9pbm9kZV9nZXRmZCBmYWlsDQo+Pj4NCj4+PiBzdHQgaGFzIGFscmVhZHkgYmVlbiBh
ZGRlZCB0byBrdm0tPmFyY2guc3BhcHJfdGNlX3RhYmxlcywgYnV0IGlmIA0KPj4+IGFub25faW5v
ZGVfZ2V0ZmQgZmFpbCwgc3R0IGlzIHVudXNlZCB2YWwsIHNvIGNhbGwgbGlzdF9kZWxfcmN1LCBh
bmQgIA0KPj4+IGZyZWUgYXMgcXVpY2tseSBhcyBwb3NzaWJsZQ0KPj4+DQo+Pj4gbmV3IHBhdGNo
Og0KPj4+DQo+Pj4gLS0tDQo+Pj4gIGFyY2gvcG93ZXJwYy9rdm0vYm9vazNzXzY0X3Zpby5jIHwg
MTAgKysrKysrKysrLQ0KPj4+ICAxIGZpbGUgY2hhbmdlZCwgOSBpbnNlcnRpb25zKCspLCAxIGRl
bGV0aW9uKC0pDQo+Pj4NCj4+PiBkaWZmIC0tZ2l0IGEvYXJjaC9wb3dlcnBjL2t2bS9ib29rM3Nf
NjRfdmlvLmMgDQo+Pj4gYi9hcmNoL3Bvd2VycGMva3ZtL2Jvb2szc182NF92aW8uYw0KPj4+IGlu
ZGV4IGExNjBjMTQuLmUyMjI4ZjEgMTAwNjQ0DQo+Pj4gLS0tIGEvYXJjaC9wb3dlcnBjL2t2bS9i
b29rM3NfNjRfdmlvLmMNCj4+PiArKysgYi9hcmNoL3Bvd2VycGMva3ZtL2Jvb2szc182NF92aW8u
Yw0KPj4+IEBAIC0zNDEsOCArMzQxLDE2IEBAIGxvbmcga3ZtX3ZtX2lvY3RsX2NyZWF0ZV9zcGFw
cl90Y2Uoc3RydWN0IGt2bSANCj4+PiAqa3ZtLA0KPj4+DQo+Pj4gICAgICAgICBtdXRleF91bmxv
Y2soJmt2bS0+bG9jayk7DQo+Pj4NCj4+PiAtICAgICAgIHJldHVybiBhbm9uX2lub2RlX2dldGZk
KCJrdm0tc3BhcHItdGNlIiwgJmt2bV9zcGFwcl90Y2VfZm9wcywNCj4+PiArICAgICAgIHJldCA9
IGFub25faW5vZGVfZ2V0ZmQoImt2bS1zcGFwci10Y2UiLCAma3ZtX3NwYXByX3RjZV9mb3BzLA0K
Pj4+ICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgc3R0LCBPX1JEV1IgfCBPX0NMT0VY
RUMpOw0KPj4+ICsgICAgICAgaWYgKHJldCA8IDApIHsNCj4+PiArICAgICAgICAgICAgICAgbXV0
ZXhfbG9jaygma3ZtLT5sb2NrKTsNCj4+PiArICAgICAgICAgICAgICAgbGlzdF9kZWxfcmN1KCZz
dHQtPmxpc3QpOw0KPg0KPi4uLiBkb24ndCB3ZSBoYXZlIHRvIHRha2UgY2FyZSBvZiByY3Ugc3lu
Y2hyb25pemF0aW9uIGJlZm9yZSBmcmVlaW5nIGl0Pw0KPg0KPj4+ICsgICAgICAgICAgICAgICBt
dXRleF91bmxvY2soJmt2bS0+bG9jayk7DQo+Pj4gKyAgICAgICAgICAgICAgIGt2bV9wdXRfa3Zt
KGt2bSk7DQo+Pj4gKyAgICAgICAgICAgICAgIGdvdG8gZmFpbDsNCj4+PiArICAgICAgIH0NCj4+
PiArICAgICAgIHJldHVybiByZXQ7DQo+DQo+b2Ygc2ltcGx5DQo+DQo+aWYgKCFyZXQpDQo+CXJl
dHVybiAwOw0KPg0KPm11dGV4X2xvY2soJmt2bS0+bG9jayk7DQo+bGlzdF9kZWxfcmN1KCZzdHQt
Pmxpc3QpOw0KPm11dGV4X3VubG9jaygma3ZtLT5sb2NrKTsNCj5rdm1fcHV0X2t2bShrdm0pOw0K
Pg0KPg0KPj4gDQo+PiBJdCBzZWVtcyB0byBtZSB0aGF0IGl0IHdvdWxkIGJlIGJldHRlciB0byBk
byB0aGUgYW5vbl9pbm9kZV9nZXRmZCgpIA0KPj4gY2FsbCBiZWZvcmUgdGhlIGt2bV9nZXRfa3Zt
KCkgY2FsbCwgYW5kIGdvIHRvIHRoZSBmYWlsIGxhYmVsIGlmIGl0IA0KPj4gZmFpbHMuDQo+DQo+
SSB3b3VsZCBoYXZlIHN1Z2dlc3RlZCB0byBub3QgYWRkIGl0IHRvIHRoZSBsaXN0IGJlZm9yZSBp
dCBoYXMgYmVlbiANCj5mdWxseSBjcmVhdGVkIChzbyBub2JvZHkgY2FuIGhhdmUgYWNjZXNzIHRv
IGl0KS4gQnV0IEkgZ3Vlc3MgdGhhbiB3ZSANCj5uZWVkIGFub3RoZXIgbGV2ZWwgb2YgcHJvdGVj
dGlvbihlLmcuIGt2bS0+bG9jaykuDQo+DQo+QW0gSSBtaXNzaW5nIHNvbWV0aGluZywgb3IgaXMg
a3ZtX3ZtX2lvY3RsX2NyZWF0ZV9zcGFwcl90Y2UoKSByYWN5Pw0KPg0KPlRoZSAtRUJVU1kgY2hl
Y2sgaXMgZG9uZSB3aXRob3V0IGFueSBsb2NraW5nLCBzbyB0d28gcGFyYWxsZWwgY3JlYXRvcnMg
DQo+Y291bGQgY3JlYXRlIGFuIGluY29uc2lzdGVuY3ksIG5vPyBTaG91bGRuJ3QgdGhpcyBhbGwg
YmUgcHJvdGVjdGVkIGJ5DQo+a3ZtLT5sb2NrPw0KPg0KPj4gDQo+PiBQYXVsLg0KPj4gDQo+DQo+
SW5kZXBlbmRlbnQgb2YgdGhlIGZpeCwgSSdkIHN1Z2dlc3QgdGhlIGZvbGxvd2luZyBjbGVhbnVw
Lg0KPg0KPg0KPkZyb20gOTc5ZjU1MDgzZWU5NjVlMjU4MjdhODc0M2U4YTlmZGI4NTIzMWE2ZiBN
b24gU2VwIDE3IDAwOjAwOjAwIDIwMDENCj5Gcm9tOiBEYXZpZCBIaWxkZW5icmFuZCA8ZGF2aWRA
cmVkaGF0LmNvbT4NCj5EYXRlOiBXZWQsIDIzIEF1ZyAyMDE3IDEwOjA4OjU4ICswMjAwDQo+U3Vi
amVjdDogW1BBVENIIHYxIDEvMV0gS1ZNOiBQUEM6IGNsZWFudXAga3ZtX3ZtX2lvY3RsX2NyZWF0
ZV9zcGFwcl90Y2UNCj4NCj5MZXQncyBzaW1wbGlmeSBlcnJvciBoYW5kbGluZy4NCj4NCj5TaWdu
ZWQtb2ZmLWJ5OiBEYXZpZCBIaWxkZW5icmFuZCA8ZGF2aWRAcmVkaGF0LmNvbT4NCj4tLS0NCj4g
YXJjaC9wb3dlcnBjL2t2bS9ib29rM3NfNjRfdmlvLmMgfCAyOSArKysrKysrKysrKy0tLS0tLS0t
LS0tLS0tLS0tLQ0KPiAxIGZpbGUgY2hhbmdlZCwgMTEgaW5zZXJ0aW9ucygrKSwgMTggZGVsZXRp
b25zKC0pDQo+DQo+ZGlmZiAtLWdpdCBhL2FyY2gvcG93ZXJwYy9rdm0vYm9vazNzXzY0X3Zpby5j
DQo+Yi9hcmNoL3Bvd2VycGMva3ZtL2Jvb2szc182NF92aW8uYw0KPmluZGV4IGExNjBjMTQzMDRl
Yi4uNmJhYzQ5MjkyMjk2IDEwMDY0NA0KPi0tLSBhL2FyY2gvcG93ZXJwYy9rdm0vYm9vazNzXzY0
X3Zpby5jDQo+KysrIGIvYXJjaC9wb3dlcnBjL2t2bS9ib29rM3NfNjRfdmlvLmMNCj5AQCAtMjk1
LDggKzI5NSw3IEBAIGxvbmcga3ZtX3ZtX2lvY3RsX2NyZWF0ZV9zcGFwcl90Y2Uoc3RydWN0IGt2
bSAqa3ZtLCAgDQo+ew0KPiAJc3RydWN0IGt2bXBwY19zcGFwcl90Y2VfdGFibGUgKnN0dCA9IE5V
TEw7DQo+IAl1bnNpZ25lZCBsb25nIG5wYWdlcywgc2l6ZTsNCj4tCWludCByZXQgPSAtRU5PTUVN
Ow0KPi0JaW50IGk7DQo+KwlpbnQgaSwgcmV0Ow0KPg0KPiAJaWYgKCFhcmdzLT5zaXplKQ0KPiAJ
CXJldHVybiAtRUlOVkFMOw0KPkBAIC0zMTAsMTYgKzMwOSwxMyBAQCBsb25nIGt2bV92bV9pb2N0
bF9jcmVhdGVfc3BhcHJfdGNlKHN0cnVjdCBrdm0gKmt2bSwNCj4gCXNpemUgPSBfQUxJR05fVVAo
YXJncy0+c2l6ZSwgUEFHRV9TSVpFID4+IDMpOw0KPiAJbnBhZ2VzID0ga3ZtcHBjX3RjZV9wYWdl
cyhzaXplKTsNCj4gCXJldCA9IGt2bXBwY19hY2NvdW50X21lbWxpbWl0KGt2bXBwY19zdHRfcGFn
ZXMobnBhZ2VzKSwgdHJ1ZSk7DQo+LQlpZiAocmV0KSB7DQo+LQkJc3R0ID0gTlVMTDsNCj4tCQln
b3RvIGZhaWw7DQo+LQl9DQo+KwlpZiAocmV0KQ0KPisJCXJldHVybiByZXQ7DQo+DQo+LQlyZXQg
PSAtRU5PTUVNOw0KPiAJc3R0ID0ga3phbGxvYyhzaXplb2YoKnN0dCkgKyBucGFnZXMgKiBzaXpl
b2Yoc3RydWN0IHBhZ2UgKiksDQo+IAkJICAgICAgR0ZQX0tFUk5FTCk7DQo+IAlpZiAoIXN0dCkN
Cj4tCQlnb3RvIGZhaWw7DQo+KwkJcmV0dXJuIC1FTk9NRU07DQo+DQo+IAlzdHQtPmxpb2JuID0g
YXJncy0+bGlvYm47DQo+IAlzdHQtPnBhZ2Vfc2hpZnQgPSBhcmdzLT5wYWdlX3NoaWZ0Ow0KPkBA
IC0zMzEsNyArMzI3LDcgQEAgbG9uZyBrdm1fdm1faW9jdGxfY3JlYXRlX3NwYXByX3RjZShzdHJ1
Y3Qga3ZtICprdm0sDQo+IAlmb3IgKGkgPSAwOyBpIDwgbnBhZ2VzOyBpKyspIHsNCj4gCQlzdHQt
PnBhZ2VzW2ldID0gYWxsb2NfcGFnZShHRlBfS0VSTkVMIHwgX19HRlBfWkVSTyk7DQo+IAkJaWYg
KCFzdHQtPnBhZ2VzW2ldKQ0KPi0JCQlnb3RvIGZhaWw7DQo+KwkJCWdvdG8gZmFpbF9mcmVlOw0K
PiAJfQ0KPg0KPiAJa3ZtX2dldF9rdm0oa3ZtKTsNCj5AQCAtMzQ0LDE1ICszNDAsMTIgQEAgbG9u
ZyBrdm1fdm1faW9jdGxfY3JlYXRlX3NwYXByX3RjZShzdHJ1Y3Qga3ZtICprdm0sDQo+IAlyZXR1
cm4gYW5vbl9pbm9kZV9nZXRmZCgia3ZtLXNwYXByLXRjZSIsICZrdm1fc3BhcHJfdGNlX2ZvcHMs
DQo+IAkJCQlzdHQsIE9fUkRXUiB8IE9fQ0xPRVhFQyk7DQo+DQo+LWZhaWw6DQo+LQlpZiAoc3R0
KSB7DQo+LQkJZm9yIChpID0gMDsgaSA8IG5wYWdlczsgaSsrKQ0KPi0JCQlpZiAoc3R0LT5wYWdl
c1tpXSkNCj4tCQkJCV9fZnJlZV9wYWdlKHN0dC0+cGFnZXNbaV0pOw0KPi0NCj4tCQlrZnJlZShz
dHQpOw0KPi0JfQ0KPi0JcmV0dXJuIHJldDsNCj4rZmFpbF9mcmVlOg0KPisJZm9yIChpID0gMDsg
aSA8IG5wYWdlczsgaSsrKQ0KPisJCWlmIChzdHQtPnBhZ2VzW2ldKQ0KPisJCQlfX2ZyZWVfcGFn
ZShzdHQtPnBhZ2VzW2ldKTsNCj4rCWtmcmVlKHN0dCk7DQo+KwlyZXR1cm4gLUVOT01FTTsNCj4g
fQ0KPg0KPiBzdGF0aWMgdm9pZCBrdm1wcGNfY2xlYXJfdGNlKHN0cnVjdCBpb21tdV90YWJsZSAq
dGJsLCB1bnNpZ25lZCBsb25nIA0KPmVudHJ5KQ0KPi0tDQo+Mi4xMy41DQo+DQo+DQo+LS0NCj4N
Cj5UaGFua3MsDQo+DQo+RGF2aWQNCj4NCg0KVXBkYXRlIHBhdGNoIGJhc2VkIG9uIGFkdmljZSBm
cm9tIERhdmlkIEhpbGRlbmJyYW5kIGFuZCBQYXVsIE1hY2tlcnJhcw0KDQotLS0NCiBhcmNoL3Bv
d2VycGMva3ZtL2Jvb2szc182NF92aW8uYyB8IDEzICsrKysrKysrKy0tLS0NCiAxIGZpbGUgY2hh
bmdlZCwgOSBpbnNlcnRpb25zKCspLCA0IGRlbGV0aW9ucygtKQ0KDQpkaWZmIC0tZ2l0IGEvYXJj
aC9wb3dlcnBjL2t2bS9ib29rM3NfNjRfdmlvLmMgYi9hcmNoL3Bvd2VycGMva3ZtL2Jvb2szc182
NF92aW8uYw0KaW5kZXggYTE2MGMxNC4uNTE3NTk0YSAxMDA2NDQNCi0tLSBhL2FyY2gvcG93ZXJw
Yy9rdm0vYm9vazNzXzY0X3Zpby5jDQorKysgYi9hcmNoL3Bvd2VycGMva3ZtL2Jvb2szc182NF92
aW8uYw0KQEAgLTMzNCwxNiArMzM0LDIxIEBAIGxvbmcga3ZtX3ZtX2lvY3RsX2NyZWF0ZV9zcGFw
cl90Y2Uoc3RydWN0IGt2bSAqa3ZtLA0KIAkJCWdvdG8gZmFpbDsNCiAJfQ0KIA0KLQlrdm1fZ2V0
X2t2bShrdm0pOw0KLQ0KIAltdXRleF9sb2NrKCZrdm0tPmxvY2spOw0KIAlsaXN0X2FkZF9yY3Uo
JnN0dC0+bGlzdCwgJmt2bS0+YXJjaC5zcGFwcl90Y2VfdGFibGVzKTsNCiANCiAJbXV0ZXhfdW5s
b2NrKCZrdm0tPmxvY2spOw0KIA0KLQlyZXR1cm4gYW5vbl9pbm9kZV9nZXRmZCgia3ZtLXNwYXBy
LXRjZSIsICZrdm1fc3BhcHJfdGNlX2ZvcHMsDQorCXJldCA9IGFub25faW5vZGVfZ2V0ZmQoImt2
bS1zcGFwci10Y2UiLCAma3ZtX3NwYXByX3RjZV9mb3BzLA0KIAkJCQlzdHQsIE9fUkRXUiB8IE9f
Q0xPRVhFQyk7DQotDQorCWlmIChyZXQgPj0gMCkgew0KKwkJa3ZtX2dldF9rdm0oa3ZtKTsNCisJ
CXJldHVybiByZXQ7DQorCX0NCisJbXV0ZXhfbG9jaygma3ZtLT5sb2NrKTsNCisJbGlzdF9kZWxf
cmN1KCZzdHQtPmxpc3QpOw0KKwltdXRleF91bmxvY2soJmt2bS0+bG9jayk7DQorCXN5bmNocm9u
aXplX3JjdSgpOw0KIGZhaWw6DQogCWlmIChzdHQpIHsNCiAJCWZvciAoaSA9IDA7IGkgPCBucGFn
ZXM7IGkrKykNCi0tIA0KDQoNCg==
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH] fix memory leak on kvm_vm_ioctl_create_spapr_tce
@ 2017-08-24 1:06 ` Nixiaoming
0 siblings, 0 replies; 35+ messages in thread
From: Nixiaoming @ 2017-08-24 1:06 UTC (permalink / raw)
To: David Hildenbrand, Paul Mackerras
Cc: agraf, pbonzini, rkrcmar, benh, mpe, kvm-ppc, kvm, linuxppc-dev,
linux-kernel
Pk9uIDIzLjA4LjIwMTcgMDg6MDYsIFBhdWwgTWFja2VycmFzIHdyb3RlOg0KPj4gT24gV2VkLCBB
dWcgMjMsIDIwMTcgYXQgMDE6NDM6MDhBTSArMDAwMCwgTml4aWFvbWluZyB3cm90ZToNCj4+Pj4g
T24gMjIuMDguMjAxNyAxNzoxNSwgRGF2aWQgSGlsZGVuYnJhbmQgd3JvdGU6DQo+Pj4+PiBPbiAy
Mi4wOC4yMDE3IDE2OjI4LCBuaXhpYW9taW5nIHdyb3RlOg0KPj4+Pj4+IG1pc3Mga2ZyZWUoc3R0
KSB3aGVuIGFub25faW5vZGVfZ2V0ZmQgcmV0dXJuIGZhaWwgc28gYWRkIGNoZWNrIA0KPj4+Pj4+
IGFub25faW5vZGVfZ2V0ZmQgcmV0dXJuIHZhbCwgYW5kIGtmcmVlIHN0dA0KPj4+Pj4+DQo+Pj4+
Pj4gU2lnbmVkLW9mZi1ieTogbml4aWFvbWluZyA8bml4aWFvbWluZ0BodWF3ZWkuY29tPg0KPj4+
Pj4+IC0tLQ0KPj4+Pj4+ICBhcmNoL3Bvd2VycGMva3ZtL2Jvb2szc182NF92aW8uYyB8IDUgKysr
Ky0NCj4+Pj4+PiAgMSBmaWxlIGNoYW5nZWQsIDQgaW5zZXJ0aW9ucygrKSwgMSBkZWxldGlvbigt
KQ0KPj4+Pj4+DQo+Pj4+Pj4gZGlmZiAtLWdpdCBhL2FyY2gvcG93ZXJwYy9rdm0vYm9vazNzXzY0
X3Zpby5jDQo+Pj4+Pj4gYi9hcmNoL3Bvd2VycGMva3ZtL2Jvb2szc182NF92aW8uYw0KPj4+Pj4+
IGluZGV4IGExNjBjMTQuLmEwYjQ0NTkgMTAwNjQ0DQo+Pj4+Pj4gLS0tIGEvYXJjaC9wb3dlcnBj
L2t2bS9ib29rM3NfNjRfdmlvLmMNCj4+Pj4+PiArKysgYi9hcmNoL3Bvd2VycGMva3ZtL2Jvb2sz
c182NF92aW8uYw0KPj4+Pj4+IEBAIC0zNDEsOCArMzQxLDExIEBAIGxvbmcga3ZtX3ZtX2lvY3Rs
X2NyZWF0ZV9zcGFwcl90Y2Uoc3RydWN0IA0KPj4+Pj4+IGt2bSAqa3ZtLA0KPj4+Pj4+ICANCj4+
Pj4+PiAgCW11dGV4X3VubG9jaygma3ZtLT5sb2NrKTsNCj4+Pj4+PiAgDQo+Pj4+Pj4gLQlyZXR1
cm4gYW5vbl9pbm9kZV9nZXRmZCgia3ZtLXNwYXByLXRjZSIsICZrdm1fc3BhcHJfdGNlX2ZvcHMs
DQo+Pj4+Pj4gKwlyZXQgPSBhbm9uX2lub2RlX2dldGZkKCJrdm0tc3BhcHItdGNlIiwgJmt2bV9z
cGFwcl90Y2VfZm9wcywNCj4+Pj4+PiAgCQkJCXN0dCwgT19SRFdSIHwgT19DTE9FWEVDKTsNCj4+
Pj4+PiArCWlmIChyZXQgPCAwKQ0KPj4+Pj4+ICsJCWdvdG8gZmFpbDsNCj4+Pj4+PiArCXJldHVy
biByZXQ7DQo+Pj4+Pj4gIA0KPj4+Pj4+ICBmYWlsOg0KPj4+Pj4+ICAJaWYgKHN0dCkgew0KPj4+
Pj4+DQo+Pj4+Pg0KPj4+Pj4NCj4+Pj4+IHN0dCBoYXMgYWxyZWFkeSBiZWVuIGFkZGVkIHRvIGt2
bS0+YXJjaC5zcGFwcl90Y2VfdGFibGVzLCBzbyANCj4+Pj4+IGZyZWVpbmcgaXQgaXMgZXZpbCBJ
TUhPLiBJIGRvbid0IGtub3cgdGhhdCBjb2RlLCBzbyBJIGRvbid0IGtub3cgDQo+Pj4+PiBpZiB0
aGVyZSBpcyBzb21lIG90aGVyIHBsYWNlIHRoYXQgd2lsbCBtYWtlIHN1cmUgdGhhdCBldmVyeXRo
aW5nIA0KPj4+Pj4gaW4NCj4+Pj4+IGt2bS0+YXJjaC5zcGFwcl90Y2VfdGFibGVzIHdpbGwgcHJv
cGVybHkgZ2V0IGZyZWVkLCBldmVuIHdoZW4gbm8gDQo+Pj4+PiBrdm0tPnJlbGVhc2UNCj4+Pj4+
IGZ1bmN0aW9uIGhhcyBiZWVuIGNhbGxlZCAoa3ZtX3NwYXByX3RjZV9yZWxlYXNlKS4NCj4+Pj4+
DQo+Pj4+DQo+Pj4+IElmIGl0IGlzIHJlYWxseSBub3QgZnJlZWQsIHRoYW4gYWxzbyBrdm1fcHV0
X2t2bShzdHQtPmt2bSkgaXMgbWlzc2luZy4NCj4+Pj4NCj4+Pj4gLS0NCj4+Pj4NCj4+Pj4gVGhh
bmtzLA0KPj4+Pg0KPj4+PiBEYXZpZA0KPj4+Pg0KPj4+DQo+Pj4gaWYgKCFzdHQpIHJldHVybiAt
RU5PTUVNOw0KPj4+IGt2bV9nZXRfa3ZtKGt2bSk7DQo+Pj4gaWYgYW5vbl9pbm9kZV9nZXRmZCBy
ZXR1cm4gLUVOT01FTQ0KPj4+IFRoZSB1c2VyIGNhbiBub3QgZGV0ZXJtaW5lIHdoZXRoZXIga3Zt
X2dldF9rdm0gaGFzIGJlZW4gY2FsbGVkIHNvIA0KPj4+IG5lZWQgYWRkIGt2bV9wZXRfa3ZtIHdo
ZW4gYW5vbl9pbm9kZV9nZXRmZCBmYWlsDQo+Pj4NCj4+PiBzdHQgaGFzIGFscmVhZHkgYmVlbiBh
ZGRlZCB0byBrdm0tPmFyY2guc3BhcHJfdGNlX3RhYmxlcywgYnV0IGlmIA0KPj4+IGFub25faW5v
ZGVfZ2V0ZmQgZmFpbCwgc3R0IGlzIHVudXNlZCB2YWwsIHNvIGNhbGwgbGlzdF9kZWxfcmN1LCBh
bmQgIA0KPj4+IGZyZWUgYXMgcXVpY2tseSBhcyBwb3NzaWJsZQ0KPj4+DQo+Pj4gbmV3IHBhdGNo
Og0KPj4+DQo+Pj4gLS0tDQo+Pj4gIGFyY2gvcG93ZXJwYy9rdm0vYm9vazNzXzY0X3Zpby5jIHwg
MTAgKysrKysrKysrLQ0KPj4+ICAxIGZpbGUgY2hhbmdlZCwgOSBpbnNlcnRpb25zKCspLCAxIGRl
bGV0aW9uKC0pDQo+Pj4NCj4+PiBkaWZmIC0tZ2l0IGEvYXJjaC9wb3dlcnBjL2t2bS9ib29rM3Nf
NjRfdmlvLmMgDQo+Pj4gYi9hcmNoL3Bvd2VycGMva3ZtL2Jvb2szc182NF92aW8uYw0KPj4+IGlu
ZGV4IGExNjBjMTQuLmUyMjI4ZjEgMTAwNjQ0DQo+Pj4gLS0tIGEvYXJjaC9wb3dlcnBjL2t2bS9i
b29rM3NfNjRfdmlvLmMNCj4+PiArKysgYi9hcmNoL3Bvd2VycGMva3ZtL2Jvb2szc182NF92aW8u
Yw0KPj4+IEBAIC0zNDEsOCArMzQxLDE2IEBAIGxvbmcga3ZtX3ZtX2lvY3RsX2NyZWF0ZV9zcGFw
cl90Y2Uoc3RydWN0IGt2bSANCj4+PiAqa3ZtLA0KPj4+DQo+Pj4gICAgICAgICBtdXRleF91bmxv
Y2soJmt2bS0+bG9jayk7DQo+Pj4NCj4+PiAtICAgICAgIHJldHVybiBhbm9uX2lub2RlX2dldGZk
KCJrdm0tc3BhcHItdGNlIiwgJmt2bV9zcGFwcl90Y2VfZm9wcywNCj4+PiArICAgICAgIHJldCA9
IGFub25faW5vZGVfZ2V0ZmQoImt2bS1zcGFwci10Y2UiLCAma3ZtX3NwYXByX3RjZV9mb3BzLA0K
Pj4+ICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgc3R0LCBPX1JEV1IgfCBPX0NMT0VY
RUMpOw0KPj4+ICsgICAgICAgaWYgKHJldCA8IDApIHsNCj4+PiArICAgICAgICAgICAgICAgbXV0
ZXhfbG9jaygma3ZtLT5sb2NrKTsNCj4+PiArICAgICAgICAgICAgICAgbGlzdF9kZWxfcmN1KCZz
dHQtPmxpc3QpOw0KPg0KPi4uLiBkb24ndCB3ZSBoYXZlIHRvIHRha2UgY2FyZSBvZiByY3Ugc3lu
Y2hyb25pemF0aW9uIGJlZm9yZSBmcmVlaW5nIGl0Pw0KPg0KPj4+ICsgICAgICAgICAgICAgICBt
dXRleF91bmxvY2soJmt2bS0+bG9jayk7DQo+Pj4gKyAgICAgICAgICAgICAgIGt2bV9wdXRfa3Zt
KGt2bSk7DQo+Pj4gKyAgICAgICAgICAgICAgIGdvdG8gZmFpbDsNCj4+PiArICAgICAgIH0NCj4+
PiArICAgICAgIHJldHVybiByZXQ7DQo+DQo+b2Ygc2ltcGx5DQo+DQo+aWYgKCFyZXQpDQo+CXJl
dHVybiAwOw0KPg0KPm11dGV4X2xvY2soJmt2bS0+bG9jayk7DQo+bGlzdF9kZWxfcmN1KCZzdHQt
Pmxpc3QpOw0KPm11dGV4X3VubG9jaygma3ZtLT5sb2NrKTsNCj5rdm1fcHV0X2t2bShrdm0pOw0K
Pg0KPg0KPj4gDQo+PiBJdCBzZWVtcyB0byBtZSB0aGF0IGl0IHdvdWxkIGJlIGJldHRlciB0byBk
byB0aGUgYW5vbl9pbm9kZV9nZXRmZCgpIA0KPj4gY2FsbCBiZWZvcmUgdGhlIGt2bV9nZXRfa3Zt
KCkgY2FsbCwgYW5kIGdvIHRvIHRoZSBmYWlsIGxhYmVsIGlmIGl0IA0KPj4gZmFpbHMuDQo+DQo+
SSB3b3VsZCBoYXZlIHN1Z2dlc3RlZCB0byBub3QgYWRkIGl0IHRvIHRoZSBsaXN0IGJlZm9yZSBp
dCBoYXMgYmVlbiANCj5mdWxseSBjcmVhdGVkIChzbyBub2JvZHkgY2FuIGhhdmUgYWNjZXNzIHRv
IGl0KS4gQnV0IEkgZ3Vlc3MgdGhhbiB3ZSANCj5uZWVkIGFub3RoZXIgbGV2ZWwgb2YgcHJvdGVj
dGlvbihlLmcuIGt2bS0+bG9jaykuDQo+DQo+QW0gSSBtaXNzaW5nIHNvbWV0aGluZywgb3IgaXMg
a3ZtX3ZtX2lvY3RsX2NyZWF0ZV9zcGFwcl90Y2UoKSByYWN5Pw0KPg0KPlRoZSAtRUJVU1kgY2hl
Y2sgaXMgZG9uZSB3aXRob3V0IGFueSBsb2NraW5nLCBzbyB0d28gcGFyYWxsZWwgY3JlYXRvcnMg
DQo+Y291bGQgY3JlYXRlIGFuIGluY29uc2lzdGVuY3ksIG5vPyBTaG91bGRuJ3QgdGhpcyBhbGwg
YmUgcHJvdGVjdGVkIGJ5DQo+a3ZtLT5sb2NrPw0KPg0KPj4gDQo+PiBQYXVsLg0KPj4gDQo+DQo+
SW5kZXBlbmRlbnQgb2YgdGhlIGZpeCwgSSdkIHN1Z2dlc3QgdGhlIGZvbGxvd2luZyBjbGVhbnVw
Lg0KPg0KPg0KPkZyb20gOTc5ZjU1MDgzZWU5NjVlMjU4MjdhODc0M2U4YTlmZGI4NTIzMWE2ZiBN
b24gU2VwIDE3IDAwOjAwOjAwIDIwMDENCj5Gcm9tOiBEYXZpZCBIaWxkZW5icmFuZCA8ZGF2aWRA
cmVkaGF0LmNvbT4NCj5EYXRlOiBXZWQsIDIzIEF1ZyAyMDE3IDEwOjA4OjU4ICswMjAwDQo+U3Vi
amVjdDogW1BBVENIIHYxIDEvMV0gS1ZNOiBQUEM6IGNsZWFudXAga3ZtX3ZtX2lvY3RsX2NyZWF0
ZV9zcGFwcl90Y2UNCj4NCj5MZXQncyBzaW1wbGlmeSBlcnJvciBoYW5kbGluZy4NCj4NCj5TaWdu
ZWQtb2ZmLWJ5OiBEYXZpZCBIaWxkZW5icmFuZCA8ZGF2aWRAcmVkaGF0LmNvbT4NCj4tLS0NCj4g
YXJjaC9wb3dlcnBjL2t2bS9ib29rM3NfNjRfdmlvLmMgfCAyOSArKysrKysrKysrKy0tLS0tLS0t
LS0tLS0tLS0tLQ0KPiAxIGZpbGUgY2hhbmdlZCwgMTEgaW5zZXJ0aW9ucygrKSwgMTggZGVsZXRp
b25zKC0pDQo+DQo+ZGlmZiAtLWdpdCBhL2FyY2gvcG93ZXJwYy9rdm0vYm9vazNzXzY0X3Zpby5j
DQo+Yi9hcmNoL3Bvd2VycGMva3ZtL2Jvb2szc182NF92aW8uYw0KPmluZGV4IGExNjBjMTQzMDRl
Yi4uNmJhYzQ5MjkyMjk2IDEwMDY0NA0KPi0tLSBhL2FyY2gvcG93ZXJwYy9rdm0vYm9vazNzXzY0
X3Zpby5jDQo+KysrIGIvYXJjaC9wb3dlcnBjL2t2bS9ib29rM3NfNjRfdmlvLmMNCj5AQCAtMjk1
LDggKzI5NSw3IEBAIGxvbmcga3ZtX3ZtX2lvY3RsX2NyZWF0ZV9zcGFwcl90Y2Uoc3RydWN0IGt2
bSAqa3ZtLCAgDQo+ew0KPiAJc3RydWN0IGt2bXBwY19zcGFwcl90Y2VfdGFibGUgKnN0dCA9IE5V
TEw7DQo+IAl1bnNpZ25lZCBsb25nIG5wYWdlcywgc2l6ZTsNCj4tCWludCByZXQgPSAtRU5PTUVN
Ow0KPi0JaW50IGk7DQo+KwlpbnQgaSwgcmV0Ow0KPg0KPiAJaWYgKCFhcmdzLT5zaXplKQ0KPiAJ
CXJldHVybiAtRUlOVkFMOw0KPkBAIC0zMTAsMTYgKzMwOSwxMyBAQCBsb25nIGt2bV92bV9pb2N0
bF9jcmVhdGVfc3BhcHJfdGNlKHN0cnVjdCBrdm0gKmt2bSwNCj4gCXNpemUgPSBfQUxJR05fVVAo
YXJncy0+c2l6ZSwgUEFHRV9TSVpFID4+IDMpOw0KPiAJbnBhZ2VzID0ga3ZtcHBjX3RjZV9wYWdl
cyhzaXplKTsNCj4gCXJldCA9IGt2bXBwY19hY2NvdW50X21lbWxpbWl0KGt2bXBwY19zdHRfcGFn
ZXMobnBhZ2VzKSwgdHJ1ZSk7DQo+LQlpZiAocmV0KSB7DQo+LQkJc3R0ID0gTlVMTDsNCj4tCQln
b3RvIGZhaWw7DQo+LQl9DQo+KwlpZiAocmV0KQ0KPisJCXJldHVybiByZXQ7DQo+DQo+LQlyZXQg
PSAtRU5PTUVNOw0KPiAJc3R0ID0ga3phbGxvYyhzaXplb2YoKnN0dCkgKyBucGFnZXMgKiBzaXpl
b2Yoc3RydWN0IHBhZ2UgKiksDQo+IAkJICAgICAgR0ZQX0tFUk5FTCk7DQo+IAlpZiAoIXN0dCkN
Cj4tCQlnb3RvIGZhaWw7DQo+KwkJcmV0dXJuIC1FTk9NRU07DQo+DQo+IAlzdHQtPmxpb2JuID0g
YXJncy0+bGlvYm47DQo+IAlzdHQtPnBhZ2Vfc2hpZnQgPSBhcmdzLT5wYWdlX3NoaWZ0Ow0KPkBA
IC0zMzEsNyArMzI3LDcgQEAgbG9uZyBrdm1fdm1faW9jdGxfY3JlYXRlX3NwYXByX3RjZShzdHJ1
Y3Qga3ZtICprdm0sDQo+IAlmb3IgKGkgPSAwOyBpIDwgbnBhZ2VzOyBpKyspIHsNCj4gCQlzdHQt
PnBhZ2VzW2ldID0gYWxsb2NfcGFnZShHRlBfS0VSTkVMIHwgX19HRlBfWkVSTyk7DQo+IAkJaWYg
KCFzdHQtPnBhZ2VzW2ldKQ0KPi0JCQlnb3RvIGZhaWw7DQo+KwkJCWdvdG8gZmFpbF9mcmVlOw0K
PiAJfQ0KPg0KPiAJa3ZtX2dldF9rdm0oa3ZtKTsNCj5AQCAtMzQ0LDE1ICszNDAsMTIgQEAgbG9u
ZyBrdm1fdm1faW9jdGxfY3JlYXRlX3NwYXByX3RjZShzdHJ1Y3Qga3ZtICprdm0sDQo+IAlyZXR1
cm4gYW5vbl9pbm9kZV9nZXRmZCgia3ZtLXNwYXByLXRjZSIsICZrdm1fc3BhcHJfdGNlX2ZvcHMs
DQo+IAkJCQlzdHQsIE9fUkRXUiB8IE9fQ0xPRVhFQyk7DQo+DQo+LWZhaWw6DQo+LQlpZiAoc3R0
KSB7DQo+LQkJZm9yIChpID0gMDsgaSA8IG5wYWdlczsgaSsrKQ0KPi0JCQlpZiAoc3R0LT5wYWdl
c1tpXSkNCj4tCQkJCV9fZnJlZV9wYWdlKHN0dC0+cGFnZXNbaV0pOw0KPi0NCj4tCQlrZnJlZShz
dHQpOw0KPi0JfQ0KPi0JcmV0dXJuIHJldDsNCj4rZmFpbF9mcmVlOg0KPisJZm9yIChpID0gMDsg
aSA8IG5wYWdlczsgaSsrKQ0KPisJCWlmIChzdHQtPnBhZ2VzW2ldKQ0KPisJCQlfX2ZyZWVfcGFn
ZShzdHQtPnBhZ2VzW2ldKTsNCj4rCWtmcmVlKHN0dCk7DQo+KwlyZXR1cm4gLUVOT01FTTsNCj4g
fQ0KPg0KPiBzdGF0aWMgdm9pZCBrdm1wcGNfY2xlYXJfdGNlKHN0cnVjdCBpb21tdV90YWJsZSAq
dGJsLCB1bnNpZ25lZCBsb25nIA0KPmVudHJ5KQ0KPi0tDQo+Mi4xMy41DQo+DQo+DQo+LS0NCj4N
Cj5UaGFua3MsDQo+DQo+RGF2aWQNCj4NCg0KVXBkYXRlIHBhdGNoIGJhc2VkIG9uIGFkdmljZSBm
cm9tIERhdmlkIEhpbGRlbmJyYW5kIGFuZCBQYXVsIE1hY2tlcnJhcw0KDQotLS0NCiBhcmNoL3Bv
d2VycGMva3ZtL2Jvb2szc182NF92aW8uYyB8IDEzICsrKysrKysrKy0tLS0NCiAxIGZpbGUgY2hh
bmdlZCwgOSBpbnNlcnRpb25zKCspLCA0IGRlbGV0aW9ucygtKQ0KDQpkaWZmIC0tZ2l0IGEvYXJj
aC9wb3dlcnBjL2t2bS9ib29rM3NfNjRfdmlvLmMgYi9hcmNoL3Bvd2VycGMva3ZtL2Jvb2szc182
NF92aW8uYw0KaW5kZXggYTE2MGMxNC4uNTE3NTk0YSAxMDA2NDQNCi0tLSBhL2FyY2gvcG93ZXJw
Yy9rdm0vYm9vazNzXzY0X3Zpby5jDQorKysgYi9hcmNoL3Bvd2VycGMva3ZtL2Jvb2szc182NF92
aW8uYw0KQEAgLTMzNCwxNiArMzM0LDIxIEBAIGxvbmcga3ZtX3ZtX2lvY3RsX2NyZWF0ZV9zcGFw
cl90Y2Uoc3RydWN0IGt2bSAqa3ZtLA0KIAkJCWdvdG8gZmFpbDsNCiAJfQ0KIA0KLQlrdm1fZ2V0
X2t2bShrdm0pOw0KLQ0KIAltdXRleF9sb2NrKCZrdm0tPmxvY2spOw0KIAlsaXN0X2FkZF9yY3Uo
JnN0dC0+bGlzdCwgJmt2bS0+YXJjaC5zcGFwcl90Y2VfdGFibGVzKTsNCiANCiAJbXV0ZXhfdW5s
b2NrKCZrdm0tPmxvY2spOw0KIA0KLQlyZXR1cm4gYW5vbl9pbm9kZV9nZXRmZCgia3ZtLXNwYXBy
LXRjZSIsICZrdm1fc3BhcHJfdGNlX2ZvcHMsDQorCXJldCA9IGFub25faW5vZGVfZ2V0ZmQoImt2
bS1zcGFwci10Y2UiLCAma3ZtX3NwYXByX3RjZV9mb3BzLA0KIAkJCQlzdHQsIE9fUkRXUiB8IE9f
Q0xPRVhFQyk7DQotDQorCWlmIChyZXQgPj0gMCkgew0KKwkJa3ZtX2dldF9rdm0oa3ZtKTsNCisJ
CXJldHVybiByZXQ7DQorCX0NCisJbXV0ZXhfbG9jaygma3ZtLT5sb2NrKTsNCisJbGlzdF9kZWxf
cmN1KCZzdHQtPmxpc3QpOw0KKwltdXRleF91bmxvY2soJmt2bS0+bG9jayk7DQorCXN5bmNocm9u
aXplX3JjdSgpOw0KIGZhaWw6DQogCWlmIChzdHQpIHsNCiAJCWZvciAoaSA9IDA7IGkgPCBucGFn
ZXM7IGkrKykNCi0tIA0KDQoNCg=
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: Re: [PATCH] fix memory leak on kvm_vm_ioctl_create_spapr_tce
2017-08-23 6:06 ` Paul Mackerras
@ 2017-08-27 21:02 ` Al Viro
-1 siblings, 0 replies; 35+ messages in thread
From: Al Viro @ 2017-08-27 21:02 UTC (permalink / raw)
To: Paul Mackerras
Cc: Nixiaoming, David Hildenbrand, agraf, pbonzini, rkrcmar, benh,
mpe, kvm-ppc, kvm, linuxppc-dev, linux-kernel
On Wed, Aug 23, 2017 at 04:06:24PM +1000, Paul Mackerras wrote:
> It seems to me that it would be better to do the anon_inode_getfd()
> call before the kvm_get_kvm() call, and go to the fail label if it
> fails.
And what happens if another thread does close() on the (guessed) fd?
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: Re: [PATCH] fix memory leak on kvm_vm_ioctl_create_spapr_tce
@ 2017-08-27 21:02 ` Al Viro
0 siblings, 0 replies; 35+ messages in thread
From: Al Viro @ 2017-08-27 21:02 UTC (permalink / raw)
To: Paul Mackerras
Cc: Nixiaoming, David Hildenbrand, agraf, pbonzini, rkrcmar, benh,
mpe, kvm-ppc, kvm, linuxppc-dev, linux-kernel
On Wed, Aug 23, 2017 at 04:06:24PM +1000, Paul Mackerras wrote:
> It seems to me that it would be better to do the anon_inode_getfd()
> call before the kvm_get_kvm() call, and go to the fail label if it
> fails.
And what happens if another thread does close() on the (guessed) fd?
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: Re: [PATCH] fix memory leak on kvm_vm_ioctl_create_spapr_tce
2017-08-27 21:02 ` Al Viro
@ 2017-08-28 4:38 ` Paul Mackerras
-1 siblings, 0 replies; 35+ messages in thread
From: Paul Mackerras @ 2017-08-28 4:38 UTC (permalink / raw)
To: Al Viro
Cc: Nixiaoming, David Hildenbrand, agraf, pbonzini, rkrcmar, benh,
mpe, kvm-ppc, kvm, linuxppc-dev, linux-kernel
On Sun, Aug 27, 2017 at 10:02:20PM +0100, Al Viro wrote:
> On Wed, Aug 23, 2017 at 04:06:24PM +1000, Paul Mackerras wrote:
>
> > It seems to me that it would be better to do the anon_inode_getfd()
> > call before the kvm_get_kvm() call, and go to the fail label if it
> > fails.
>
> And what happens if another thread does close() on the (guessed) fd?
Chaos ensues, but mostly because we don't have proper mutual exclusion
on the modifications to the list. I'll add a mutex_lock/unlock to
kvm_spapr_tce_release() and move the anon_inode_getfd() call inside
the mutex.
It looks like the other possible uses of the fd (mmap, and passing it
as a parameter to the KVM_DEV_VFIO_GROUP_SET_SPAPR_TCE ioctl on a KVM
device fd) are safe.
Thanks,
Paul.
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: Re: [PATCH] fix memory leak on kvm_vm_ioctl_create_spapr_tce
@ 2017-08-28 4:38 ` Paul Mackerras
0 siblings, 0 replies; 35+ messages in thread
From: Paul Mackerras @ 2017-08-28 4:38 UTC (permalink / raw)
To: Al Viro
Cc: Nixiaoming, David Hildenbrand, agraf, pbonzini, rkrcmar, benh,
mpe, kvm-ppc, kvm, linuxppc-dev, linux-kernel
On Sun, Aug 27, 2017 at 10:02:20PM +0100, Al Viro wrote:
> On Wed, Aug 23, 2017 at 04:06:24PM +1000, Paul Mackerras wrote:
>
> > It seems to me that it would be better to do the anon_inode_getfd()
> > call before the kvm_get_kvm() call, and go to the fail label if it
> > fails.
>
> And what happens if another thread does close() on the (guessed) fd?
Chaos ensues, but mostly because we don't have proper mutual exclusion
on the modifications to the list. I'll add a mutex_lock/unlock to
kvm_spapr_tce_release() and move the anon_inode_getfd() call inside
the mutex.
It looks like the other possible uses of the fd (mmap, and passing it
as a parameter to the KVM_DEV_VFIO_GROUP_SET_SPAPR_TCE ioctl on a KVM
device fd) are safe.
Thanks,
Paul.
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: Re: [PATCH] fix memory leak on kvm_vm_ioctl_create_spapr_tce
2017-08-28 4:38 ` Paul Mackerras
@ 2017-08-28 5:28 ` Al Viro
-1 siblings, 0 replies; 35+ messages in thread
From: Al Viro @ 2017-08-28 5:28 UTC (permalink / raw)
To: Paul Mackerras
Cc: Nixiaoming, David Hildenbrand, agraf, pbonzini, rkrcmar, benh,
mpe, kvm-ppc, kvm, linuxppc-dev, linux-kernel
On Mon, Aug 28, 2017 at 02:38:37PM +1000, Paul Mackerras wrote:
> On Sun, Aug 27, 2017 at 10:02:20PM +0100, Al Viro wrote:
> > On Wed, Aug 23, 2017 at 04:06:24PM +1000, Paul Mackerras wrote:
> >
> > > It seems to me that it would be better to do the anon_inode_getfd()
> > > call before the kvm_get_kvm() call, and go to the fail label if it
> > > fails.
> >
> > And what happens if another thread does close() on the (guessed) fd?
>
> Chaos ensues, but mostly because we don't have proper mutual exclusion
> on the modifications to the list. I'll add a mutex_lock/unlock to
> kvm_spapr_tce_release() and move the anon_inode_getfd() call inside
> the mutex.
>
> It looks like the other possible uses of the fd (mmap, and passing it
> as a parameter to the KVM_DEV_VFIO_GROUP_SET_SPAPR_TCE ioctl on a KVM
> device fd) are safe.
Frankly, it's a lot saner to have "no failure points past anon_inode_getfd()"
policy...
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: Re: [PATCH] fix memory leak on kvm_vm_ioctl_create_spapr_tce
@ 2017-08-28 5:28 ` Al Viro
0 siblings, 0 replies; 35+ messages in thread
From: Al Viro @ 2017-08-28 5:28 UTC (permalink / raw)
To: Paul Mackerras
Cc: Nixiaoming, David Hildenbrand, agraf, pbonzini, rkrcmar, benh,
mpe, kvm-ppc, kvm, linuxppc-dev, linux-kernel
On Mon, Aug 28, 2017 at 02:38:37PM +1000, Paul Mackerras wrote:
> On Sun, Aug 27, 2017 at 10:02:20PM +0100, Al Viro wrote:
> > On Wed, Aug 23, 2017 at 04:06:24PM +1000, Paul Mackerras wrote:
> >
> > > It seems to me that it would be better to do the anon_inode_getfd()
> > > call before the kvm_get_kvm() call, and go to the fail label if it
> > > fails.
> >
> > And what happens if another thread does close() on the (guessed) fd?
>
> Chaos ensues, but mostly because we don't have proper mutual exclusion
> on the modifications to the list. I'll add a mutex_lock/unlock to
> kvm_spapr_tce_release() and move the anon_inode_getfd() call inside
> the mutex.
>
> It looks like the other possible uses of the fd (mmap, and passing it
> as a parameter to the KVM_DEV_VFIO_GROUP_SET_SPAPR_TCE ioctl on a KVM
> device fd) are safe.
Frankly, it's a lot saner to have "no failure points past anon_inode_getfd()"
policy...
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: Re: [PATCH] fix memory leak on kvm_vm_ioctl_create_spapr_tce
2017-08-28 5:28 ` Al Viro
@ 2017-08-28 6:06 ` Paul Mackerras
-1 siblings, 0 replies; 35+ messages in thread
From: Paul Mackerras @ 2017-08-28 6:06 UTC (permalink / raw)
To: Al Viro
Cc: Nixiaoming, David Hildenbrand, agraf, pbonzini, rkrcmar, benh,
mpe, kvm-ppc, kvm, linuxppc-dev, linux-kernel
On Mon, Aug 28, 2017 at 06:28:08AM +0100, Al Viro wrote:
> On Mon, Aug 28, 2017 at 02:38:37PM +1000, Paul Mackerras wrote:
> > On Sun, Aug 27, 2017 at 10:02:20PM +0100, Al Viro wrote:
> > > On Wed, Aug 23, 2017 at 04:06:24PM +1000, Paul Mackerras wrote:
> > >
> > > > It seems to me that it would be better to do the anon_inode_getfd()
> > > > call before the kvm_get_kvm() call, and go to the fail label if it
> > > > fails.
> > >
> > > And what happens if another thread does close() on the (guessed) fd?
> >
> > Chaos ensues, but mostly because we don't have proper mutual exclusion
> > on the modifications to the list. I'll add a mutex_lock/unlock to
> > kvm_spapr_tce_release() and move the anon_inode_getfd() call inside
> > the mutex.
> >
> > It looks like the other possible uses of the fd (mmap, and passing it
> > as a parameter to the KVM_DEV_VFIO_GROUP_SET_SPAPR_TCE ioctl on a KVM
> > device fd) are safe.
>
> Frankly, it's a lot saner to have "no failure points past anon_inode_getfd()"
> policy...
Right. In my latest patch, there are no failure points past
anon_inode_getfd().
Paul.
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: Re: [PATCH] fix memory leak on kvm_vm_ioctl_create_spapr_tce
@ 2017-08-28 6:06 ` Paul Mackerras
0 siblings, 0 replies; 35+ messages in thread
From: Paul Mackerras @ 2017-08-28 6:06 UTC (permalink / raw)
To: Al Viro
Cc: Nixiaoming, David Hildenbrand, agraf, pbonzini, rkrcmar, benh,
mpe, kvm-ppc, kvm, linuxppc-dev, linux-kernel
On Mon, Aug 28, 2017 at 06:28:08AM +0100, Al Viro wrote:
> On Mon, Aug 28, 2017 at 02:38:37PM +1000, Paul Mackerras wrote:
> > On Sun, Aug 27, 2017 at 10:02:20PM +0100, Al Viro wrote:
> > > On Wed, Aug 23, 2017 at 04:06:24PM +1000, Paul Mackerras wrote:
> > >
> > > > It seems to me that it would be better to do the anon_inode_getfd()
> > > > call before the kvm_get_kvm() call, and go to the fail label if it
> > > > fails.
> > >
> > > And what happens if another thread does close() on the (guessed) fd?
> >
> > Chaos ensues, but mostly because we don't have proper mutual exclusion
> > on the modifications to the list. I'll add a mutex_lock/unlock to
> > kvm_spapr_tce_release() and move the anon_inode_getfd() call inside
> > the mutex.
> >
> > It looks like the other possible uses of the fd (mmap, and passing it
> > as a parameter to the KVM_DEV_VFIO_GROUP_SET_SPAPR_TCE ioctl on a KVM
> > device fd) are safe.
>
> Frankly, it's a lot saner to have "no failure points past anon_inode_getfd()"
> policy...
Right. In my latest patch, there are no failure points past
anon_inode_getfd().
Paul.
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: Re: [PATCH] fix memory leak on kvm_vm_ioctl_create_spapr_tce
2017-08-28 5:28 ` Al Viro
@ 2017-08-28 11:31 ` Michael Ellerman
-1 siblings, 0 replies; 35+ messages in thread
From: Michael Ellerman @ 2017-08-28 11:31 UTC (permalink / raw)
To: Al Viro, Paul Mackerras
Cc: Nixiaoming, David Hildenbrand, agraf, pbonzini, rkrcmar, benh,
kvm-ppc, kvm, linuxppc-dev, linux-kernel
Al Viro <viro@ZenIV.linux.org.uk> writes:
> On Mon, Aug 28, 2017 at 02:38:37PM +1000, Paul Mackerras wrote:
>> On Sun, Aug 27, 2017 at 10:02:20PM +0100, Al Viro wrote:
>> > On Wed, Aug 23, 2017 at 04:06:24PM +1000, Paul Mackerras wrote:
>> >
>> > > It seems to me that it would be better to do the anon_inode_getfd()
>> > > call before the kvm_get_kvm() call, and go to the fail label if it
>> > > fails.
>> >
>> > And what happens if another thread does close() on the (guessed) fd?
>>
>> Chaos ensues, but mostly because we don't have proper mutual exclusion
>> on the modifications to the list. I'll add a mutex_lock/unlock to
>> kvm_spapr_tce_release() and move the anon_inode_getfd() call inside
>> the mutex.
>>
>> It looks like the other possible uses of the fd (mmap, and passing it
>> as a parameter to the KVM_DEV_VFIO_GROUP_SET_SPAPR_TCE ioctl on a KVM
>> device fd) are safe.
>
> Frankly, it's a lot saner to have "no failure points past anon_inode_getfd()"
> policy...
Actually I thought that was a hard rule. But I don't see it documented
or mentioned anywhere so I'm not sure now why I thought that.
cheers
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: Re: [PATCH] fix memory leak on kvm_vm_ioctl_create_spapr_tce
@ 2017-08-28 11:31 ` Michael Ellerman
0 siblings, 0 replies; 35+ messages in thread
From: Michael Ellerman @ 2017-08-28 11:31 UTC (permalink / raw)
To: Al Viro, Paul Mackerras
Cc: Nixiaoming, David Hildenbrand, agraf, pbonzini, rkrcmar, benh,
kvm-ppc, kvm, linuxppc-dev, linux-kernel
Al Viro <viro@ZenIV.linux.org.uk> writes:
> On Mon, Aug 28, 2017 at 02:38:37PM +1000, Paul Mackerras wrote:
>> On Sun, Aug 27, 2017 at 10:02:20PM +0100, Al Viro wrote:
>> > On Wed, Aug 23, 2017 at 04:06:24PM +1000, Paul Mackerras wrote:
>> >
>> > > It seems to me that it would be better to do the anon_inode_getfd()
>> > > call before the kvm_get_kvm() call, and go to the fail label if it
>> > > fails.
>> >
>> > And what happens if another thread does close() on the (guessed) fd?
>>
>> Chaos ensues, but mostly because we don't have proper mutual exclusion
>> on the modifications to the list. I'll add a mutex_lock/unlock to
>> kvm_spapr_tce_release() and move the anon_inode_getfd() call inside
>> the mutex.
>>
>> It looks like the other possible uses of the fd (mmap, and passing it
>> as a parameter to the KVM_DEV_VFIO_GROUP_SET_SPAPR_TCE ioctl on a KVM
>> device fd) are safe.
>
> Frankly, it's a lot saner to have "no failure points past anon_inode_getfd()"
> policy...
Actually I thought that was a hard rule. But I don't see it documented
or mentioned anywhere so I'm not sure now why I thought that.
cheers
^ permalink raw reply [flat|nested] 35+ messages in thread
end of thread, other threads:[~2017-08-28 11:31 UTC | newest]
Thread overview: 35+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-08-22 14:28 [PATCH] fix memory leak on kvm_vm_ioctl_create_spapr_tce nixiaoming
2017-08-22 14:28 ` nixiaoming
2017-08-22 15:15 ` David Hildenbrand
2017-08-22 15:15 ` David Hildenbrand
2017-08-22 15:23 ` David Hildenbrand
2017-08-22 15:23 ` David Hildenbrand
2017-08-23 1:43 ` Nixiaoming
2017-08-23 1:43 ` Nixiaoming
2017-08-23 1:43 ` Nixiaoming
2017-08-23 6:06 ` Paul Mackerras
2017-08-23 6:06 ` Paul Mackerras
2017-08-23 8:25 ` David Hildenbrand
2017-08-23 8:25 ` David Hildenbrand
2017-08-23 9:16 ` David Hildenbrand
2017-08-23 9:16 ` David Hildenbrand
2017-08-23 10:17 ` 答复: " Nixiaoming
2017-08-23 10:17 ` 答复: [PATCH] fix memory leak on kvm_vm_ioc =?utf-8?B?dGxfY3JlYXRlX3NwY Nixiaoming
2017-08-23 10:17 ` 答复: [PATCH] fix memory leak on kvm_vm_ioctl_create_spapr_tce Nixiaoming
2017-08-24 1:06 ` Nixiaoming
2017-08-24 1:06 ` Nixiaoming
2017-08-24 1:06 ` Nixiaoming
2017-08-27 21:02 ` Al Viro
2017-08-27 21:02 ` Al Viro
2017-08-28 4:38 ` Paul Mackerras
2017-08-28 4:38 ` Paul Mackerras
2017-08-28 5:28 ` Al Viro
2017-08-28 5:28 ` Al Viro
2017-08-28 6:06 ` Paul Mackerras
2017-08-28 6:06 ` Paul Mackerras
2017-08-28 11:31 ` Michael Ellerman
2017-08-28 11:31 ` Michael Ellerman
-- strict thread matches above, loose matches on Subject: below --
2017-08-22 14:18 [PATCH] fix memory leak on kvm_vm_ioctl_get_htab_fd nixiaoming
2017-08-22 14:18 ` nixiaoming
2017-08-22 15:51 ` Paolo Bonzini
2017-08-22 15:51 ` Paolo Bonzini
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.