diff for duplicates of <20191217225118.GF11771@linux.intel.com> diff --git a/a/1.txt b/N1/1.txt index 50b7e29..aa182f1 100644 --- a/a/1.txt +++ b/N1/1.txt @@ -27,9 +27,9 @@ On Tue, Dec 17, 2019 at 05:24:46PM -0500, Peter Xu wrote: > > + goto out_bitmap; > > memcpy(slots, __kvm_memslots(kvm, as_id), sizeof(struct kvm_memslots)); > > -> > if ((change = KVM_MR_DELETE) || (change = KVM_MR_MOVE)) { +> > if ((change == KVM_MR_DELETE) || (change == KVM_MR_MOVE)) { > > @@ -1144,8 +1144,9 @@ int __kvm_set_memory_region(struct kvm *kvm, -> > if (change = KVM_MR_DELETE || change = KVM_MR_MOVE) +> > if (change == KVM_MR_DELETE || change == KVM_MR_MOVE) > > slots = install_new_memslots(kvm, as_id, slots); > > kvfree(slots); > > -out_free: @@ -42,7 +42,7 @@ On Tue, Dec 17, 2019 at 05:24:46PM -0500, Peter Xu wrote: > kvm_free_memslot() did cover that but I see that you explicitly > dropped it. Could I ask why? Thanks, -In that case, old.dirty_bitmap = new.dirty_bitmap, i.e. shouldn't be freed +In that case, old.dirty_bitmap == new.dirty_bitmap, i.e. shouldn't be freed by this error path since doing so would result in a use-after-free via the old memslot. @@ -54,7 +54,7 @@ In __kvm_set_memory_region(), @old and @new start with the same dirty_bitmap. And @new is modified based on KVM_MEM_LOG_DIRTY_PAGES. If LOG_DIRTY_PAGES is set in both @new and @old, then both the "if" and "else if" evaluate -false, i.e. new.dirty_bitmap = old.dirty_bitmap. +false, i.e. new.dirty_bitmap == old.dirty_bitmap. /* Allocate/free page dirty bitmap as needed */ if (!(new.flags & KVM_MEM_LOG_DIRTY_PAGES)) @@ -81,8 +81,12 @@ yeids this, since @old is obviously non-NULL The dirty_bitmap allocation logic guarantees that new.dirty_bitmap is a) NULL (the "if" case") - b) != old.dirty_bitmap iff old.dirty_bitmap = NULL (the "else if" case) - c) = old.dirty_bitmap (the implicit "else" case). + b) != old.dirty_bitmap iff old.dirty_bitmap == NULL (the "else if" case) + c) == old.dirty_bitmap (the implicit "else" case). kvm_free_memslot() frees @new.dirty_bitmap iff its != @old.dirty_bitmap, thus the explicit destroy only needs to check for (b). +_______________________________________________ +kvmarm mailing list +kvmarm@lists.cs.columbia.edu +https://lists.cs.columbia.edu/mailman/listinfo/kvmarm diff --git a/a/content_digest b/N1/content_digest index ecf2476..7d5064e 100644 --- a/a/content_digest +++ b/N1/content_digest @@ -3,26 +3,26 @@ "ref\020191217222446.GK7258@xz-x1\0" "From\0Sean Christopherson <sean.j.christopherson@intel.com>\0" "Subject\0Re: [PATCH v4 07/19] KVM: Explicitly free allocated-but-unused dirty bitmap\0" - "Date\0Tue, 17 Dec 2019 22:51:18 +0000\0" + "Date\0Tue, 17 Dec 2019 14:51:18 -0800\0" "To\0Peter Xu <peterx@redhat.com>\0" - "Cc\0James Hogan <jhogan@kernel.org>" - Paul Mackerras <paulus@ozlabs.org> - Christian Borntraeger <borntraeger@de.ibm.com> + "Cc\0Wanpeng Li <wanpengli@tencent.com>" Janosch Frank <frankja@linux.ibm.com> - Paolo Bonzini <pbonzini@redhat.com> - Marc Zyngier <maz@kernel.org> - linux-arm-kernel@lists.infradead.org - Wanpeng Li <wanpengli@tencent.com> - " Philippe Mathieu-Daud\303\251 <f4bug@amsat.org>" kvm@vger.kernel.org David Hildenbrand <david@redhat.com> + James Hogan <jhogan@kernel.org> Joerg Roedel <joro@8bytes.org> Cornelia Huck <cohuck@redhat.com> - linux-mips@vger.kernel.org linux-kernel@vger.kernel.org + " Philippe Mathieu-Daud\303\251 <f4bug@amsat.org>" kvm-ppc@vger.kernel.org + linux-mips@vger.kernel.org + Paul Mackerras <paulus@ozlabs.org> + Christian Borntraeger <borntraeger@de.ibm.com> + Marc Zyngier <maz@kernel.org> + Paolo Bonzini <pbonzini@redhat.com> Vitaly Kuznetsov <vkuznets@redhat.com> kvmarm@lists.cs.columbia.edu + linux-arm-kernel@lists.infradead.org " Jim Mattson <jmattson@google.com>\0" "\00:1\0" "b\0" @@ -55,9 +55,9 @@ "> > +\t\tgoto out_bitmap;\n" "> > \tmemcpy(slots, __kvm_memslots(kvm, as_id), sizeof(struct kvm_memslots));\n" "> > \n" - "> > \tif ((change = KVM_MR_DELETE) || (change = KVM_MR_MOVE)) {\n" + "> > \tif ((change == KVM_MR_DELETE) || (change == KVM_MR_MOVE)) {\n" "> > @@ -1144,8 +1144,9 @@ int __kvm_set_memory_region(struct kvm *kvm,\n" - "> > \tif (change = KVM_MR_DELETE || change = KVM_MR_MOVE)\n" + "> > \tif (change == KVM_MR_DELETE || change == KVM_MR_MOVE)\n" "> > \t\tslots = install_new_memslots(kvm, as_id, slots);\n" "> > \tkvfree(slots);\n" "> > -out_free:\n" @@ -70,7 +70,7 @@ "> kvm_free_memslot() did cover that but I see that you explicitly\n" "> dropped it. Could I ask why? Thanks,\n" "\n" - "In that case, old.dirty_bitmap = new.dirty_bitmap, i.e. shouldn't be freed\n" + "In that case, old.dirty_bitmap == new.dirty_bitmap, i.e. shouldn't be freed\n" "by this error path since doing so would result in a use-after-free via the\n" "old memslot.\n" "\n" @@ -82,7 +82,7 @@ "\n" "And @new is modified based on KVM_MEM_LOG_DIRTY_PAGES. If LOG_DIRTY_PAGES\n" "is set in both @new and @old, then both the \"if\" and \"else if\" evaluate\n" - "false, i.e. new.dirty_bitmap = old.dirty_bitmap.\n" + "false, i.e. new.dirty_bitmap == old.dirty_bitmap.\n" "\n" "\t/* Allocate/free page dirty bitmap as needed */\n" "\tif (!(new.flags & KVM_MEM_LOG_DIRTY_PAGES))\n" @@ -109,10 +109,14 @@ "\n" "The dirty_bitmap allocation logic guarantees that new.dirty_bitmap is\n" " a) NULL (the \"if\" case\")\n" - " b) != old.dirty_bitmap iff old.dirty_bitmap = NULL (the \"else if\" case)\n" - " c) = old.dirty_bitmap (the implicit \"else\" case).\n" + " b) != old.dirty_bitmap iff old.dirty_bitmap == NULL (the \"else if\" case)\n" + " c) == old.dirty_bitmap (the implicit \"else\" case).\n" "\n" "kvm_free_memslot() frees @new.dirty_bitmap iff its != @old.dirty_bitmap,\n" - thus the explicit destroy only needs to check for (b). + "thus the explicit destroy only needs to check for (b).\n" + "_______________________________________________\n" + "kvmarm mailing list\n" + "kvmarm@lists.cs.columbia.edu\n" + https://lists.cs.columbia.edu/mailman/listinfo/kvmarm -65e686d48a8a5a1d04aceddc1956aee66133417652c6e15af34df3408b98d281 +6722174add51f3eb29a4a5559af2c38edb219667305492532a18017ec5eb961a
diff --git a/a/1.txt b/N2/1.txt index 50b7e29..74b315d 100644 --- a/a/1.txt +++ b/N2/1.txt @@ -27,9 +27,9 @@ On Tue, Dec 17, 2019 at 05:24:46PM -0500, Peter Xu wrote: > > + goto out_bitmap; > > memcpy(slots, __kvm_memslots(kvm, as_id), sizeof(struct kvm_memslots)); > > -> > if ((change = KVM_MR_DELETE) || (change = KVM_MR_MOVE)) { +> > if ((change == KVM_MR_DELETE) || (change == KVM_MR_MOVE)) { > > @@ -1144,8 +1144,9 @@ int __kvm_set_memory_region(struct kvm *kvm, -> > if (change = KVM_MR_DELETE || change = KVM_MR_MOVE) +> > if (change == KVM_MR_DELETE || change == KVM_MR_MOVE) > > slots = install_new_memslots(kvm, as_id, slots); > > kvfree(slots); > > -out_free: @@ -42,7 +42,7 @@ On Tue, Dec 17, 2019 at 05:24:46PM -0500, Peter Xu wrote: > kvm_free_memslot() did cover that but I see that you explicitly > dropped it. Could I ask why? Thanks, -In that case, old.dirty_bitmap = new.dirty_bitmap, i.e. shouldn't be freed +In that case, old.dirty_bitmap == new.dirty_bitmap, i.e. shouldn't be freed by this error path since doing so would result in a use-after-free via the old memslot. @@ -54,7 +54,7 @@ In __kvm_set_memory_region(), @old and @new start with the same dirty_bitmap. And @new is modified based on KVM_MEM_LOG_DIRTY_PAGES. If LOG_DIRTY_PAGES is set in both @new and @old, then both the "if" and "else if" evaluate -false, i.e. new.dirty_bitmap = old.dirty_bitmap. +false, i.e. new.dirty_bitmap == old.dirty_bitmap. /* Allocate/free page dirty bitmap as needed */ if (!(new.flags & KVM_MEM_LOG_DIRTY_PAGES)) @@ -81,8 +81,8 @@ yeids this, since @old is obviously non-NULL The dirty_bitmap allocation logic guarantees that new.dirty_bitmap is a) NULL (the "if" case") - b) != old.dirty_bitmap iff old.dirty_bitmap = NULL (the "else if" case) - c) = old.dirty_bitmap (the implicit "else" case). + b) != old.dirty_bitmap iff old.dirty_bitmap == NULL (the "else if" case) + c) == old.dirty_bitmap (the implicit "else" case). kvm_free_memslot() frees @new.dirty_bitmap iff its != @old.dirty_bitmap, thus the explicit destroy only needs to check for (b). diff --git a/a/content_digest b/N2/content_digest index ecf2476..994731f 100644 --- a/a/content_digest +++ b/N2/content_digest @@ -3,7 +3,7 @@ "ref\020191217222446.GK7258@xz-x1\0" "From\0Sean Christopherson <sean.j.christopherson@intel.com>\0" "Subject\0Re: [PATCH v4 07/19] KVM: Explicitly free allocated-but-unused dirty bitmap\0" - "Date\0Tue, 17 Dec 2019 22:51:18 +0000\0" + "Date\0Tue, 17 Dec 2019 14:51:18 -0800\0" "To\0Peter Xu <peterx@redhat.com>\0" "Cc\0James Hogan <jhogan@kernel.org>" Paul Mackerras <paulus@ozlabs.org> @@ -55,9 +55,9 @@ "> > +\t\tgoto out_bitmap;\n" "> > \tmemcpy(slots, __kvm_memslots(kvm, as_id), sizeof(struct kvm_memslots));\n" "> > \n" - "> > \tif ((change = KVM_MR_DELETE) || (change = KVM_MR_MOVE)) {\n" + "> > \tif ((change == KVM_MR_DELETE) || (change == KVM_MR_MOVE)) {\n" "> > @@ -1144,8 +1144,9 @@ int __kvm_set_memory_region(struct kvm *kvm,\n" - "> > \tif (change = KVM_MR_DELETE || change = KVM_MR_MOVE)\n" + "> > \tif (change == KVM_MR_DELETE || change == KVM_MR_MOVE)\n" "> > \t\tslots = install_new_memslots(kvm, as_id, slots);\n" "> > \tkvfree(slots);\n" "> > -out_free:\n" @@ -70,7 +70,7 @@ "> kvm_free_memslot() did cover that but I see that you explicitly\n" "> dropped it. Could I ask why? Thanks,\n" "\n" - "In that case, old.dirty_bitmap = new.dirty_bitmap, i.e. shouldn't be freed\n" + "In that case, old.dirty_bitmap == new.dirty_bitmap, i.e. shouldn't be freed\n" "by this error path since doing so would result in a use-after-free via the\n" "old memslot.\n" "\n" @@ -82,7 +82,7 @@ "\n" "And @new is modified based on KVM_MEM_LOG_DIRTY_PAGES. If LOG_DIRTY_PAGES\n" "is set in both @new and @old, then both the \"if\" and \"else if\" evaluate\n" - "false, i.e. new.dirty_bitmap = old.dirty_bitmap.\n" + "false, i.e. new.dirty_bitmap == old.dirty_bitmap.\n" "\n" "\t/* Allocate/free page dirty bitmap as needed */\n" "\tif (!(new.flags & KVM_MEM_LOG_DIRTY_PAGES))\n" @@ -109,10 +109,10 @@ "\n" "The dirty_bitmap allocation logic guarantees that new.dirty_bitmap is\n" " a) NULL (the \"if\" case\")\n" - " b) != old.dirty_bitmap iff old.dirty_bitmap = NULL (the \"else if\" case)\n" - " c) = old.dirty_bitmap (the implicit \"else\" case).\n" + " b) != old.dirty_bitmap iff old.dirty_bitmap == NULL (the \"else if\" case)\n" + " c) == old.dirty_bitmap (the implicit \"else\" case).\n" "\n" "kvm_free_memslot() frees @new.dirty_bitmap iff its != @old.dirty_bitmap,\n" thus the explicit destroy only needs to check for (b). -65e686d48a8a5a1d04aceddc1956aee66133417652c6e15af34df3408b98d281 +6486917bb05e885d54ce709b8c38c8fff3a834efe22dfe9559c31b453eaa36f6
diff --git a/a/1.txt b/N3/1.txt index 50b7e29..61d2607 100644 --- a/a/1.txt +++ b/N3/1.txt @@ -27,9 +27,9 @@ On Tue, Dec 17, 2019 at 05:24:46PM -0500, Peter Xu wrote: > > + goto out_bitmap; > > memcpy(slots, __kvm_memslots(kvm, as_id), sizeof(struct kvm_memslots)); > > -> > if ((change = KVM_MR_DELETE) || (change = KVM_MR_MOVE)) { +> > if ((change == KVM_MR_DELETE) || (change == KVM_MR_MOVE)) { > > @@ -1144,8 +1144,9 @@ int __kvm_set_memory_region(struct kvm *kvm, -> > if (change = KVM_MR_DELETE || change = KVM_MR_MOVE) +> > if (change == KVM_MR_DELETE || change == KVM_MR_MOVE) > > slots = install_new_memslots(kvm, as_id, slots); > > kvfree(slots); > > -out_free: @@ -42,7 +42,7 @@ On Tue, Dec 17, 2019 at 05:24:46PM -0500, Peter Xu wrote: > kvm_free_memslot() did cover that but I see that you explicitly > dropped it. Could I ask why? Thanks, -In that case, old.dirty_bitmap = new.dirty_bitmap, i.e. shouldn't be freed +In that case, old.dirty_bitmap == new.dirty_bitmap, i.e. shouldn't be freed by this error path since doing so would result in a use-after-free via the old memslot. @@ -54,7 +54,7 @@ In __kvm_set_memory_region(), @old and @new start with the same dirty_bitmap. And @new is modified based on KVM_MEM_LOG_DIRTY_PAGES. If LOG_DIRTY_PAGES is set in both @new and @old, then both the "if" and "else if" evaluate -false, i.e. new.dirty_bitmap = old.dirty_bitmap. +false, i.e. new.dirty_bitmap == old.dirty_bitmap. /* Allocate/free page dirty bitmap as needed */ if (!(new.flags & KVM_MEM_LOG_DIRTY_PAGES)) @@ -81,8 +81,13 @@ yeids this, since @old is obviously non-NULL The dirty_bitmap allocation logic guarantees that new.dirty_bitmap is a) NULL (the "if" case") - b) != old.dirty_bitmap iff old.dirty_bitmap = NULL (the "else if" case) - c) = old.dirty_bitmap (the implicit "else" case). + b) != old.dirty_bitmap iff old.dirty_bitmap == NULL (the "else if" case) + c) == old.dirty_bitmap (the implicit "else" case). kvm_free_memslot() frees @new.dirty_bitmap iff its != @old.dirty_bitmap, thus the explicit destroy only needs to check for (b). + +_______________________________________________ +linux-arm-kernel mailing list +linux-arm-kernel@lists.infradead.org +http://lists.infradead.org/mailman/listinfo/linux-arm-kernel diff --git a/a/content_digest b/N3/content_digest index ecf2476..a64c0ea 100644 --- a/a/content_digest +++ b/N3/content_digest @@ -3,26 +3,26 @@ "ref\020191217222446.GK7258@xz-x1\0" "From\0Sean Christopherson <sean.j.christopherson@intel.com>\0" "Subject\0Re: [PATCH v4 07/19] KVM: Explicitly free allocated-but-unused dirty bitmap\0" - "Date\0Tue, 17 Dec 2019 22:51:18 +0000\0" + "Date\0Tue, 17 Dec 2019 14:51:18 -0800\0" "To\0Peter Xu <peterx@redhat.com>\0" - "Cc\0James Hogan <jhogan@kernel.org>" - Paul Mackerras <paulus@ozlabs.org> - Christian Borntraeger <borntraeger@de.ibm.com> + "Cc\0Wanpeng Li <wanpengli@tencent.com>" Janosch Frank <frankja@linux.ibm.com> - Paolo Bonzini <pbonzini@redhat.com> - Marc Zyngier <maz@kernel.org> - linux-arm-kernel@lists.infradead.org - Wanpeng Li <wanpengli@tencent.com> - " Philippe Mathieu-Daud\303\251 <f4bug@amsat.org>" kvm@vger.kernel.org David Hildenbrand <david@redhat.com> + James Hogan <jhogan@kernel.org> Joerg Roedel <joro@8bytes.org> Cornelia Huck <cohuck@redhat.com> - linux-mips@vger.kernel.org linux-kernel@vger.kernel.org + " Philippe Mathieu-Daud\303\251 <f4bug@amsat.org>" kvm-ppc@vger.kernel.org + linux-mips@vger.kernel.org + Paul Mackerras <paulus@ozlabs.org> + Christian Borntraeger <borntraeger@de.ibm.com> + Marc Zyngier <maz@kernel.org> + Paolo Bonzini <pbonzini@redhat.com> Vitaly Kuznetsov <vkuznets@redhat.com> kvmarm@lists.cs.columbia.edu + linux-arm-kernel@lists.infradead.org " Jim Mattson <jmattson@google.com>\0" "\00:1\0" "b\0" @@ -55,9 +55,9 @@ "> > +\t\tgoto out_bitmap;\n" "> > \tmemcpy(slots, __kvm_memslots(kvm, as_id), sizeof(struct kvm_memslots));\n" "> > \n" - "> > \tif ((change = KVM_MR_DELETE) || (change = KVM_MR_MOVE)) {\n" + "> > \tif ((change == KVM_MR_DELETE) || (change == KVM_MR_MOVE)) {\n" "> > @@ -1144,8 +1144,9 @@ int __kvm_set_memory_region(struct kvm *kvm,\n" - "> > \tif (change = KVM_MR_DELETE || change = KVM_MR_MOVE)\n" + "> > \tif (change == KVM_MR_DELETE || change == KVM_MR_MOVE)\n" "> > \t\tslots = install_new_memslots(kvm, as_id, slots);\n" "> > \tkvfree(slots);\n" "> > -out_free:\n" @@ -70,7 +70,7 @@ "> kvm_free_memslot() did cover that but I see that you explicitly\n" "> dropped it. Could I ask why? Thanks,\n" "\n" - "In that case, old.dirty_bitmap = new.dirty_bitmap, i.e. shouldn't be freed\n" + "In that case, old.dirty_bitmap == new.dirty_bitmap, i.e. shouldn't be freed\n" "by this error path since doing so would result in a use-after-free via the\n" "old memslot.\n" "\n" @@ -82,7 +82,7 @@ "\n" "And @new is modified based on KVM_MEM_LOG_DIRTY_PAGES. If LOG_DIRTY_PAGES\n" "is set in both @new and @old, then both the \"if\" and \"else if\" evaluate\n" - "false, i.e. new.dirty_bitmap = old.dirty_bitmap.\n" + "false, i.e. new.dirty_bitmap == old.dirty_bitmap.\n" "\n" "\t/* Allocate/free page dirty bitmap as needed */\n" "\tif (!(new.flags & KVM_MEM_LOG_DIRTY_PAGES))\n" @@ -109,10 +109,15 @@ "\n" "The dirty_bitmap allocation logic guarantees that new.dirty_bitmap is\n" " a) NULL (the \"if\" case\")\n" - " b) != old.dirty_bitmap iff old.dirty_bitmap = NULL (the \"else if\" case)\n" - " c) = old.dirty_bitmap (the implicit \"else\" case).\n" + " b) != old.dirty_bitmap iff old.dirty_bitmap == NULL (the \"else if\" case)\n" + " c) == old.dirty_bitmap (the implicit \"else\" case).\n" "\n" "kvm_free_memslot() frees @new.dirty_bitmap iff its != @old.dirty_bitmap,\n" - thus the explicit destroy only needs to check for (b). + "thus the explicit destroy only needs to check for (b).\n" + "\n" + "_______________________________________________\n" + "linux-arm-kernel mailing list\n" + "linux-arm-kernel@lists.infradead.org\n" + http://lists.infradead.org/mailman/listinfo/linux-arm-kernel -65e686d48a8a5a1d04aceddc1956aee66133417652c6e15af34df3408b98d281 +68b1110c82f69e0c7e78576805eb372d2d4c6b1667e83a783323902ff0ddb89e
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.