All of lore.kernel.org
 help / color / mirror / Atom feed
From: Sean Christopherson <seanjc@google.com>
To: Wei Wang <wei.w.wang@intel.com>
Cc: pbonzini@redhat.com, mhal@rbox.co, kvm@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2 1/2] KVM: destruct kvm_io_device while unregistering it from kvm_io_bus
Date: Thu, 23 Mar 2023 08:43:34 -0700	[thread overview]
Message-ID: <ZBxzphnyLPwBimKL@google.com> (raw)
In-Reply-To: <20230207123713.3905-2-wei.w.wang@intel.com>

On Tue, Feb 07, 2023, Wei Wang wrote:
> Current usage of kvm_io_device requires users to destruct it with an extra
> call of kvm_iodevice_destructor after the device gets unregistered from
> kvm_io_bus. This is not necessary and can cause errors if a user forgot
> to make the extra call.
> 
> Simplify the usage by combining kvm_iodevice_destructor into
> kvm_io_bus_unregister_dev. This reduces LOCs a bit for users and can
> avoid the leakage of destructing the device explicitly.

The changelog should really call out that coalesced_mmio_ops and ioeventfd_ops
are the only kvm_io_device_ops instances that implement ->destructor.  Without
that info, this change looks super dangerous as it's not obvious other paths won't
end up with a use-after-free.

Paolo, if/when you take this, can you tack on something like:

Note, coalesced_mmio_ops and ioeventfd_ops are the only instances of
kvm_io_device_ops that implement a destructor, all other callers of
kvm_io_bus_unregister_dev() are unaffected by this change.

> Signed-off-by: Wei Wang <wei.w.wang@intel.com>
> ---

Reviewed-by: Sean Christopherson <seanjc@google.com>

  reply	other threads:[~2023-03-23 15:43 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-02-07 12:37 [PATCH v2 0/2] kvm_io_bus_unregister_dev cleanup Wei Wang
2023-02-07 12:37 ` [PATCH v2 1/2] KVM: destruct kvm_io_device while unregistering it from kvm_io_bus Wei Wang
2023-03-23 15:43   ` Sean Christopherson [this message]
2023-02-07 12:37 ` [PATCH v2 2/2] kvm/eventfd: use list_for_each_entry when deassign ioeventfd Wei Wang
2023-03-23 15:30   ` Sean Christopherson
2023-06-13 23:21 ` [PATCH v2 0/2] kvm_io_bus_unregister_dev cleanup Sean Christopherson

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=ZBxzphnyLPwBimKL@google.com \
    --to=seanjc@google.com \
    --cc=kvm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mhal@rbox.co \
    --cc=pbonzini@redhat.com \
    --cc=wei.w.wang@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.