* [PATCH 23/82] KVM: Refactor intentional wrap-around calculation
[not found] <20240122235208.work.748-kees@kernel.org>
@ 2024-01-23 0:26 ` Kees Cook
2024-01-24 16:25 ` Sean Christopherson
2024-01-23 0:27 ` [PATCH 25/82] KVM: SVM: " Kees Cook
` (2 subsequent siblings)
3 siblings, 1 reply; 8+ messages in thread
From: Kees Cook @ 2024-01-23 0:26 UTC (permalink / raw)
To: linux-hardening
Cc: Kees Cook, Paolo Bonzini, kvm, Gustavo A. R. Silva, Bill Wendling,
Justin Stitt, linux-kernel
In an effort to separate intentional arithmetic wrap-around from
unexpected wrap-around, we need to refactor places that depend on this
kind of math. One of the most common code patterns of this is:
VAR + value < VAR
Notable, this is considered "undefined behavior" for signed and pointer
types, which the kernel works around by using the -fno-strict-overflow
option in the build[1] (which used to just be -fwrapv). Regardless, we
want to get the kernel source to the position where we can meaningfully
instrument arithmetic wrap-around conditions and catch them when they
are unexpected, regardless of whether they are signed, unsigned, or
pointer types.
Refactor open-coded unsigned wrap-around addition test to use
check_add_overflow(), retaining the result for later usage (which removes
the redundant open-coded addition). This paves the way to enabling the
unsigned wrap-around sanitizer[2] in the future.
Link: https://git.kernel.org/linus/68df3755e383e6fecf2354a67b08f92f18536594 [1]
Link: https://github.com/KSPP/linux/issues/27 [2]
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: kvm@vger.kernel.org
Signed-off-by: Kees Cook <keescook@chromium.org>
---
virt/kvm/coalesced_mmio.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/virt/kvm/coalesced_mmio.c b/virt/kvm/coalesced_mmio.c
index 1b90acb6e3fe..0a3b706fbf4c 100644
--- a/virt/kvm/coalesced_mmio.c
+++ b/virt/kvm/coalesced_mmio.c
@@ -25,17 +25,19 @@ static inline struct kvm_coalesced_mmio_dev *to_mmio(struct kvm_io_device *dev)
static int coalesced_mmio_in_range(struct kvm_coalesced_mmio_dev *dev,
gpa_t addr, int len)
{
+ gpa_t sum;
+
/* is it in a batchable area ?
* (addr,len) is fully included in
* (zone->addr, zone->size)
*/
if (len < 0)
return 0;
- if (addr + len < addr)
+ if (check_add_overflow(addr, len, &sum))
return 0;
if (addr < dev->zone.addr)
return 0;
- if (addr + len > dev->zone.addr + dev->zone.size)
+ if (sum > dev->zone.addr + dev->zone.size)
return 0;
return 1;
}
--
2.34.1
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH 23/82] KVM: Refactor intentional wrap-around calculation
2024-01-23 0:26 ` [PATCH 23/82] KVM: Refactor intentional wrap-around calculation Kees Cook
@ 2024-01-24 16:25 ` Sean Christopherson
0 siblings, 0 replies; 8+ messages in thread
From: Sean Christopherson @ 2024-01-24 16:25 UTC (permalink / raw)
To: Kees Cook
Cc: linux-hardening, Paolo Bonzini, kvm, Gustavo A. R. Silva,
Bill Wendling, Justin Stitt, linux-kernel
On Mon, Jan 22, 2024, Kees Cook wrote:
> In an effort to separate intentional arithmetic wrap-around from
> unexpected wrap-around, we need to refactor places that depend on this
> kind of math. One of the most common code patterns of this is:
>
> VAR + value < VAR
>
> Notable, this is considered "undefined behavior" for signed and pointer
> types, which the kernel works around by using the -fno-strict-overflow
> option in the build[1] (which used to just be -fwrapv). Regardless, we
> want to get the kernel source to the position where we can meaningfully
> instrument arithmetic wrap-around conditions and catch them when they
> are unexpected, regardless of whether they are signed, unsigned, or
> pointer types.
>
> Refactor open-coded unsigned wrap-around addition test to use
> check_add_overflow(), retaining the result for later usage (which removes
> the redundant open-coded addition). This paves the way to enabling the
> unsigned wrap-around sanitizer[2] in the future.
>
> Link: https://git.kernel.org/linus/68df3755e383e6fecf2354a67b08f92f18536594 [1]
> Link: https://github.com/KSPP/linux/issues/27 [2]
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: kvm@vger.kernel.org
> Signed-off-by: Kees Cook <keescook@chromium.org>
> ---
> virt/kvm/coalesced_mmio.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/virt/kvm/coalesced_mmio.c b/virt/kvm/coalesced_mmio.c
> index 1b90acb6e3fe..0a3b706fbf4c 100644
> --- a/virt/kvm/coalesced_mmio.c
> +++ b/virt/kvm/coalesced_mmio.c
> @@ -25,17 +25,19 @@ static inline struct kvm_coalesced_mmio_dev *to_mmio(struct kvm_io_device *dev)
> static int coalesced_mmio_in_range(struct kvm_coalesced_mmio_dev *dev,
> gpa_t addr, int len)
> {
> + gpa_t sum;
s/sum/end?
Also, given that your're fixing a gpa_t, which is a u64, presumably that means
that this code in __kvm_set_memory_region() also needs to be fixed:
if (mem->guest_phys_addr + mem->memory_size < mem->guest_phys_addr)
return -EINVAL;
and for that one I'd really like to avoid an ignored output parameter (KVM
converts the incoming mem->memory_size to pages, so the "sum" is never used
directly).
Would it make sense to add an API that feeds a dummy "sum" value? I assume UBSAN
won't fire on the usage of the known good value, i.e. using the output parameter
isn't necessary for functional correctness. Having an API that does just the
check would trim down the size of many of these patches and avoid having to come
up with names for the local variables. And IMO, the existing code is a wee bit
more intuitive, it'd be nice to give developers the flexibility to choose which
flavor yields the "best" code on a case-by-case basis.
> +
> /* is it in a batchable area ?
> * (addr,len) is fully included in
> * (zone->addr, zone->size)
> */
> if (len < 0)
> return 0;
> - if (addr + len < addr)
> + if (check_add_overflow(addr, len, &sum))
> return 0;
> if (addr < dev->zone.addr)
> return 0;
> - if (addr + len > dev->zone.addr + dev->zone.size)
> + if (sum > dev->zone.addr + dev->zone.size)
> return 0;
> return 1;
> }
> --
> 2.34.1
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 25/82] KVM: SVM: Refactor intentional wrap-around calculation
[not found] <20240122235208.work.748-kees@kernel.org>
2024-01-23 0:26 ` [PATCH 23/82] KVM: Refactor intentional wrap-around calculation Kees Cook
@ 2024-01-23 0:27 ` Kees Cook
2024-01-24 16:15 ` Sean Christopherson
2024-01-23 0:27 ` [PATCH 32/82] vringh: " Kees Cook
2024-01-23 0:27 ` [PATCH 58/82] s390/mm: Refactor intentional wrap-around test Kees Cook
3 siblings, 1 reply; 8+ messages in thread
From: Kees Cook @ 2024-01-23 0:27 UTC (permalink / raw)
To: linux-hardening
Cc: Kees Cook, Sean Christopherson, Paolo Bonzini, Thomas Gleixner,
Ingo Molnar, Borislav Petkov, Dave Hansen, x86, H. Peter Anvin,
kvm, Gustavo A. R. Silva, Bill Wendling, Justin Stitt,
linux-kernel
In an effort to separate intentional arithmetic wrap-around from
unexpected wrap-around, we need to refactor places that depend on this
kind of math. One of the most common code patterns of this is:
VAR + value < VAR
Notably, this is considered "undefined behavior" for signed and pointer
types, which the kernel works around by using the -fno-strict-overflow
option in the build[1] (which used to just be -fwrapv). Regardless, we
want to get the kernel source to the position where we can meaningfully
instrument arithmetic wrap-around conditions and catch them when they
are unexpected, regardless of whether they are signed[2], unsigned[3],
or pointer[4] types.
Refactor open-coded unsigned wrap-around addition test to use
check_add_overflow(), retaining the result for later usage (which removes
the redundant open-coded addition). This paves the way to enabling the
wrap-around sanitizers in the future.
Link: https://git.kernel.org/linus/68df3755e383e6fecf2354a67b08f92f18536594 [1]
Link: https://github.com/KSPP/linux/issues/26 [2]
Link: https://github.com/KSPP/linux/issues/27 [3]
Link: https://github.com/KSPP/linux/issues/344 [4]
Cc: Sean Christopherson <seanjc@google.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Dave Hansen <dave.hansen@linux.intel.com>
Cc: x86@kernel.org
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: kvm@vger.kernel.org
Signed-off-by: Kees Cook <keescook@chromium.org>
---
arch/x86/kvm/svm/sev.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
index f760106c31f8..12a6a2b1ac81 100644
--- a/arch/x86/kvm/svm/sev.c
+++ b/arch/x86/kvm/svm/sev.c
@@ -400,16 +400,17 @@ static struct page **sev_pin_memory(struct kvm *kvm, unsigned long uaddr,
unsigned long locked, lock_limit;
struct page **pages;
unsigned long first, last;
+ unsigned long sum;
int ret;
lockdep_assert_held(&kvm->lock);
- if (ulen == 0 || uaddr + ulen < uaddr)
+ if (ulen == 0 || check_add_overflow(uaddr, ulen, &sum))
return ERR_PTR(-EINVAL);
/* Calculate number of pages. */
first = (uaddr & PAGE_MASK) >> PAGE_SHIFT;
- last = ((uaddr + ulen - 1) & PAGE_MASK) >> PAGE_SHIFT;
+ last = ((sum - 1) & PAGE_MASK) >> PAGE_SHIFT;
npages = (last - first + 1);
locked = sev->pages_locked + npages;
--
2.34.1
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH 25/82] KVM: SVM: Refactor intentional wrap-around calculation
2024-01-23 0:27 ` [PATCH 25/82] KVM: SVM: " Kees Cook
@ 2024-01-24 16:15 ` Sean Christopherson
0 siblings, 0 replies; 8+ messages in thread
From: Sean Christopherson @ 2024-01-24 16:15 UTC (permalink / raw)
To: Kees Cook
Cc: linux-hardening, Paolo Bonzini, Thomas Gleixner, Ingo Molnar,
Borislav Petkov, Dave Hansen, x86, H. Peter Anvin, kvm,
Gustavo A. R. Silva, Bill Wendling, Justin Stitt, linux-kernel
On Mon, Jan 22, 2024, Kees Cook wrote:
> In an effort to separate intentional arithmetic wrap-around from
> unexpected wrap-around, we need to refactor places that depend on this
> kind of math. One of the most common code patterns of this is:
>
> VAR + value < VAR
>
> Notably, this is considered "undefined behavior" for signed and pointer
> types, which the kernel works around by using the -fno-strict-overflow
> option in the build[1] (which used to just be -fwrapv). Regardless, we
> want to get the kernel source to the position where we can meaningfully
> instrument arithmetic wrap-around conditions and catch them when they
> are unexpected, regardless of whether they are signed[2], unsigned[3],
> or pointer[4] types.
>
> Refactor open-coded unsigned wrap-around addition test to use
> check_add_overflow(), retaining the result for later usage (which removes
> the redundant open-coded addition). This paves the way to enabling the
> wrap-around sanitizers in the future.
IIUC, the plan is to get UBSAN to detect unexpected overflow, at which point an
explicit annotation will be needed to avoid false positives. If that's correct,
can you put something like that in these changelogs? Nothing in the changelog
actually says _why_ open coded wrap-around checks will be problematic.
> Link: https://git.kernel.org/linus/68df3755e383e6fecf2354a67b08f92f18536594 [1]
> Link: https://github.com/KSPP/linux/issues/26 [2]
> Link: https://github.com/KSPP/linux/issues/27 [3]
> Link: https://github.com/KSPP/linux/issues/344 [4]
> Cc: Sean Christopherson <seanjc@google.com>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Ingo Molnar <mingo@redhat.com>
> Cc: Borislav Petkov <bp@alien8.de>
> Cc: Dave Hansen <dave.hansen@linux.intel.com>
> Cc: x86@kernel.org
> Cc: "H. Peter Anvin" <hpa@zytor.com>
> Cc: kvm@vger.kernel.org
> Signed-off-by: Kees Cook <keescook@chromium.org>
> ---
> arch/x86/kvm/svm/sev.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
> index f760106c31f8..12a6a2b1ac81 100644
> --- a/arch/x86/kvm/svm/sev.c
> +++ b/arch/x86/kvm/svm/sev.c
> @@ -400,16 +400,17 @@ static struct page **sev_pin_memory(struct kvm *kvm, unsigned long uaddr,
> unsigned long locked, lock_limit;
> struct page **pages;
> unsigned long first, last;
> + unsigned long sum;
Similar to Marc's comments, I would much prefer to call this uaddr_last.
> int ret;
>
> lockdep_assert_held(&kvm->lock);
>
> - if (ulen == 0 || uaddr + ulen < uaddr)
> + if (ulen == 0 || check_add_overflow(uaddr, ulen, &sum))
> return ERR_PTR(-EINVAL);
>
> /* Calculate number of pages. */
> first = (uaddr & PAGE_MASK) >> PAGE_SHIFT;
> - last = ((uaddr + ulen - 1) & PAGE_MASK) >> PAGE_SHIFT;
> + last = ((sum - 1) & PAGE_MASK) >> PAGE_SHIFT;
> npages = (last - first + 1);
>
> locked = sev->pages_locked + npages;
> --
> 2.34.1
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 32/82] vringh: Refactor intentional wrap-around calculation
[not found] <20240122235208.work.748-kees@kernel.org>
2024-01-23 0:26 ` [PATCH 23/82] KVM: Refactor intentional wrap-around calculation Kees Cook
2024-01-23 0:27 ` [PATCH 25/82] KVM: SVM: " Kees Cook
@ 2024-01-23 0:27 ` Kees Cook
2024-01-26 19:31 ` Eugenio Perez Martin
2024-01-23 0:27 ` [PATCH 58/82] s390/mm: Refactor intentional wrap-around test Kees Cook
3 siblings, 1 reply; 8+ messages in thread
From: Kees Cook @ 2024-01-23 0:27 UTC (permalink / raw)
To: linux-hardening
Cc: Kees Cook, Michael S. Tsirkin, Jason Wang, kvm, virtualization,
netdev, Gustavo A. R. Silva, Bill Wendling, Justin Stitt,
linux-kernel
In an effort to separate intentional arithmetic wrap-around from
unexpected wrap-around, we need to refactor places that depend on this
kind of math. One of the most common code patterns of this is:
VAR + value < VAR
Notably, this is considered "undefined behavior" for signed and pointer
types, which the kernel works around by using the -fno-strict-overflow
option in the build[1] (which used to just be -fwrapv). Regardless, we
want to get the kernel source to the position where we can meaningfully
instrument arithmetic wrap-around conditions and catch them when they
are unexpected, regardless of whether they are signed[2], unsigned[3],
or pointer[4] types.
Refactor open-coded unsigned wrap-around addition test to use
check_add_overflow(), retaining the result for later usage (which removes
the redundant open-coded addition). This paves the way to enabling the
unsigned wrap-around sanitizer[2] in the future.
Link: https://git.kernel.org/linus/68df3755e383e6fecf2354a67b08f92f18536594 [1]
Link: https://github.com/KSPP/linux/issues/26 [2]
Link: https://github.com/KSPP/linux/issues/27 [3]
Link: https://github.com/KSPP/linux/issues/344 [4]
Cc: "Michael S. Tsirkin" <mst@redhat.com>
Cc: Jason Wang <jasowang@redhat.com>
Cc: kvm@vger.kernel.org
Cc: virtualization@lists.linux.dev
Cc: netdev@vger.kernel.org
Signed-off-by: Kees Cook <keescook@chromium.org>
---
drivers/vhost/vringh.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/drivers/vhost/vringh.c b/drivers/vhost/vringh.c
index 7b8fd977f71c..07442f0a52bd 100644
--- a/drivers/vhost/vringh.c
+++ b/drivers/vhost/vringh.c
@@ -145,6 +145,8 @@ static inline bool range_check(struct vringh *vrh, u64 addr, size_t *len,
bool (*getrange)(struct vringh *,
u64, struct vringh_range *))
{
+ u64 sum;
+
if (addr < range->start || addr > range->end_incl) {
if (!getrange(vrh, addr, range))
return false;
@@ -152,20 +154,20 @@ static inline bool range_check(struct vringh *vrh, u64 addr, size_t *len,
BUG_ON(addr < range->start || addr > range->end_incl);
/* To end of memory? */
- if (unlikely(addr + *len == 0)) {
+ if (unlikely(U64_MAX - addr == *len)) {
if (range->end_incl == -1ULL)
return true;
goto truncate;
}
/* Otherwise, don't wrap. */
- if (addr + *len < addr) {
+ if (check_add_overflow(addr, *len, &sum)) {
vringh_bad("Wrapping descriptor %zu@0x%llx",
*len, (unsigned long long)addr);
return false;
}
- if (unlikely(addr + *len - 1 > range->end_incl))
+ if (unlikely(sum - 1 > range->end_incl))
goto truncate;
return true;
--
2.34.1
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH 32/82] vringh: Refactor intentional wrap-around calculation
2024-01-23 0:27 ` [PATCH 32/82] vringh: " Kees Cook
@ 2024-01-26 19:31 ` Eugenio Perez Martin
2024-01-26 19:42 ` Kees Cook
0 siblings, 1 reply; 8+ messages in thread
From: Eugenio Perez Martin @ 2024-01-26 19:31 UTC (permalink / raw)
To: Kees Cook
Cc: linux-hardening, Michael S. Tsirkin, Jason Wang, kvm,
virtualization, netdev, Gustavo A. R. Silva, Bill Wendling,
Justin Stitt, linux-kernel
On Tue, Jan 23, 2024 at 2:42 AM Kees Cook <keescook@chromium.org> wrote:
>
> In an effort to separate intentional arithmetic wrap-around from
> unexpected wrap-around, we need to refactor places that depend on this
> kind of math. One of the most common code patterns of this is:
>
> VAR + value < VAR
>
> Notably, this is considered "undefined behavior" for signed and pointer
> types, which the kernel works around by using the -fno-strict-overflow
> option in the build[1] (which used to just be -fwrapv). Regardless, we
> want to get the kernel source to the position where we can meaningfully
> instrument arithmetic wrap-around conditions and catch them when they
> are unexpected, regardless of whether they are signed[2], unsigned[3],
> or pointer[4] types.
>
> Refactor open-coded unsigned wrap-around addition test to use
> check_add_overflow(), retaining the result for later usage (which removes
> the redundant open-coded addition). This paves the way to enabling the
> unsigned wrap-around sanitizer[2] in the future.
>
> Link: https://git.kernel.org/linus/68df3755e383e6fecf2354a67b08f92f18536594 [1]
> Link: https://github.com/KSPP/linux/issues/26 [2]
> Link: https://github.com/KSPP/linux/issues/27 [3]
> Link: https://github.com/KSPP/linux/issues/344 [4]
> Cc: "Michael S. Tsirkin" <mst@redhat.com>
> Cc: Jason Wang <jasowang@redhat.com>
> Cc: kvm@vger.kernel.org
> Cc: virtualization@lists.linux.dev
> Cc: netdev@vger.kernel.org
> Signed-off-by: Kees Cook <keescook@chromium.org>
> ---
> drivers/vhost/vringh.c | 8 +++++---
> 1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/vhost/vringh.c b/drivers/vhost/vringh.c
> index 7b8fd977f71c..07442f0a52bd 100644
> --- a/drivers/vhost/vringh.c
> +++ b/drivers/vhost/vringh.c
> @@ -145,6 +145,8 @@ static inline bool range_check(struct vringh *vrh, u64 addr, size_t *len,
> bool (*getrange)(struct vringh *,
> u64, struct vringh_range *))
> {
> + u64 sum;
I understand this is part of a bulk change so little time to think
about names :). But what about "end" or similar?
Either way,
Acked-by: Eugenio Pérez <eperezma@redhat.com>
> +
> if (addr < range->start || addr > range->end_incl) {
> if (!getrange(vrh, addr, range))
> return false;
> @@ -152,20 +154,20 @@ static inline bool range_check(struct vringh *vrh, u64 addr, size_t *len,
> BUG_ON(addr < range->start || addr > range->end_incl);
>
> /* To end of memory? */
> - if (unlikely(addr + *len == 0)) {
> + if (unlikely(U64_MAX - addr == *len)) {
> if (range->end_incl == -1ULL)
> return true;
> goto truncate;
> }
>
> /* Otherwise, don't wrap. */
> - if (addr + *len < addr) {
> + if (check_add_overflow(addr, *len, &sum)) {
> vringh_bad("Wrapping descriptor %zu@0x%llx",
> *len, (unsigned long long)addr);
> return false;
> }
>
> - if (unlikely(addr + *len - 1 > range->end_incl))
> + if (unlikely(sum - 1 > range->end_incl))
> goto truncate;
> return true;
>
> --
> 2.34.1
>
>
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH 32/82] vringh: Refactor intentional wrap-around calculation
2024-01-26 19:31 ` Eugenio Perez Martin
@ 2024-01-26 19:42 ` Kees Cook
0 siblings, 0 replies; 8+ messages in thread
From: Kees Cook @ 2024-01-26 19:42 UTC (permalink / raw)
To: Eugenio Perez Martin
Cc: linux-hardening, Michael S. Tsirkin, Jason Wang, kvm,
virtualization, netdev, Gustavo A. R. Silva, Bill Wendling,
Justin Stitt, linux-kernel
On Fri, Jan 26, 2024 at 08:31:04PM +0100, Eugenio Perez Martin wrote:
> On Tue, Jan 23, 2024 at 2:42 AM Kees Cook <keescook@chromium.org> wrote:
> >
> > In an effort to separate intentional arithmetic wrap-around from
> > unexpected wrap-around, we need to refactor places that depend on this
> > kind of math. One of the most common code patterns of this is:
> >
> > VAR + value < VAR
> >
> > Notably, this is considered "undefined behavior" for signed and pointer
> > types, which the kernel works around by using the -fno-strict-overflow
> > option in the build[1] (which used to just be -fwrapv). Regardless, we
> > want to get the kernel source to the position where we can meaningfully
> > instrument arithmetic wrap-around conditions and catch them when they
> > are unexpected, regardless of whether they are signed[2], unsigned[3],
> > or pointer[4] types.
> >
> > Refactor open-coded unsigned wrap-around addition test to use
> > check_add_overflow(), retaining the result for later usage (which removes
> > the redundant open-coded addition). This paves the way to enabling the
> > unsigned wrap-around sanitizer[2] in the future.
> >
> > Link: https://git.kernel.org/linus/68df3755e383e6fecf2354a67b08f92f18536594 [1]
> > Link: https://github.com/KSPP/linux/issues/26 [2]
> > Link: https://github.com/KSPP/linux/issues/27 [3]
> > Link: https://github.com/KSPP/linux/issues/344 [4]
> > Cc: "Michael S. Tsirkin" <mst@redhat.com>
> > Cc: Jason Wang <jasowang@redhat.com>
> > Cc: kvm@vger.kernel.org
> > Cc: virtualization@lists.linux.dev
> > Cc: netdev@vger.kernel.org
> > Signed-off-by: Kees Cook <keescook@chromium.org>
> > ---
> > drivers/vhost/vringh.c | 8 +++++---
> > 1 file changed, 5 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/vhost/vringh.c b/drivers/vhost/vringh.c
> > index 7b8fd977f71c..07442f0a52bd 100644
> > --- a/drivers/vhost/vringh.c
> > +++ b/drivers/vhost/vringh.c
> > @@ -145,6 +145,8 @@ static inline bool range_check(struct vringh *vrh, u64 addr, size_t *len,
> > bool (*getrange)(struct vringh *,
> > u64, struct vringh_range *))
> > {
> > + u64 sum;
>
> I understand this is part of a bulk change so little time to think
> about names :). But what about "end" or similar?
>
> Either way,
> Acked-by: Eugenio Pérez <eperezma@redhat.com>
Thanks! Yeah, you are not alone in suggesting "end" in a several of
these patches. :)
-Kees
--
Kees Cook
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 58/82] s390/mm: Refactor intentional wrap-around test
[not found] <20240122235208.work.748-kees@kernel.org>
` (2 preceding siblings ...)
2024-01-23 0:27 ` [PATCH 32/82] vringh: " Kees Cook
@ 2024-01-23 0:27 ` Kees Cook
3 siblings, 0 replies; 8+ messages in thread
From: Kees Cook @ 2024-01-23 0:27 UTC (permalink / raw)
To: linux-hardening
Cc: Kees Cook, Christian Borntraeger, Janosch Frank, Claudio Imbrenda,
David Hildenbrand, Alexander Gordeev, Gerald Schaefer,
Heiko Carstens, Vasily Gorbik, Sven Schnelle, kvm, linux-s390,
Gustavo A. R. Silva, Bill Wendling, Justin Stitt, linux-kernel
In an effort to separate intentional arithmetic wrap-around from
unexpected wrap-around, we need to refactor places that depend on this
kind of math. One of the most common code patterns of this is:
VAR + value < VAR
Notably, this is considered "undefined behavior" for signed and pointer
types, which the kernel works around by using the -fno-strict-overflow
option in the build[1] (which used to just be -fwrapv). Regardless, we
want to get the kernel source to the position where we can meaningfully
instrument arithmetic wrap-around conditions and catch them when they
are unexpected, regardless of whether they are signed[2], unsigned[3],
or pointer[4] types.
Refactor open-coded wrap-around addition test to use add_would_overflow().
This paves the way to enabling the wrap-around sanitizers in the future.
Link: https://git.kernel.org/linus/68df3755e383e6fecf2354a67b08f92f18536594 [1]
Link: https://github.com/KSPP/linux/issues/26 [2]
Link: https://github.com/KSPP/linux/issues/27 [3]
Link: https://github.com/KSPP/linux/issues/344 [4]
Cc: Christian Borntraeger <borntraeger@linux.ibm.com>
Cc: Janosch Frank <frankja@linux.ibm.com>
Cc: Claudio Imbrenda <imbrenda@linux.ibm.com>
Cc: David Hildenbrand <david@redhat.com>
Cc: Alexander Gordeev <agordeev@linux.ibm.com>
Cc: Gerald Schaefer <gerald.schaefer@linux.ibm.com>
Cc: Heiko Carstens <hca@linux.ibm.com>
Cc: Vasily Gorbik <gor@linux.ibm.com>
Cc: Sven Schnelle <svens@linux.ibm.com>
Cc: kvm@vger.kernel.org
Cc: linux-s390@vger.kernel.org
Signed-off-by: Kees Cook <keescook@chromium.org>
---
arch/s390/mm/gmap.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/s390/mm/gmap.c b/arch/s390/mm/gmap.c
index 6f96b5a71c63..977b61ab59f2 100644
--- a/arch/s390/mm/gmap.c
+++ b/arch/s390/mm/gmap.c
@@ -411,7 +411,7 @@ int gmap_unmap_segment(struct gmap *gmap, unsigned long to, unsigned long len)
BUG_ON(gmap_is_shadow(gmap));
if ((to | len) & (PMD_SIZE - 1))
return -EINVAL;
- if (len == 0 || to + len < to)
+ if (len == 0 || add_would_overflow(to, len))
return -EINVAL;
flush = 0;
@@ -443,7 +443,7 @@ int gmap_map_segment(struct gmap *gmap, unsigned long from,
BUG_ON(gmap_is_shadow(gmap));
if ((from | to | len) & (PMD_SIZE - 1))
return -EINVAL;
- if (len == 0 || from + len < from || to + len < to ||
+ if (len == 0 || add_would_overflow(from, len) || add_would_overflow(to, len) ||
from + len - 1 > TASK_SIZE_MAX || to + len - 1 > gmap->asce_end)
return -EINVAL;
--
2.34.1
^ permalink raw reply related [flat|nested] 8+ messages in thread