[permalink] [raw]

Subject: [PATCH 08/82] iov_iter: Avoid wrap-around instrumentation in copy_compat_iovec_from_user

The loop counter "i" in copy_compat_iovec_from_user() is an int, but
because the nr_segs argument is unsigned long, the signed overflow
sanitizer got worried "i" could wrap around. Instead of making "i" an
unsigned long (which may enlarge the type size), switch both nr_segs
and i to u32. There is no truncation with nr_segs since its is never
larger than UIO_MAXIOV anyway. This keeps sanitizer instrumentation[1]
out of a UACCESS path:

vmlinux.o: warning: objtool: copy_compat_iovec_from_user+0xa9: call to __ubsan_handle_add_overflow() with UACCESS enabled

Link: https://github.com/KSPP/linux/issues/26 [1]
Cc: Alexander Viro <[email protected]>
Cc: Andrew Morton <[email protected]>
Signed-off-by: Kees Cook <[email protected]>
---
lib/iov_iter.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/lib/iov_iter.c b/lib/iov_iter.c
index e0aa6b440ca5..d797a43dca91 100644
--- a/lib/iov_iter.c
+++ b/lib/iov_iter.c
@@ -1166,11 +1166,12 @@ const void *dup_iter(struct iov_iter *new, struct iov_iter *old, gfp_t flags)
EXPORT_SYMBOL(dup_iter);

static __noclone int copy_compat_iovec_from_user(struct iovec *iov,
- const struct iovec __user *uvec, unsigned long nr_segs)
+ const struct iovec __user *uvec, u32 nr_segs)
{
const struct compat_iovec __user *uiov =
(const struct compat_iovec __user *)uvec;
- int ret = -EFAULT, i;
+ int ret = -EFAULT;
+ u32 i;

if (!user_access_begin(uiov, nr_segs * sizeof(*uiov)))
return -EFAULT;
--
2.34.1

2024-01-23 01:18:01

by Kees Cook

[permalink] [raw]

Subject: [PATCH 17/82] drm/vc4: Refactor intentional wrap-around calculation

2024-01-23 01:18:13

by Kees Cook

[permalink] [raw]

Subject: [PATCH 02/82] overflow: Introduce add_would_overflow()

For instances where only the overflow needs to be checked (and the sum
isn't used), provide the new helper add_would_overflow(), which is
a wrapper for check_add_overflow().

Cc: "Gustavo A. R. Silva" <[email protected]>
Cc: [email protected]
Signed-off-by: Kees Cook <[email protected]>
---
include/linux/overflow.h | 16 ++++++++++++++++
1 file changed, 16 insertions(+)

diff --git a/include/linux/overflow.h b/include/linux/overflow.h
index 099f2e559aa8..ac088f73e0fd 100644
--- a/include/linux/overflow.h
+++ b/include/linux/overflow.h
@@ -108,6 +108,22 @@ static inline bool __must_check __must_check_overflow(bool overflow)
__builtin_add_overflow(__filter_integral(a), b, \
__filter_ptrint(d))))

