All of lore.kernel.org
 help / color / mirror / Atom feed
From: Leonardo Bras <leonardo@linux.ibm.com>
To: Paolo Bonzini <pbonzini@redhat.com>,
	Sean Christopherson <sean.j.christopherson@intel.com>
Cc: "Paul Mackerras" <paulus@ozlabs.org>,
	"Radim Krčmář" <rkrcmar@redhat.com>,
	kvm-ppc@vger.kernel.org, kvm@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH] KVM: Add separate helper for putting borrowed reference to kvm
Date: Wed, 27 Nov 2019 18:24:35 +0000	[thread overview]
Message-ID: <bfa563e6a584bd85d3abe953ca088281dc0e167b.camel@linux.ibm.com> (raw)
In-Reply-To: <e1a4218f-2a70-3de3-1403-dbebf8a8abdf@redhat.com>

[-- Attachment #1: Type: text/plain, Size: 1239 bytes --]

On Wed, 2019-11-27 at 17:38 +0100, Paolo Bonzini wrote:
> On 26/11/19 18:53, Leonardo Bras wrote:
> > I agree an use-after-free more problem than a memory leak, but I
> > think
> > that there is a way to solve this without leaking the memory also.
> > 
> > One option would be reordering the kvm_put_kvm(), like in this
> > patch:
> > https://lkml.org/lkml/2019/11/26/517
> 
> It's a tradeoff between "fix one bug" and "mitigate all bugs of that
> class", both are good things to do.  Reordering the kvm_put_kvm()
> fixes
> the bug.  kvm_put_kvm_no_destroy() makes all bugs of that kind less
> severe, but it doesn't try to fix them.
> 
> Paolo
> 

I think I understand it better now, thanks Paolo and Sean.

By what I could undestand up to now, these functions that use borrowed
references can only be called while the reference (file descriptor)
exists. 
So, suppose these threads, where:
- T1 uses a borrowed reference, and 
- T2 is releasing the reference (close, release):

T1				| T2
kvm_get_kvm()			|
...				| kvm_put_kvm()
kvm_put_kvm_no_destroy()	|

The above would not trigger a use-after-free bug, but will cause a
memory leak.
Is my above understanding right?

Best regards,

Leonardo


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

WARNING: multiple messages have this Message-ID (diff)
From: Leonardo Bras <leonardo@linux.ibm.com>
To: Paolo Bonzini <pbonzini@redhat.com>,
	Sean Christopherson <sean.j.christopherson@intel.com>
Cc: "Paul Mackerras" <paulus@ozlabs.org>,
	"Radim Krčmář" <rkrcmar@redhat.com>,
	kvm-ppc@vger.kernel.org, kvm@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH] KVM: Add separate helper for putting borrowed reference to kvm
Date: Wed, 27 Nov 2019 15:24:35 -0300	[thread overview]
Message-ID: <bfa563e6a584bd85d3abe953ca088281dc0e167b.camel@linux.ibm.com> (raw)
In-Reply-To: <e1a4218f-2a70-3de3-1403-dbebf8a8abdf@redhat.com>

[-- Attachment #1: Type: text/plain, Size: 1239 bytes --]

On Wed, 2019-11-27 at 17:38 +0100, Paolo Bonzini wrote:
> On 26/11/19 18:53, Leonardo Bras wrote:
> > I agree an use-after-free more problem than a memory leak, but I
> > think
> > that there is a way to solve this without leaking the memory also.
> > 
> > One option would be reordering the kvm_put_kvm(), like in this
> > patch:
> > https://lkml.org/lkml/2019/11/26/517
> 
> It's a tradeoff between "fix one bug" and "mitigate all bugs of that
> class", both are good things to do.  Reordering the kvm_put_kvm()
> fixes
> the bug.  kvm_put_kvm_no_destroy() makes all bugs of that kind less
> severe, but it doesn't try to fix them.
> 
> Paolo
> 

I think I understand it better now, thanks Paolo and Sean.

By what I could undestand up to now, these functions that use borrowed
references can only be called while the reference (file descriptor)
exists. 
So, suppose these threads, where:
- T1 uses a borrowed reference, and 
- T2 is releasing the reference (close, release):

T1				| T2
kvm_get_kvm()			|
...				| kvm_put_kvm()
kvm_put_kvm_no_destroy()	|

The above would not trigger a use-after-free bug, but will cause a
memory leak.
Is my above understanding right?

Best regards,

Leonardo


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

  reply	other threads:[~2019-11-27 18:24 UTC|newest]

Thread overview: 34+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-10-21 22:58 [PATCH] KVM: Add separate helper for putting borrowed reference to kvm Sean Christopherson
2019-10-21 22:58 ` Sean Christopherson
2019-10-22 13:49 ` Paolo Bonzini
2019-10-22 13:49   ` Paolo Bonzini
2019-11-26 16:44 ` Leonardo Bras
2019-11-26 16:44   ` Leonardo Bras
2019-11-26 17:14   ` Sean Christopherson
2019-11-26 17:14     ` Sean Christopherson
2019-11-26 17:53     ` Leonardo Bras
2019-11-26 17:53       ` Leonardo Bras
2019-11-27 16:38       ` Paolo Bonzini
2019-11-27 16:38         ` Paolo Bonzini
2019-11-27 18:24         ` Leonardo Bras [this message]
2019-11-27 18:24           ` Leonardo Bras
2019-11-27 18:32           ` Paolo Bonzini
2019-11-27 18:32             ` Paolo Bonzini
2019-11-27 19:25             ` Leonardo Bras
2019-11-27 19:25               ` Leonardo Bras
2019-11-27 19:47               ` Sean Christopherson
2019-11-27 19:47                 ` Sean Christopherson
2019-11-27 20:15                 ` Leonardo Bras
2019-11-27 20:15                   ` Leonardo Bras
2019-11-27 21:57                   ` Leonardo Bras
2019-11-27 21:57                     ` Leonardo Bras
2019-11-28  1:00                     ` Sean Christopherson
2019-11-28  1:00                       ` Sean Christopherson
2019-11-28 16:29                       ` Leonardo Bras
2019-11-28 16:29                         ` Leonardo Bras
2019-11-28 13:49                     ` Paolo Bonzini
2019-11-28 13:49                       ` Paolo Bonzini
2019-11-28 16:04                       ` Leonardo Bras
2019-11-28 16:04                         ` Leonardo Bras
2019-11-26 17:57     ` Leonardo Bras
2019-11-26 17:57       ` Leonardo Bras

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=bfa563e6a584bd85d3abe953ca088281dc0e167b.camel@linux.ibm.com \
    --to=leonardo@linux.ibm.com \
    --cc=kvm-ppc@vger.kernel.org \
    --cc=kvm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=paulus@ozlabs.org \
    --cc=pbonzini@redhat.com \
    --cc=rkrcmar@redhat.com \
    --cc=sean.j.christopherson@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.