Re: [PATCH V3 2/3] kvm: don't register wildcard MMIO EVENTFD on two buses
On Tue, Sep 01, 2015 at 04:22:22PM +0800, Jason Wang wrote: > > > On 09/01/2015 02:54 PM, Michael S. Tsirkin wrote: > > On Tue, Sep 01, 2015 at 12:47:36PM +0800, Jason Wang wrote: > >> > >> On 09/01/2015 12:31 PM, Michael S. Tsirkin wrote: > >>> On Tue, Sep 01, 2015 at 11:33:43AM +0800, Jason Wang wrote: > On 08/31/2015 07:33 PM, Michael S. Tsirkin wrote: > > On Mon, Aug 31, 2015 at 04:03:59PM +0800, Jason Wang wrote: > >>> On 08/31/2015 03:29 PM, Michael S. Tsirkin wrote: > > Thinking more about this, invoking the 0-length write after > > the != 0 length one would be better: it would mean > > we only > > handle the userspace MMIO like this. > > Right. > > > > Using current unittest. This patch is about 2.9% slower than > > before, and > > invoking 0-length write after is still 1.1% slower > > (mmio-datamatch-eventfd). > > > > /patch/result/-+%/ > > /base/2957/0/ > > /V3/3043/+2.9%/ > > /V3+invoking != 0 length first/2990/+1.1%/ > > > > So looks like the best method is not searching > > KVM_FAST_MMIO_BUS during > > KVM_MMIO_BUS. Instead, let userspace to register both datamatch > > and > > wildcard in this case. Does this sound good to you? > > No - we can't change userspace. > >>> Actually, the change was as simple as following. So I don't get the > >>> reason why. > > Because it's too late - we committed to a specific userspace ABI > > when this was merged in kernel, we must maintain it. > Ok ( Though I don't think it has real users for this now because it was > actually broken). > >>> It actually worked most of the time - you only trigger a use after free > >>> on deregister. > >>> > >> It doesn't work for amd and intel machine without ept. > > I thought it does :( > > > > Even if I thought yours is a good API (and I don't BTW - it's exposing > > internal implementation details) it's too late to change it. > I believe we should document the special treatment in kernel of zero > length mmio eventfd in api.txt? If yes, is this an exposing? If not, how > can userspace know the advantages of this and use it? For better API, > probably we need another new flag just for fast mmio and obsolete > current one by failing the assigning for zero length mmio eventfd. > >>> I sent a patch to update api.txt already as part of > >>> kvm: add KVM_CAP_IOEVENTFD_PF capability. > >>> I should probably split it out. > >>> > >>> Sorry, I don't think the api change you propose makes sense - just fix the > >>> crash in the existing one. > >>> > >> Ok, so I believe the fix should go: > >> > >> - having two ioeventfds when we want to assign zero length mmio eventfd > > You mean the in-kernel data structures? > > Yes. > > > > >> - change the kvm_io_bus_sort_cmp() and can handle zero length correctly > > This one's for amd/non ept, right? I'd rather we implemented the > > fast mmio optimization for these. > > Agree, but we'd better fix it and backport it to stable first? I would say fix it upstream first. Worry about stable later. And I don't see a lot of value in adding a temporary hack - it's not too much work to just do the optimal thing directly. But I won't nack a temporary solution if you insist. -- MST -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH V3 2/3] kvm: don't register wildcard MMIO EVENTFD on two buses
On 09/01/2015 02:54 PM, Michael S. Tsirkin wrote: > On Tue, Sep 01, 2015 at 12:47:36PM +0800, Jason Wang wrote: >> >> On 09/01/2015 12:31 PM, Michael S. Tsirkin wrote: >>> On Tue, Sep 01, 2015 at 11:33:43AM +0800, Jason Wang wrote: On 08/31/2015 07:33 PM, Michael S. Tsirkin wrote: > On Mon, Aug 31, 2015 at 04:03:59PM +0800, Jason Wang wrote: >>> On 08/31/2015 03:29 PM, Michael S. Tsirkin wrote: > Thinking more about this, invoking the 0-length write after > the != 0 length one would be better: it would mean we > only > handle the userspace MMIO like this. > Right. > > Using current unittest. This patch is about 2.9% slower than > before, and > invoking 0-length write after is still 1.1% slower > (mmio-datamatch-eventfd). > > /patch/result/-+%/ > /base/2957/0/ > /V3/3043/+2.9%/ > /V3+invoking != 0 length first/2990/+1.1%/ > > So looks like the best method is not searching KVM_FAST_MMIO_BUS > during > KVM_MMIO_BUS. Instead, let userspace to register both datamatch > and > wildcard in this case. Does this sound good to you? > No - we can't change userspace. >>> Actually, the change was as simple as following. So I don't get the >>> reason why. > Because it's too late - we committed to a specific userspace ABI > when this was merged in kernel, we must maintain it. Ok ( Though I don't think it has real users for this now because it was actually broken). >>> It actually worked most of the time - you only trigger a use after free >>> on deregister. >>> >> It doesn't work for amd and intel machine without ept. > I thought it does :( > > Even if I thought yours is a good API (and I don't BTW - it's exposing > internal implementation details) it's too late to change it. I believe we should document the special treatment in kernel of zero length mmio eventfd in api.txt? If yes, is this an exposing? If not, how can userspace know the advantages of this and use it? For better API, probably we need another new flag just for fast mmio and obsolete current one by failing the assigning for zero length mmio eventfd. >>> I sent a patch to update api.txt already as part of >>> kvm: add KVM_CAP_IOEVENTFD_PF capability. >>> I should probably split it out. >>> >>> Sorry, I don't think the api change you propose makes sense - just fix the >>> crash in the existing one. >>> >> Ok, so I believe the fix should go: >> >> - having two ioeventfds when we want to assign zero length mmio eventfd > You mean the in-kernel data structures? Yes. > >> - change the kvm_io_bus_sort_cmp() and can handle zero length correctly > This one's for amd/non ept, right? I'd rather we implemented the > fast mmio optimization for these. Agree, but we'd better fix it and backport it to stable first? -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH V3 2/3] kvm: don't register wildcard MMIO EVENTFD on two buses
On Tue, Sep 01, 2015 at 12:47:36PM +0800, Jason Wang wrote: > > > On 09/01/2015 12:31 PM, Michael S. Tsirkin wrote: > > On Tue, Sep 01, 2015 at 11:33:43AM +0800, Jason Wang wrote: > >> > >> On 08/31/2015 07:33 PM, Michael S. Tsirkin wrote: > >>> On Mon, Aug 31, 2015 at 04:03:59PM +0800, Jason Wang wrote: > > > > On 08/31/2015 03:29 PM, Michael S. Tsirkin wrote: > >>> Thinking more about this, invoking the 0-length write after > >>> the != 0 length one would be better: it would mean we > >>> only > >>> handle the userspace MMIO like this. > >>> Right. > >>> > >>> Using current unittest. This patch is about 2.9% slower than > >>> before, and > >>> invoking 0-length write after is still 1.1% slower > >>> (mmio-datamatch-eventfd). > >>> > >>> /patch/result/-+%/ > >>> /base/2957/0/ > >>> /V3/3043/+2.9%/ > >>> /V3+invoking != 0 length first/2990/+1.1%/ > >>> > >>> So looks like the best method is not searching KVM_FAST_MMIO_BUS > >>> during > >>> KVM_MMIO_BUS. Instead, let userspace to register both datamatch > >>> and > >>> wildcard in this case. Does this sound good to you? > >>> No - we can't change userspace. > > Actually, the change was as simple as following. So I don't get the > > reason why. > >>> Because it's too late - we committed to a specific userspace ABI > >>> when this was merged in kernel, we must maintain it. > >> Ok ( Though I don't think it has real users for this now because it was > >> actually broken). > > It actually worked most of the time - you only trigger a use after free > > on deregister. > > > > It doesn't work for amd and intel machine without ept. I thought it does :( > >>> Even if I thought yours is a good API (and I don't BTW - it's exposing > >>> internal implementation details) it's too late to change it. > >> I believe we should document the special treatment in kernel of zero > >> length mmio eventfd in api.txt? If yes, is this an exposing? If not, how > >> can userspace know the advantages of this and use it? For better API, > >> probably we need another new flag just for fast mmio and obsolete > >> current one by failing the assigning for zero length mmio eventfd. > > I sent a patch to update api.txt already as part of > > kvm: add KVM_CAP_IOEVENTFD_PF capability. > > I should probably split it out. > > > > Sorry, I don't think the api change you propose makes sense - just fix the > > crash in the existing one. > > > > Ok, so I believe the fix should go: > > - having two ioeventfds when we want to assign zero length mmio eventfd You mean the in-kernel data structures? > - change the kvm_io_bus_sort_cmp() and can handle zero length correctly This one's for amd/non ept, right? I'd rather we implemented the fast mmio optimization for these. > What's your thought? -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH V3 2/3] kvm: don't register wildcard MMIO EVENTFD on two buses
On 09/01/2015 12:31 PM, Michael S. Tsirkin wrote: > On Tue, Sep 01, 2015 at 11:33:43AM +0800, Jason Wang wrote: >> >> On 08/31/2015 07:33 PM, Michael S. Tsirkin wrote: >>> On Mon, Aug 31, 2015 at 04:03:59PM +0800, Jason Wang wrote: > > On 08/31/2015 03:29 PM, Michael S. Tsirkin wrote: >>> Thinking more about this, invoking the 0-length write after >>> the != 0 length one would be better: it would mean we >>> only >>> handle the userspace MMIO like this. >>> Right. >>> >>> Using current unittest. This patch is about 2.9% slower than >>> before, and >>> invoking 0-length write after is still 1.1% slower >>> (mmio-datamatch-eventfd). >>> >>> /patch/result/-+%/ >>> /base/2957/0/ >>> /V3/3043/+2.9%/ >>> /V3+invoking != 0 length first/2990/+1.1%/ >>> >>> So looks like the best method is not searching KVM_FAST_MMIO_BUS >>> during >>> KVM_MMIO_BUS. Instead, let userspace to register both datamatch and >>> wildcard in this case. Does this sound good to you? >>> No - we can't change userspace. > Actually, the change was as simple as following. So I don't get the > reason why. >>> Because it's too late - we committed to a specific userspace ABI >>> when this was merged in kernel, we must maintain it. >> Ok ( Though I don't think it has real users for this now because it was >> actually broken). > It actually worked most of the time - you only trigger a use after free > on deregister. > It doesn't work for amd and intel machine without ept. >>> Even if I thought yours is a good API (and I don't BTW - it's exposing >>> internal implementation details) it's too late to change it. >> I believe we should document the special treatment in kernel of zero >> length mmio eventfd in api.txt? If yes, is this an exposing? If not, how >> can userspace know the advantages of this and use it? For better API, >> probably we need another new flag just for fast mmio and obsolete >> current one by failing the assigning for zero length mmio eventfd. > I sent a patch to update api.txt already as part of > kvm: add KVM_CAP_IOEVENTFD_PF capability. > I should probably split it out. > > Sorry, I don't think the api change you propose makes sense - just fix the > crash in the existing one. > Ok, so I believe the fix should go: - having two ioeventfds when we want to assign zero length mmio eventfd - change the kvm_io_bus_sort_cmp() and can handle zero length correctly What's your thought? -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH V3 2/3] kvm: don't register wildcard MMIO EVENTFD on two buses
On Tue, Sep 01, 2015 at 11:33:43AM +0800, Jason Wang wrote: > > > On 08/31/2015 07:33 PM, Michael S. Tsirkin wrote: > > On Mon, Aug 31, 2015 at 04:03:59PM +0800, Jason Wang wrote: > >> > > >> > > >> > On 08/31/2015 03:29 PM, Michael S. Tsirkin wrote: > >>> > > Thinking more about this, invoking the 0-length write after > >>> > > > >> > the != 0 length one would be better: it would mean > >>> > > > >> > we only > >>> > > > >> > handle the userspace MMIO like this. > >>> > >>> > > Right. > >>> > >>> > > > > > >> > > > > >> > Using current unittest. This patch is about 2.9% slower than > > > >> > before, and > > > >> > invoking 0-length write after is still 1.1% slower > > > >> > (mmio-datamatch-eventfd). > > > >> > > > > >> > /patch/result/-+%/ > > > >> > /base/2957/0/ > > > >> > /V3/3043/+2.9%/ > > > >> > /V3+invoking != 0 length first/2990/+1.1%/ > > > >> > > > > >> > So looks like the best method is not searching KVM_FAST_MMIO_BUS > > > >> > during > > > >> > KVM_MMIO_BUS. Instead, let userspace to register both datamatch > > > >> > and > > > >> > wildcard in this case. Does this sound good to you? > >>> > > No - we can't change userspace. > >> > > >> > Actually, the change was as simple as following. So I don't get the > >> > reason why. > > Because it's too late - we committed to a specific userspace ABI > > when this was merged in kernel, we must maintain it. > > Ok ( Though I don't think it has real users for this now because it was > actually broken). It actually worked most of the time - you only trigger a use after free on deregister. > > Even if I thought yours is a good API (and I don't BTW - it's exposing > > internal implementation details) it's too late to change it. > > I believe we should document the special treatment in kernel of zero > length mmio eventfd in api.txt? If yes, is this an exposing? If not, how > can userspace know the advantages of this and use it? For better API, > probably we need another new flag just for fast mmio and obsolete > current one by failing the assigning for zero length mmio eventfd. I sent a patch to update api.txt already as part of kvm: add KVM_CAP_IOEVENTFD_PF capability. I should probably split it out. Sorry, I don't think the api change you propose makes sense - just fix the crash in the existing one. -- MST -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH V3 2/3] kvm: don't register wildcard MMIO EVENTFD on two buses
On 08/31/2015 07:33 PM, Michael S. Tsirkin wrote: > On Mon, Aug 31, 2015 at 04:03:59PM +0800, Jason Wang wrote: >> > >> > >> > On 08/31/2015 03:29 PM, Michael S. Tsirkin wrote: >>> > > Thinking more about this, invoking the 0-length write after >>> > > > >> > the != 0 length one would be better: it would mean >>> > > > >> > we only >>> > > > >> > handle the userspace MMIO like this. >>> > >>> > > Right. >>> > >>> > > > > >> > > > >> > Using current unittest. This patch is about 2.9% slower than > > >> > before, and > > >> > invoking 0-length write after is still 1.1% slower > > >> > (mmio-datamatch-eventfd). > > >> > > > >> > /patch/result/-+%/ > > >> > /base/2957/0/ > > >> > /V3/3043/+2.9%/ > > >> > /V3+invoking != 0 length first/2990/+1.1%/ > > >> > > > >> > So looks like the best method is not searching KVM_FAST_MMIO_BUS > > >> > during > > >> > KVM_MMIO_BUS. Instead, let userspace to register both datamatch and > > >> > wildcard in this case. Does this sound good to you? >>> > > No - we can't change userspace. >> > >> > Actually, the change was as simple as following. So I don't get the >> > reason why. > Because it's too late - we committed to a specific userspace ABI > when this was merged in kernel, we must maintain it. Ok ( Though I don't think it has real users for this now because it was actually broken). > Even if I thought yours is a good API (and I don't BTW - it's exposing > internal implementation details) it's too late to change it. I believe we should document the special treatment in kernel of zero length mmio eventfd in api.txt? If yes, is this an exposing? If not, how can userspace know the advantages of this and use it? For better API, probably we need another new flag just for fast mmio and obsolete current one by failing the assigning for zero length mmio eventfd. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH V3 2/3] kvm: don't register wildcard MMIO EVENTFD on two buses
On Mon, Aug 31, 2015 at 04:03:59PM +0800, Jason Wang wrote:
>
>
> On 08/31/2015 03:29 PM, Michael S. Tsirkin wrote:
> > Thinking more about this, invoking the 0-length write after
> > > >> > the != 0 length one would be better: it would mean we only
> > > >> > handle the userspace MMIO like this.
> >>> > > Right.
> >>> > >
> >> >
> >> > Using current unittest. This patch is about 2.9% slower than before, and
> >> > invoking 0-length write after is still 1.1% slower
> >> > (mmio-datamatch-eventfd).
> >> >
> >> > /patch/result/-+%/
> >> > /base/2957/0/
> >> > /V3/3043/+2.9%/
> >> > /V3+invoking != 0 length first/2990/+1.1%/
> >> >
> >> > So looks like the best method is not searching KVM_FAST_MMIO_BUS during
> >> > KVM_MMIO_BUS. Instead, let userspace to register both datamatch and
> >> > wildcard in this case. Does this sound good to you?
> > No - we can't change userspace.
>
> Actually, the change was as simple as following. So I don't get the
> reason why.
Because it's too late - we committed to a specific userspace ABI
when this was merged in kernel, we must maintain it.
Even if I thought yours is a good API (and I don't BTW - it's exposing
internal implementation details) it's too late to change it.
> diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
> index 9935029..42ee986 100644
> --- a/hw/virtio/virtio-pci.c
> +++ b/hw/virtio/virtio-pci.c
> @@ -288,6 +288,8 @@ static int
> virtio_pci_set_host_notifier_internal(VirtIOPCIProxy *proxy,
> if (modern) {
> memory_region_add_eventfd(modern_mr, modern_addr, 2,
>true, n, notifier);
> +memory_region_add_eventfd(modern_mr, modern_addr, 0,
> + false, n, notifier);
> }
> if (legacy) {
> memory_region_add_eventfd(legacy_mr, legacy_addr, 2,
> @@ -297,6 +299,8 @@ static int
> virtio_pci_set_host_notifier_internal(VirtIOPCIProxy *proxy,
> if (modern) {
> memory_region_del_eventfd(modern_mr, modern_addr, 2,
>true, n, notifier);
> +memory_region_del_eventfd(modern_mr, modern_addr, 0,
> + false, n, notifier);
> }
> if (legacy) {
> memory_region_del_eventfd(legacy_mr, legacy_addr, 2,
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH V3 2/3] kvm: don't register wildcard MMIO EVENTFD on two buses
On 08/31/2015 03:29 PM, Michael S. Tsirkin wrote:
> Thinking more about this, invoking the 0-length write after
> > >> > the != 0 length one would be better: it would mean we only
> > >> > handle the userspace MMIO like this.
>>> > > Right.
>>> > >
>> >
>> > Using current unittest. This patch is about 2.9% slower than before, and
>> > invoking 0-length write after is still 1.1% slower
>> > (mmio-datamatch-eventfd).
>> >
>> > /patch/result/-+%/
>> > /base/2957/0/
>> > /V3/3043/+2.9%/
>> > /V3+invoking != 0 length first/2990/+1.1%/
>> >
>> > So looks like the best method is not searching KVM_FAST_MMIO_BUS during
>> > KVM_MMIO_BUS. Instead, let userspace to register both datamatch and
>> > wildcard in this case. Does this sound good to you?
> No - we can't change userspace.
Actually, the change was as simple as following. So I don't get the
reason why.
diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
index 9935029..42ee986 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -288,6 +288,8 @@ static int
virtio_pci_set_host_notifier_internal(VirtIOPCIProxy *proxy,
if (modern) {
memory_region_add_eventfd(modern_mr, modern_addr, 2,
true, n, notifier);
+memory_region_add_eventfd(modern_mr, modern_addr, 0,
+ false, n, notifier);
}
if (legacy) {
memory_region_add_eventfd(legacy_mr, legacy_addr, 2,
@@ -297,6 +299,8 @@ static int
virtio_pci_set_host_notifier_internal(VirtIOPCIProxy *proxy,
if (modern) {
memory_region_del_eventfd(modern_mr, modern_addr, 2,
true, n, notifier);
+memory_region_del_eventfd(modern_mr, modern_addr, 0,
+ false, n, notifier);
}
if (legacy) {
memory_region_del_eventfd(legacy_mr, legacy_addr, 2,
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH V3 2/3] kvm: don't register wildcard MMIO EVENTFD on two buses
On Mon, Aug 31, 2015 at 11:12:07AM +0800, Jason Wang wrote:
>
>
> On 08/26/2015 01:10 PM, Jason Wang wrote:
> > On 08/25/2015 07:51 PM, Michael S. Tsirkin wrote:
> >> > On Tue, Aug 25, 2015 at 05:05:47PM +0800, Jason Wang wrote:
> >> > We register wildcard mmio eventfd on two buses, one for KVM_MMIO_BUS
> >> > and another is KVM_FAST_MMIO_BUS. This leads to issue:
> >> >
> >> > - kvm_io_bus_destroy() knows nothing about the devices on two buses
> >> > points to a single dev. Which will lead double free [1] during
> >> > exit.
> >> > - wildcard eventfd ignores data len, so it was registered as a
> >> > kvm_io_range with zero length. This will fail the binary search in
> >> > kvm_io_bus_get_first_dev() when we try to emulate through
> >> > KVM_MMIO_BUS. This will cause userspace io emulation request
> >> > instead
> >> > of a eventfd notification (virtqueue kick will be trapped by qemu
> >> > instead of vhost in this case).
> >> >
> >> > Fixing this by don't register wildcard mmio eventfd on two
> >> > buses. Instead, only register it in KVM_FAST_MMIO_BUS. This fixes
> >> > the
> >> > double free issue of kvm_io_bus_destroy(). For the arch/setups that
> >> > does not utilize KVM_FAST_MMIO_BUS, before searching KVM_MMIO_BUS,
> >> > try
> >> > KVM_FAST_MMIO_BUS first to see it it has a match.
> >> >
> >> > [1] Panic caused by double free:
> >> >
> >> > CPU: 1 PID: 2894 Comm: qemu-system-x86 Not tainted
> >> > 3.19.0-26-generic #28-Ubuntu
> >> > Hardware name: LENOVO 2356BG6/2356BG6, BIOS G7ET96WW (2.56 )
> >> > 09/12/2013
> >> > task: 88009ae0c4b0 ti: 88020e7f task.ti:
> >> > 88020e7f
> >> > RIP: 0010:[] []
> >> > ioeventfd_release+0x28/0x60 [kvm]
> >> > RSP: 0018:88020e7f3bc8 EFLAGS: 00010292
> >> > RAX: dead00200200 RBX: 8801ec19c900 RCX: 00018200016d
> >> > RDX: 8801ec19cf80 RSI: ea0008bf1d40 RDI: 8801ec19c900
> >> > RBP: 88020e7f3bd8 R08: 2fc75a01 R09: 00018200016d
> >> > R10: c07df6ae R11: 88022fc75a98 R12: 88021e7cc000
> >> > R13: 88021e7cca48 R14: 88021e7cca50 R15: 8801ec19c880
> >> > FS: 7fc1ee3e6700() GS:88023e24()
> >> > knlGS:
> >> > CS: 0010 DS: ES: CR0: 80050033
> >> > CR2: 7f8f389d8000 CR3: 00023dc13000 CR4: 001427e0
> >> > Stack:
> >> > 88021e7cc000 88020e7f3be8 c07e2622
> >> > 88020e7f3c38 c07df69a 880232524160 88020e792d80
> >> > 880219b78c00 0008 8802321686a8
> >> > Call Trace:
> >> > [] ioeventfd_destructor+0x12/0x20 [kvm]
> >> > [] kvm_put_kvm+0xca/0x210 [kvm]
> >> > [] kvm_vcpu_release+0x18/0x20 [kvm]
> >> > [] __fput+0xe7/0x250
> >> > [] fput+0xe/0x10
> >> > [] task_work_run+0xd4/0xf0
> >> > [] do_exit+0x368/0xa50
> >> > [] ? recalc_sigpending+0x1f/0x60
> >> > [] do_group_exit+0x45/0xb0
> >> > [] get_signal+0x291/0x750
> >> > [] do_signal+0x28/0xab0
> >> > [] ? do_futex+0xdb/0x5d0
> >> > [] ? __wake_up_locked_key+0x18/0x20
> >> > [] ? SyS_futex+0x76/0x170
> >> > [] do_notify_resume+0x69/0xb0
> >> > [] int_signal+0x12/0x17
> >> > Code: 5d c3 90 0f 1f 44 00 00 55 48 89 e5 53 48 89 fb 48 83 ec 08
> >> > 48 8b 7f 20 e8 06 d6 a5 c0 48 8b 43 08 48 8b 13 48 89 df 48 89 42
> >> > 08 <48> 89 10 48 b8 00 01 10 00 00
> >> > RIP [] ioeventfd_release+0x28/0x60 [kvm]
> >> > RSP
> >> >
> >> > Cc: Gleb Natapov
> >> > Cc: Paolo Bonzini
> >> > Cc: Michael S. Tsirkin
> >> > Signed-off-by: Jason Wang
> >> > ---
> >> > Changes from V2:
> >> > - Tweak styles and comment suggested by Cornelia.
> >> > Changes from v1:
> >> > - change ioeventfd_bus_from_flags() to return KVM_FAST_MMIO_BUS when
> >> > needed to save lots of unnecessary changes.
> >> > ---
> >> > virt/kvm/eventfd.c | 31 +--
> >> > virt/kvm/kvm_main.c | 16 ++--
> >> > 2 files changed, 23 insertions(+), 24 deletions(-)
> >> >
> >> > diff --git a/virt/kvm/eventfd.c b/virt/kvm/eventfd.c
> >> > index 9ff4193..c3ffdc3 100644
> >> > --- a/virt/kvm/eventfd.c
> >> > +++ b/virt/kvm/eventfd.c
> >> > @@ -762,13 +762,16 @@ ioeventfd_check_collision(struct kvm *kvm,
> >> > struct _ioeventfd *p)
> >> > return false;
> >> > }
> >> >
> >> > -static enum kvm_bus ioeventfd_bus_from_flags(__u32 flags)
> >> > +static enum kvm_bus ioeventfd_bus_from_args(struct kvm_ioeventfd
> >> > *args)
> >> > {
> >> > - if (flags & KVM_IOEVENTFD_FLAG_PIO
Re: [PATCH V3 2/3] kvm: don't register wildcard MMIO EVENTFD on two buses
On 08/26/2015 01:10 PM, Jason Wang wrote:
> On 08/25/2015 07:51 PM, Michael S. Tsirkin wrote:
>> > On Tue, Aug 25, 2015 at 05:05:47PM +0800, Jason Wang wrote:
>> > We register wildcard mmio eventfd on two buses, one for KVM_MMIO_BUS
>> > and another is KVM_FAST_MMIO_BUS. This leads to issue:
>> >
>> > - kvm_io_bus_destroy() knows nothing about the devices on two buses
>> > points to a single dev. Which will lead double free [1] during exit.
>> > - wildcard eventfd ignores data len, so it was registered as a
>> > kvm_io_range with zero length. This will fail the binary search in
>> > kvm_io_bus_get_first_dev() when we try to emulate through
>> > KVM_MMIO_BUS. This will cause userspace io emulation request instead
>> > of a eventfd notification (virtqueue kick will be trapped by qemu
>> > instead of vhost in this case).
>> >
>> > Fixing this by don't register wildcard mmio eventfd on two
>> > buses. Instead, only register it in KVM_FAST_MMIO_BUS. This fixes the
>> > double free issue of kvm_io_bus_destroy(). For the arch/setups that
>> > does not utilize KVM_FAST_MMIO_BUS, before searching KVM_MMIO_BUS, try
>> > KVM_FAST_MMIO_BUS first to see it it has a match.
>> >
>> > [1] Panic caused by double free:
>> >
>> > CPU: 1 PID: 2894 Comm: qemu-system-x86 Not tainted 3.19.0-26-generic
>> > #28-Ubuntu
>> > Hardware name: LENOVO 2356BG6/2356BG6, BIOS G7ET96WW (2.56 )
>> > 09/12/2013
>> > task: 88009ae0c4b0 ti: 88020e7f task.ti: 88020e7f
>> > RIP: 0010:[] []
>> > ioeventfd_release+0x28/0x60 [kvm]
>> > RSP: 0018:88020e7f3bc8 EFLAGS: 00010292
>> > RAX: dead00200200 RBX: 8801ec19c900 RCX: 00018200016d
>> > RDX: 8801ec19cf80 RSI: ea0008bf1d40 RDI: 8801ec19c900
>> > RBP: 88020e7f3bd8 R08: 2fc75a01 R09: 00018200016d
>> > R10: c07df6ae R11: 88022fc75a98 R12: 88021e7cc000
>> > R13: 88021e7cca48 R14: 88021e7cca50 R15: 8801ec19c880
>> > FS: 7fc1ee3e6700() GS:88023e24()
>> > knlGS:
>> > CS: 0010 DS: ES: CR0: 80050033
>> > CR2: 7f8f389d8000 CR3: 00023dc13000 CR4: 001427e0
>> > Stack:
>> > 88021e7cc000 88020e7f3be8 c07e2622
>> > 88020e7f3c38 c07df69a 880232524160 88020e792d80
>> > 880219b78c00 0008 8802321686a8
>> > Call Trace:
>> > [] ioeventfd_destructor+0x12/0x20 [kvm]
>> > [] kvm_put_kvm+0xca/0x210 [kvm]
>> > [] kvm_vcpu_release+0x18/0x20 [kvm]
>> > [] __fput+0xe7/0x250
>> > [] fput+0xe/0x10
>> > [] task_work_run+0xd4/0xf0
>> > [] do_exit+0x368/0xa50
>> > [] ? recalc_sigpending+0x1f/0x60
>> > [] do_group_exit+0x45/0xb0
>> > [] get_signal+0x291/0x750
>> > [] do_signal+0x28/0xab0
>> > [] ? do_futex+0xdb/0x5d0
>> > [] ? __wake_up_locked_key+0x18/0x20
>> > [] ? SyS_futex+0x76/0x170
>> > [] do_notify_resume+0x69/0xb0
>> > [] int_signal+0x12/0x17
>> > Code: 5d c3 90 0f 1f 44 00 00 55 48 89 e5 53 48 89 fb 48 83 ec 08 48
>> > 8b 7f 20 e8 06 d6 a5 c0 48 8b 43 08 48 8b 13 48 89 df 48 89 42 08
>> > <48> 89 10 48 b8 00 01 10 00 00
>> > RIP [] ioeventfd_release+0x28/0x60 [kvm]
>> > RSP
>> >
>> > Cc: Gleb Natapov
>> > Cc: Paolo Bonzini
>> > Cc: Michael S. Tsirkin
>> > Signed-off-by: Jason Wang
>> > ---
>> > Changes from V2:
>> > - Tweak styles and comment suggested by Cornelia.
>> > Changes from v1:
>> > - change ioeventfd_bus_from_flags() to return KVM_FAST_MMIO_BUS when
>> > needed to save lots of unnecessary changes.
>> > ---
>> > virt/kvm/eventfd.c | 31 +--
>> > virt/kvm/kvm_main.c | 16 ++--
>> > 2 files changed, 23 insertions(+), 24 deletions(-)
>> >
>> > diff --git a/virt/kvm/eventfd.c b/virt/kvm/eventfd.c
>> > index 9ff4193..c3ffdc3 100644
>> > --- a/virt/kvm/eventfd.c
>> > +++ b/virt/kvm/eventfd.c
>> > @@ -762,13 +762,16 @@ ioeventfd_check_collision(struct kvm *kvm,
>> > struct _ioeventfd *p)
>> > return false;
>> > }
>> >
>> > -static enum kvm_bus ioeventfd_bus_from_flags(__u32 flags)
>> > +static enum kvm_bus ioeventfd_bus_from_args(struct kvm_ioeventfd
>> > *args)
>> > {
>> > - if (flags & KVM_IOEVENTFD_FLAG_PIO)
>> > + if (args->flags & KVM_IOEVENTFD_FLAG_PIO)
>> > return KVM_PIO_BUS;
>> > - if (flags & KVM_IOEVENTFD_FLAG_VIRTIO_CCW_NOTIFY)
>> > + if (args->flags & KVM_IOEVENTFD_FLAG_VIRTIO_CCW_NOTIFY)
>> > return KVM_VIRTIO_CCW_NOTIFY_BUS;
>> > - r
Re: [PATCH V3 2/3] kvm: don't register wildcard MMIO EVENTFD on two buses
On 08/25/2015 07:51 PM, Michael S. Tsirkin wrote:
> On Tue, Aug 25, 2015 at 05:05:47PM +0800, Jason Wang wrote:
>> > We register wildcard mmio eventfd on two buses, one for KVM_MMIO_BUS
>> > and another is KVM_FAST_MMIO_BUS. This leads to issue:
>> >
>> > - kvm_io_bus_destroy() knows nothing about the devices on two buses
>> > points to a single dev. Which will lead double free [1] during exit.
>> > - wildcard eventfd ignores data len, so it was registered as a
>> > kvm_io_range with zero length. This will fail the binary search in
>> > kvm_io_bus_get_first_dev() when we try to emulate through
>> > KVM_MMIO_BUS. This will cause userspace io emulation request instead
>> > of a eventfd notification (virtqueue kick will be trapped by qemu
>> > instead of vhost in this case).
>> >
>> > Fixing this by don't register wildcard mmio eventfd on two
>> > buses. Instead, only register it in KVM_FAST_MMIO_BUS. This fixes the
>> > double free issue of kvm_io_bus_destroy(). For the arch/setups that
>> > does not utilize KVM_FAST_MMIO_BUS, before searching KVM_MMIO_BUS, try
>> > KVM_FAST_MMIO_BUS first to see it it has a match.
>> >
>> > [1] Panic caused by double free:
>> >
>> > CPU: 1 PID: 2894 Comm: qemu-system-x86 Not tainted 3.19.0-26-generic
>> > #28-Ubuntu
>> > Hardware name: LENOVO 2356BG6/2356BG6, BIOS G7ET96WW (2.56 ) 09/12/2013
>> > task: 88009ae0c4b0 ti: 88020e7f task.ti: 88020e7f
>> > RIP: 0010:[] []
>> > ioeventfd_release+0x28/0x60 [kvm]
>> > RSP: 0018:88020e7f3bc8 EFLAGS: 00010292
>> > RAX: dead00200200 RBX: 8801ec19c900 RCX: 00018200016d
>> > RDX: 8801ec19cf80 RSI: ea0008bf1d40 RDI: 8801ec19c900
>> > RBP: 88020e7f3bd8 R08: 2fc75a01 R09: 00018200016d
>> > R10: c07df6ae R11: 88022fc75a98 R12: 88021e7cc000
>> > R13: 88021e7cca48 R14: 88021e7cca50 R15: 8801ec19c880
>> > FS: 7fc1ee3e6700() GS:88023e24()
>> > knlGS:
>> > CS: 0010 DS: ES: CR0: 80050033
>> > CR2: 7f8f389d8000 CR3: 00023dc13000 CR4: 001427e0
>> > Stack:
>> > 88021e7cc000 88020e7f3be8 c07e2622
>> > 88020e7f3c38 c07df69a 880232524160 88020e792d80
>> > 880219b78c00 0008 8802321686a8
>> > Call Trace:
>> > [] ioeventfd_destructor+0x12/0x20 [kvm]
>> > [] kvm_put_kvm+0xca/0x210 [kvm]
>> > [] kvm_vcpu_release+0x18/0x20 [kvm]
>> > [] __fput+0xe7/0x250
>> > [] fput+0xe/0x10
>> > [] task_work_run+0xd4/0xf0
>> > [] do_exit+0x368/0xa50
>> > [] ? recalc_sigpending+0x1f/0x60
>> > [] do_group_exit+0x45/0xb0
>> > [] get_signal+0x291/0x750
>> > [] do_signal+0x28/0xab0
>> > [] ? do_futex+0xdb/0x5d0
>> > [] ? __wake_up_locked_key+0x18/0x20
>> > [] ? SyS_futex+0x76/0x170
>> > [] do_notify_resume+0x69/0xb0
>> > [] int_signal+0x12/0x17
>> > Code: 5d c3 90 0f 1f 44 00 00 55 48 89 e5 53 48 89 fb 48 83 ec 08 48 8b 7f
>> > 20 e8 06 d6 a5 c0 48 8b 43 08 48 8b 13 48 89 df 48 89 42 08 <48> 89 10 48
>> > b8 00 01 10 00 00
>> > RIP [] ioeventfd_release+0x28/0x60 [kvm]
>> > RSP
>> >
>> > Cc: Gleb Natapov
>> > Cc: Paolo Bonzini
>> > Cc: Michael S. Tsirkin
>> > Signed-off-by: Jason Wang
>> > ---
>> > Changes from V2:
>> > - Tweak styles and comment suggested by Cornelia.
>> > Changes from v1:
>> > - change ioeventfd_bus_from_flags() to return KVM_FAST_MMIO_BUS when
>> > needed to save lots of unnecessary changes.
>> > ---
>> > virt/kvm/eventfd.c | 31 +--
>> > virt/kvm/kvm_main.c | 16 ++--
>> > 2 files changed, 23 insertions(+), 24 deletions(-)
>> >
>> > diff --git a/virt/kvm/eventfd.c b/virt/kvm/eventfd.c
>> > index 9ff4193..c3ffdc3 100644
>> > --- a/virt/kvm/eventfd.c
>> > +++ b/virt/kvm/eventfd.c
>> > @@ -762,13 +762,16 @@ ioeventfd_check_collision(struct kvm *kvm, struct
>> > _ioeventfd *p)
>> >return false;
>> > }
>> >
>> > -static enum kvm_bus ioeventfd_bus_from_flags(__u32 flags)
>> > +static enum kvm_bus ioeventfd_bus_from_args(struct kvm_ioeventfd *args)
>> > {
>> > - if (flags & KVM_IOEVENTFD_FLAG_PIO)
>> > + if (args->flags & KVM_IOEVENTFD_FLAG_PIO)
>> >return KVM_PIO_BUS;
>> > - if (flags & KVM_IOEVENTFD_FLAG_VIRTIO_CCW_NOTIFY)
>> > + if (args->flags & KVM_IOEVENTFD_FLAG_VIRTIO_CCW_NOTIFY)
>> >return KVM_VIRTIO_CCW_NOTIFY_BUS;
>> > - return KVM_MMIO_BUS;
>> > + /* When length is ignored, MMIO is put on a separate bus, for
>> > + * faster lookups.
>> > + */
>> > + return args->len ? KVM_MMIO_BUS : KVM_FAST_MMIO_BUS;
>> > }
>> >
>> > static int
>> > @@ -779,7 +782,7 @@ kvm_assign_ioeventfd(struct kvm *kvm, struct
>> > kvm_ioeventfd *args)
>> >struct eventfd_ctx *eventfd;
>> >int ret;
>> >
>> > - bus_idx = ioeventfd_bus_from_flags(args->flags);
>> > + bus_idx = ioeventfd_bus_from_args(args);
>> >/* must be natural-word sized, or 0 to ignore length */
Re: [PATCH V3 2/3] kvm: don't register wildcard MMIO EVENTFD on two buses
On Tue, Aug 25, 2015 at 05:05:47PM +0800, Jason Wang wrote:
> We register wildcard mmio eventfd on two buses, one for KVM_MMIO_BUS
> and another is KVM_FAST_MMIO_BUS. This leads to issue:
>
> - kvm_io_bus_destroy() knows nothing about the devices on two buses
> points to a single dev. Which will lead double free [1] during exit.
> - wildcard eventfd ignores data len, so it was registered as a
> kvm_io_range with zero length. This will fail the binary search in
> kvm_io_bus_get_first_dev() when we try to emulate through
> KVM_MMIO_BUS. This will cause userspace io emulation request instead
> of a eventfd notification (virtqueue kick will be trapped by qemu
> instead of vhost in this case).
>
> Fixing this by don't register wildcard mmio eventfd on two
> buses. Instead, only register it in KVM_FAST_MMIO_BUS. This fixes the
> double free issue of kvm_io_bus_destroy(). For the arch/setups that
> does not utilize KVM_FAST_MMIO_BUS, before searching KVM_MMIO_BUS, try
> KVM_FAST_MMIO_BUS first to see it it has a match.
>
> [1] Panic caused by double free:
>
> CPU: 1 PID: 2894 Comm: qemu-system-x86 Not tainted 3.19.0-26-generic
> #28-Ubuntu
> Hardware name: LENOVO 2356BG6/2356BG6, BIOS G7ET96WW (2.56 ) 09/12/2013
> task: 88009ae0c4b0 ti: 88020e7f task.ti: 88020e7f
> RIP: 0010:[] []
> ioeventfd_release+0x28/0x60 [kvm]
> RSP: 0018:88020e7f3bc8 EFLAGS: 00010292
> RAX: dead00200200 RBX: 8801ec19c900 RCX: 00018200016d
> RDX: 8801ec19cf80 RSI: ea0008bf1d40 RDI: 8801ec19c900
> RBP: 88020e7f3bd8 R08: 2fc75a01 R09: 00018200016d
> R10: c07df6ae R11: 88022fc75a98 R12: 88021e7cc000
> R13: 88021e7cca48 R14: 88021e7cca50 R15: 8801ec19c880
> FS: 7fc1ee3e6700() GS:88023e24() knlGS:
> CS: 0010 DS: ES: CR0: 80050033
> CR2: 7f8f389d8000 CR3: 00023dc13000 CR4: 001427e0
> Stack:
> 88021e7cc000 88020e7f3be8 c07e2622
> 88020e7f3c38 c07df69a 880232524160 88020e792d80
> 880219b78c00 0008 8802321686a8
> Call Trace:
> [] ioeventfd_destructor+0x12/0x20 [kvm]
> [] kvm_put_kvm+0xca/0x210 [kvm]
> [] kvm_vcpu_release+0x18/0x20 [kvm]
> [] __fput+0xe7/0x250
> [] fput+0xe/0x10
> [] task_work_run+0xd4/0xf0
> [] do_exit+0x368/0xa50
> [] ? recalc_sigpending+0x1f/0x60
> [] do_group_exit+0x45/0xb0
> [] get_signal+0x291/0x750
> [] do_signal+0x28/0xab0
> [] ? do_futex+0xdb/0x5d0
> [] ? __wake_up_locked_key+0x18/0x20
> [] ? SyS_futex+0x76/0x170
> [] do_notify_resume+0x69/0xb0
> [] int_signal+0x12/0x17
> Code: 5d c3 90 0f 1f 44 00 00 55 48 89 e5 53 48 89 fb 48 83 ec 08 48 8b 7f 20
> e8 06 d6 a5 c0 48 8b 43 08 48 8b 13 48 89 df 48 89 42 08 <48> 89 10 48 b8 00
> 01 10 00 00
> RIP [] ioeventfd_release+0x28/0x60 [kvm]
> RSP
>
> Cc: Gleb Natapov
> Cc: Paolo Bonzini
> Cc: Michael S. Tsirkin
> Signed-off-by: Jason Wang
> ---
> Changes from V2:
> - Tweak styles and comment suggested by Cornelia.
> Changes from v1:
> - change ioeventfd_bus_from_flags() to return KVM_FAST_MMIO_BUS when
> needed to save lots of unnecessary changes.
> ---
> virt/kvm/eventfd.c | 31 +--
> virt/kvm/kvm_main.c | 16 ++--
> 2 files changed, 23 insertions(+), 24 deletions(-)
>
> diff --git a/virt/kvm/eventfd.c b/virt/kvm/eventfd.c
> index 9ff4193..c3ffdc3 100644
> --- a/virt/kvm/eventfd.c
> +++ b/virt/kvm/eventfd.c
> @@ -762,13 +762,16 @@ ioeventfd_check_collision(struct kvm *kvm, struct
> _ioeventfd *p)
> return false;
> }
>
> -static enum kvm_bus ioeventfd_bus_from_flags(__u32 flags)
> +static enum kvm_bus ioeventfd_bus_from_args(struct kvm_ioeventfd *args)
> {
> - if (flags & KVM_IOEVENTFD_FLAG_PIO)
> + if (args->flags & KVM_IOEVENTFD_FLAG_PIO)
> return KVM_PIO_BUS;
> - if (flags & KVM_IOEVENTFD_FLAG_VIRTIO_CCW_NOTIFY)
> + if (args->flags & KVM_IOEVENTFD_FLAG_VIRTIO_CCW_NOTIFY)
> return KVM_VIRTIO_CCW_NOTIFY_BUS;
> - return KVM_MMIO_BUS;
> + /* When length is ignored, MMIO is put on a separate bus, for
> + * faster lookups.
> + */
> + return args->len ? KVM_MMIO_BUS : KVM_FAST_MMIO_BUS;
> }
>
> static int
> @@ -779,7 +782,7 @@ kvm_assign_ioeventfd(struct kvm *kvm, struct
> kvm_ioeventfd *args)
> struct eventfd_ctx *eventfd;
> int ret;
>
> - bus_idx = ioeventfd_bus_from_flags(args->flags);
> + bus_idx = ioeventfd_bus_from_args(args);
> /* must be natural-word sized, or 0 to ignore length */
> switch (args->len) {
> case 0:
> @@ -843,16 +846,6 @@ kvm_assign_ioeventfd(struct kvm *kvm, struct
> kvm_ioeventfd *args)
> if (ret < 0)
> goto unlock_fail;
>
> - /* When length is ignored, MMIO is also put on a separate bus, for
> - * faster lookups.
> - */
> - if (!args->len && !(args
Re: [PATCH V3 2/3] kvm: don't register wildcard MMIO EVENTFD on two buses
On Tue, Aug 25, 2015 at 05:05:47PM +0800, Jason Wang wrote:
> We register wildcard mmio eventfd on two buses, one for KVM_MMIO_BUS
> and another is KVM_FAST_MMIO_BUS. This leads to issue:
>
> - kvm_io_bus_destroy() knows nothing about the devices on two buses
> points to a single dev. Which will lead double free [1] during exit.
> - wildcard eventfd ignores data len, so it was registered as a
> kvm_io_range with zero length. This will fail the binary search in
> kvm_io_bus_get_first_dev() when we try to emulate through
> KVM_MMIO_BUS. This will cause userspace io emulation request instead
> of a eventfd notification (virtqueue kick will be trapped by qemu
> instead of vhost in this case).
>
> Fixing this by don't register wildcard mmio eventfd on two
> buses. Instead, only register it in KVM_FAST_MMIO_BUS. This fixes the
> double free issue of kvm_io_bus_destroy(). For the arch/setups that
> does not utilize KVM_FAST_MMIO_BUS, before searching KVM_MMIO_BUS, try
> KVM_FAST_MMIO_BUS first to see it it has a match.
>
> [1] Panic caused by double free:
>
> CPU: 1 PID: 2894 Comm: qemu-system-x86 Not tainted 3.19.0-26-generic
> #28-Ubuntu
> Hardware name: LENOVO 2356BG6/2356BG6, BIOS G7ET96WW (2.56 ) 09/12/2013
> task: 88009ae0c4b0 ti: 88020e7f task.ti: 88020e7f
> RIP: 0010:[] []
> ioeventfd_release+0x28/0x60 [kvm]
> RSP: 0018:88020e7f3bc8 EFLAGS: 00010292
> RAX: dead00200200 RBX: 8801ec19c900 RCX: 00018200016d
> RDX: 8801ec19cf80 RSI: ea0008bf1d40 RDI: 8801ec19c900
> RBP: 88020e7f3bd8 R08: 2fc75a01 R09: 00018200016d
> R10: c07df6ae R11: 88022fc75a98 R12: 88021e7cc000
> R13: 88021e7cca48 R14: 88021e7cca50 R15: 8801ec19c880
> FS: 7fc1ee3e6700() GS:88023e24() knlGS:
> CS: 0010 DS: ES: CR0: 80050033
> CR2: 7f8f389d8000 CR3: 00023dc13000 CR4: 001427e0
> Stack:
> 88021e7cc000 88020e7f3be8 c07e2622
> 88020e7f3c38 c07df69a 880232524160 88020e792d80
> 880219b78c00 0008 8802321686a8
> Call Trace:
> [] ioeventfd_destructor+0x12/0x20 [kvm]
> [] kvm_put_kvm+0xca/0x210 [kvm]
> [] kvm_vcpu_release+0x18/0x20 [kvm]
> [] __fput+0xe7/0x250
> [] fput+0xe/0x10
> [] task_work_run+0xd4/0xf0
> [] do_exit+0x368/0xa50
> [] ? recalc_sigpending+0x1f/0x60
> [] do_group_exit+0x45/0xb0
> [] get_signal+0x291/0x750
> [] do_signal+0x28/0xab0
> [] ? do_futex+0xdb/0x5d0
> [] ? __wake_up_locked_key+0x18/0x20
> [] ? SyS_futex+0x76/0x170
> [] do_notify_resume+0x69/0xb0
> [] int_signal+0x12/0x17
> Code: 5d c3 90 0f 1f 44 00 00 55 48 89 e5 53 48 89 fb 48 83 ec 08 48 8b 7f 20
> e8 06 d6 a5 c0 48 8b 43 08 48 8b 13 48 89 df 48 89 42 08 <48> 89 10 48 b8 00
> 01 10 00 00
> RIP [] ioeventfd_release+0x28/0x60 [kvm]
> RSP
>
> Cc: Gleb Natapov
> Cc: Paolo Bonzini
> Cc: Michael S. Tsirkin
> Signed-off-by: Jason Wang
Saw v3 too late. Pls see my comments on v2.
> ---
> Changes from V2:
> - Tweak styles and comment suggested by Cornelia.
> Changes from v1:
> - change ioeventfd_bus_from_flags() to return KVM_FAST_MMIO_BUS when
> needed to save lots of unnecessary changes.
> ---
> virt/kvm/eventfd.c | 31 +--
> virt/kvm/kvm_main.c | 16 ++--
> 2 files changed, 23 insertions(+), 24 deletions(-)
>
> diff --git a/virt/kvm/eventfd.c b/virt/kvm/eventfd.c
> index 9ff4193..c3ffdc3 100644
> --- a/virt/kvm/eventfd.c
> +++ b/virt/kvm/eventfd.c
> @@ -762,13 +762,16 @@ ioeventfd_check_collision(struct kvm *kvm, struct
> _ioeventfd *p)
> return false;
> }
>
> -static enum kvm_bus ioeventfd_bus_from_flags(__u32 flags)
> +static enum kvm_bus ioeventfd_bus_from_args(struct kvm_ioeventfd *args)
> {
> - if (flags & KVM_IOEVENTFD_FLAG_PIO)
> + if (args->flags & KVM_IOEVENTFD_FLAG_PIO)
> return KVM_PIO_BUS;
> - if (flags & KVM_IOEVENTFD_FLAG_VIRTIO_CCW_NOTIFY)
> + if (args->flags & KVM_IOEVENTFD_FLAG_VIRTIO_CCW_NOTIFY)
> return KVM_VIRTIO_CCW_NOTIFY_BUS;
> - return KVM_MMIO_BUS;
> + /* When length is ignored, MMIO is put on a separate bus, for
> + * faster lookups.
> + */
> + return args->len ? KVM_MMIO_BUS : KVM_FAST_MMIO_BUS;
> }
>
> static int
> @@ -779,7 +782,7 @@ kvm_assign_ioeventfd(struct kvm *kvm, struct
> kvm_ioeventfd *args)
> struct eventfd_ctx *eventfd;
> int ret;
>
> - bus_idx = ioeventfd_bus_from_flags(args->flags);
> + bus_idx = ioeventfd_bus_from_args(args);
> /* must be natural-word sized, or 0 to ignore length */
> switch (args->len) {
> case 0:
> @@ -843,16 +846,6 @@ kvm_assign_ioeventfd(struct kvm *kvm, struct
> kvm_ioeventfd *args)
> if (ret < 0)
> goto unlock_fail;
>
> - /* When length is ignored, MMIO is also put on a separate bus, for
> - * faster lookups
Re: [PATCH V3 2/3] kvm: don't register wildcard MMIO EVENTFD on two buses
On Tue, 25 Aug 2015 17:05:47 +0800 Jason Wang wrote: > We register wildcard mmio eventfd on two buses, one for KVM_MMIO_BUS > and another is KVM_FAST_MMIO_BUS. This leads to issue: > > - kvm_io_bus_destroy() knows nothing about the devices on two buses > points to a single dev. Which will lead double free [1] during exit. > - wildcard eventfd ignores data len, so it was registered as a > kvm_io_range with zero length. This will fail the binary search in > kvm_io_bus_get_first_dev() when we try to emulate through > KVM_MMIO_BUS. This will cause userspace io emulation request instead > of a eventfd notification (virtqueue kick will be trapped by qemu > instead of vhost in this case). > > Fixing this by don't register wildcard mmio eventfd on two > buses. Instead, only register it in KVM_FAST_MMIO_BUS. This fixes the > double free issue of kvm_io_bus_destroy(). For the arch/setups that > does not utilize KVM_FAST_MMIO_BUS, before searching KVM_MMIO_BUS, try > KVM_FAST_MMIO_BUS first to see it it has a match. > > [1] Panic caused by double free: > > CPU: 1 PID: 2894 Comm: qemu-system-x86 Not tainted 3.19.0-26-generic > #28-Ubuntu > Hardware name: LENOVO 2356BG6/2356BG6, BIOS G7ET96WW (2.56 ) 09/12/2013 > task: 88009ae0c4b0 ti: 88020e7f task.ti: 88020e7f > RIP: 0010:[] [] > ioeventfd_release+0x28/0x60 [kvm] > RSP: 0018:88020e7f3bc8 EFLAGS: 00010292 > RAX: dead00200200 RBX: 8801ec19c900 RCX: 00018200016d > RDX: 8801ec19cf80 RSI: ea0008bf1d40 RDI: 8801ec19c900 > RBP: 88020e7f3bd8 R08: 2fc75a01 R09: 00018200016d > R10: c07df6ae R11: 88022fc75a98 R12: 88021e7cc000 > R13: 88021e7cca48 R14: 88021e7cca50 R15: 8801ec19c880 > FS: 7fc1ee3e6700() GS:88023e24() knlGS: > CS: 0010 DS: ES: CR0: 80050033 > CR2: 7f8f389d8000 CR3: 00023dc13000 CR4: 001427e0 > Stack: > 88021e7cc000 88020e7f3be8 c07e2622 > 88020e7f3c38 c07df69a 880232524160 88020e792d80 > 880219b78c00 0008 8802321686a8 > Call Trace: > [] ioeventfd_destructor+0x12/0x20 [kvm] > [] kvm_put_kvm+0xca/0x210 [kvm] > [] kvm_vcpu_release+0x18/0x20 [kvm] > [] __fput+0xe7/0x250 > [] fput+0xe/0x10 > [] task_work_run+0xd4/0xf0 > [] do_exit+0x368/0xa50 > [] ? recalc_sigpending+0x1f/0x60 > [] do_group_exit+0x45/0xb0 > [] get_signal+0x291/0x750 > [] do_signal+0x28/0xab0 > [] ? do_futex+0xdb/0x5d0 > [] ? __wake_up_locked_key+0x18/0x20 > [] ? SyS_futex+0x76/0x170 > [] do_notify_resume+0x69/0xb0 > [] int_signal+0x12/0x17 > Code: 5d c3 90 0f 1f 44 00 00 55 48 89 e5 53 48 89 fb 48 83 ec 08 48 8b 7f 20 > e8 06 d6 a5 c0 48 8b 43 08 48 8b 13 48 89 df 48 89 42 08 <48> 89 10 48 b8 00 > 01 10 00 00 > RIP [] ioeventfd_release+0x28/0x60 [kvm] > RSP > > Cc: Gleb Natapov > Cc: Paolo Bonzini > Cc: Michael S. Tsirkin > Signed-off-by: Jason Wang > --- > Changes from V2: > - Tweak styles and comment suggested by Cornelia. > Changes from v1: > - change ioeventfd_bus_from_flags() to return KVM_FAST_MMIO_BUS when > needed to save lots of unnecessary changes. > --- > virt/kvm/eventfd.c | 31 +-- > virt/kvm/kvm_main.c | 16 ++-- > 2 files changed, 23 insertions(+), 24 deletions(-) Acked-by: Cornelia Huck -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