+/**
+ * add_would_overflow() - Check if an addition would overflow
+ * @a: first addend
+ * @b: second addend
+ *
+ * Returns true if the sum would overflow.
+ *
+ * To keep a copy of the sum when the addition doesn't overflow, use
+ * check_add_overflow() instead.
+ */
+#define add_would_overflow(a, b) \
+ __must_check_overflow(({ \
+ size_t __result; \
+ check_add_overflow(a, b, &__result);\
+ }))
+
/**
* check_sub_overflow() - Calculate subtraction with overflow checking
* @a: minuend; value to subtract from
--
2.34.1

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: Bjorn Andersson <[email protected]>
Cc: Mathieu Poirier <[email protected]>
Cc: [email protected]
Signed-off-by: Kees Cook <[email protected]>
---
drivers/remoteproc/pru_rproc.c | 2 +-
drivers/remoteproc/remoteproc_elf_loader.c | 2 +-
drivers/remoteproc/remoteproc_virtio.c | 4 ++--
3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/remoteproc/pru_rproc.c b/drivers/remoteproc/pru_rproc.c
index 327f0c7ee3d6..834249ee3dd3 100644
--- a/drivers/remoteproc/pru_rproc.c
+++ b/drivers/remoteproc/pru_rproc.c
@@ -893,7 +893,7 @@ pru_rproc_find_interrupt_map(struct device *dev, const struct firmware *fw)
continue;

/* make sure we have the entire irq map */
- if (offset + size > fw->size || offset + size < size) {
+ if (offset + size > fw->size || add_would_overflow(size, offset)) {
dev_err(dev, ".pru_irq_map section truncated\n");
return ERR_PTR(-EINVAL);
}
diff --git a/drivers/remoteproc/remoteproc_elf_loader.c b/drivers/remoteproc/remoteproc_elf_loader.c
index 94177e416047..b9231cf46d68 100644
--- a/drivers/remoteproc/remoteproc_elf_loader.c
+++ b/drivers/remoteproc/remoteproc_elf_loader.c
@@ -278,7 +278,7 @@ find_table(struct device *dev, const struct firmware *fw)
table = (struct resource_table *)(elf_data + offset);

/* make sure we have the entire table */
- if (offset + size > fw_size || offset + size < size) {
+ if (offset + size > fw_size || add_would_overflow(size, offset)) {
dev_err(dev, "resource table truncated\n");
return NULL;
}
diff --git a/drivers/remoteproc/remoteproc_virtio.c b/drivers/remoteproc/remoteproc_virtio.c
index 83d76915a6ad..58742c666e35 100644
--- a/drivers/remoteproc/remoteproc_virtio.c
+++ b/drivers/remoteproc/remoteproc_virtio.c
@@ -298,7 +298,7 @@ static void rproc_virtio_get(struct virtio_device *vdev, unsigned int offset,
rsc = (void *)rvdev->rproc->table_ptr + rvdev->rsc_offset;
cfg = &rsc->vring[rsc->num_of_vrings];

- if (offset + len > rsc->config_len || offset + len < len) {
+ if (offset + len > rsc->config_len || add_would_overflow(len, offset)) {
dev_err(&vdev->dev, "rproc_virtio_get: access out of bounds\n");
return;
}
@@ -316,7 +316,7 @@ static void rproc_virtio_set(struct virtio_device *vdev, unsigned int offset,
rsc = (void *)rvdev->rproc->table_ptr + rvdev->rsc_offset;
cfg = &rsc->vring[rsc->num_of_vrings];

- if (offset + len > rsc->config_len || offset + len < len) {
+ if (offset + len > rsc->config_len || add_would_overflow(len, offset)) {
dev_err(&vdev->dev, "rproc_virtio_set: access out of bounds\n");
return;
}
--
2.34.1

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: Andrew Morton <[email protected]>
Cc: Shuah Khan <[email protected]>
Cc: [email protected]
Cc: [email protected]
Signed-off-by: Kees Cook <[email protected]>
---
mm/memory.c | 4 ++--
mm/mmap.c | 2 +-
mm/mremap.c | 2 +-
mm/nommu.c | 4 ++--
mm/util.c | 2 +-
5 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/mm/memory.c b/mm/memory.c
index 7e1f4849463a..d47acdff7af3 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -2559,7 +2559,7 @@ int vm_iomap_memory(struct vm_area_struct *vma, phys_addr_t start, unsigned long
unsigned long vm_len, pfn, pages;

/* Check that the physical memory area passed in looks valid */
- if (start + len < start)
+ if (add_would_overflow(start, len))
return -EINVAL;
/*
* You *really* shouldn't map things that aren't page-aligned,
@@ -2569,7 +2569,7 @@ int vm_iomap_memory(struct vm_area_struct *vma, phys_addr_t start, unsigned long
len += start & ~PAGE_MASK;
pfn = start >> PAGE_SHIFT;
pages = (len + ~PAGE_MASK) >> PAGE_SHIFT;
- if (pfn + pages < pfn)
+ if (add_would_overflow(pfn, pages))
return -EINVAL;

/* We start the mapping 'vm_pgoff' pages into the area */
diff --git a/mm/mmap.c b/mm/mmap.c
index b78e83d351d2..16501fcaf511 100644
--- a/mm/mmap.c
+++ b/mm/mmap.c
@@ -3023,7 +3023,7 @@ SYSCALL_DEFINE5(remap_file_pages, unsigned long, start, unsigned long, size,
return ret;

/* Does pgoff wrap? */
- if (pgoff + (size >> PAGE_SHIFT) < pgoff)
+ if (add_would_overflow(pgoff, (size >> PAGE_SHIFT)))
return ret;

if (mmap_write_lock_killable(mm))
diff --git a/mm/mremap.c b/mm/mremap.c
index 38d98465f3d8..efa27019a05d 100644
--- a/mm/mremap.c
+++ b/mm/mremap.c
@@ -848,7 +848,7 @@ static struct vm_area_struct *vma_to_resize(unsigned long addr,
/* Need to be careful about a growing mapping */
pgoff = (addr - vma->vm_start) >> PAGE_SHIFT;
pgoff += vma->vm_pgoff;
- if (pgoff + (new_len >> PAGE_SHIFT) < pgoff)
+ if (add_would_overflow(pgoff, (new_len >> PAGE_SHIFT)))
return ERR_PTR(-EINVAL);

if (vma->vm_flags & (VM_DONTEXPAND | VM_PFNMAP))
diff --git a/mm/nommu.c b/mm/nommu.c
index b6dc558d3144..299bcfe19eed 100644
--- a/mm/nommu.c
+++ b/mm/nommu.c
@@ -202,7 +202,7 @@ EXPORT_SYMBOL(vmalloc_to_pfn);
long vread_iter(struct iov_iter *iter, const char *addr, size_t count)
{
/* Don't allow overflow */
- if ((unsigned long) addr + count < count)
+ if (add_would_overflow(count, (unsigned long)addr))
count = -(unsigned long) addr;

return copy_to_iter(addr, count, iter);
@@ -1705,7 +1705,7 @@ int access_process_vm(struct task_struct *tsk, unsigned long addr, void *buf, in
{
struct mm_struct *mm;

- if (addr + len < addr)
+ if (add_would_overflow(addr, len))
return 0;

mm = get_task_mm(tsk);
diff --git a/mm/util.c b/mm/util.c
index 5a6a9802583b..e6beeb23b48b 100644
--- a/mm/util.c
+++ b/mm/util.c
@@ -567,7 +567,7 @@ unsigned long vm_mmap(struct file *file, unsigned long addr,
unsigned long len, unsigned long prot,
unsigned long flag, unsigned long offset)
{
- if (unlikely(offset + PAGE_ALIGN(len) < offset))
+ if (unlikely(add_would_overflow(offset, PAGE_ALIGN(len))))
return -EINVAL;
if (unlikely(offset_in_page(offset)))
return -EINVAL;
--
2.34.1

2024-01-23 01:31:29

by Kees Cook

[permalink] [raw]

Subject: [PATCH 50/82] drm/vc4: Refactor intentional wrap-around test

2024-01-23 01:31:36

by Kees Cook

[permalink] [raw]

Subject: [PATCH 65/82] nios2: Refactor intentional wrap-around test

2024-01-23 01:31:37

by Kees Cook

[permalink] [raw]

Subject: [PATCH 15/82] dma-buf: Refactor intentional wrap-around calculation

2024-01-23 01:31:49

by Kees Cook

[permalink] [raw]

2024-01-23 01:33:17

by Kees Cook

[permalink] [raw]

Subject: [PATCH 60/82] powerpc: Refactor intentional wrap-around test

2024-01-23 01:33:17

by Kees Cook

[permalink] [raw]

Subject: [PATCH 72/82] scsi: sd_zbc: Refactor intentional wrap-around test

2024-01-23 01:33:46

by Kees Cook

[permalink] [raw]

Subject: [PATCH 24/82] KVM: arm64: vgic: Refactor intentional wrap-around calculation

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: Heiko Carstens <[email protected]>
Cc: Vasily Gorbik <[email protected]>
Cc: Alexander Gordeev <[email protected]>
Cc: Christian Borntraeger <[email protected]>
Cc: Sven Schnelle <[email protected]>
Cc: Nico Boehr <[email protected]>
Cc: Philipp Rudo <[email protected]>
Cc: Baoquan He <[email protected]>
Cc: Tao Liu <[email protected]>
Cc: Alexander Egorenkov <[email protected]>
Cc: [email protected]
Signed-off-by: Kees Cook <[email protected]>
---
arch/s390/include/asm/stacktrace.h | 6 ++++--
arch/s390/kernel/machine_kexec_file.c | 5 +++--
2 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/arch/s390/include/asm/stacktrace.h b/arch/s390/include/asm/stacktrace.h
index 31ec4f545e03..3ce08d32a8ad 100644
--- a/arch/s390/include/asm/stacktrace.h
+++ b/arch/s390/include/asm/stacktrace.h
@@ -34,11 +34,13 @@ int get_stack_info(unsigned long sp, struct task_struct *task,
static inline bool on_stack(struct stack_info *info,
unsigned long addr, size_t len)
{
+ unsigned long sum;
+
if (info->type == STACK_TYPE_UNKNOWN)
return false;
- if (addr + len < addr)
+ if (check_add_overflow(addr, len, &sum))
return false;
- return addr >= info->begin && addr + len <= info->end;
+ return addr >= info->begin && sum <= info->end;
}

/*
diff --git a/arch/s390/kernel/machine_kexec_file.c b/arch/s390/kernel/machine_kexec_file.c
index 8d207b82d9fe..e5e925423061 100644
--- a/arch/s390/kernel/machine_kexec_file.c
+++ b/arch/s390/kernel/machine_kexec_file.c
@@ -238,6 +238,7 @@ void *kexec_file_add_components(struct kimage *image,
unsigned long max_command_line_size = LEGACY_COMMAND_LINE_SIZE;
struct s390_load_data data = {0};
unsigned long minsize;
+ unsigned long sum;
int ret;

data.report = ipl_report_init(&ipl_block);
@@ -256,10 +257,10 @@ void *kexec_file_add_components(struct kimage *image,
if (data.parm->max_command_line_size)
max_command_line_size = data.parm->max_command_line_size;

- if (minsize + max_command_line_size < minsize)
+ if (check_add_overflow(minsize, max_command_line_size, &sum))
goto out;

- if (image->kernel_buf_len < minsize + max_command_line_size)
+ if (image->kernel_buf_len < sum)
goto out;

if (image->cmdline_buf_len >= max_command_line_size)
--
2.34.1

2024-01-23 01:59:34

by Kees Cook

[permalink] [raw]

Kees Cook <[email protected]> writes:
> 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: Michael Ellerman <[email protected]>
> Cc: Nicholas Piggin <[email protected]>
> Cc: Christophe Leroy <[email protected]>
> Cc: "Aneesh Kumar K.V" <[email protected]>
> Cc: "Naveen N. Rao" <[email protected]>
> Cc: Mahesh Salgaonkar <[email protected]>
> Cc: Vasant Hegde <[email protected]>
> Cc: dingsenjie <[email protected]>
> Cc: [email protected]
> Cc: Aneesh Kumar K.V <[email protected]>
> Cc: Naveen N. Rao <[email protected]>
> Signed-off-by: Kees Cook <[email protected]>
> ---
> arch/powerpc/platforms/powernv/opal-prd.c | 2 +-
> arch/powerpc/xmon/xmon.c | 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)

Acked-by: Michael Ellerman <[email protected]> (powerpc)

cheers