Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
New issues: - [Medium] The overlap check incorrectly rejects valid user VA mappings that end exactly at the upper boundary of the user VA space, effectively reducing the usable user VA space by one page. Pre-existing issues: - [High] The bounds check for the kernel-reserved VA range lacks integer overflow protection, allowing an attacker to bypass it and trigger undefined behavior and hardware register corruption. -- commit 544d7f2ab2018dcefc4327336a2da6f6a37e4d4f Author: Adrián Larumbe <[email protected]> drm/panthor: Add vm_bind region with kbo range overlap check This patch adds a check to ensure that user VM mappings do not overlap with the kernel BO address space carve-out. It sets a boundary for the user VA range at VM creation and enforces it during the vm_bind ioctl. > diff --git a/drivers/gpu/drm/panthor/panthor_mmu.c > b/drivers/gpu/drm/panthor/panthor_mmu.c > index 31cc57029c12c..a8de4fe6b231b 100644 > --- a/drivers/gpu/drm/panthor/panthor_mmu.c > +++ b/drivers/gpu/drm/panthor/panthor_mmu.c [ ... ] > @@ -2893,6 +2896,8 @@ panthor_vm_create(struct panthor_device *ptdev, bool > for_mcu, > va_range = full_va_range; > } > > + vm->user_va_end = kernel_va_start - 1; > + > mutex_init(&vm->mm_lock); [ ... ] > @@ -2981,6 +2986,10 @@ panthor_vm_bind_prepare_op_ctx(struct drm_file *file, > if (!IS_ALIGNED(op->va | op->size | op->bo_offset, vm_pgsz)) > return -EINVAL; > > + /* We don't allow mappings that overlap with kbo's reserved range */ > + if (op->va + op->size > vm->user_va_end) > + return -EINVAL; [Severity: Medium] Since op->va + op->size represents the exclusive end of the mapping, and vm->user_va_end is the inclusive upper bound (kernel_va_start - 1), does this reject valid mappings that end exactly at the boundary? If a mapping ends exactly at kernel_va_start, op->va + op->size will be equal to kernel_va_start. The condition kernel_va_start > kernel_va_start - 1 will evaluate to true and return -EINVAL, effectively reducing the usable user VA space by one page. [Severity: High] This isn't a bug introduced by this patch, but is it possible for this bounds check to be bypassed due to integer overflow? If an unprivileged user calls the vm_bind ioctl with a sparse mapping and parameters that overflow when added (for example, va = 0x200000 and size = 0xffffffffffe00000), the wrapped va + size evaluates to 0. This successfully bypasses the new check. Later in the execution path, this bypass triggers undefined behavior when pack_region_range() calculates region_end = start + size, which wraps to 0. This makes region_end - 1 equal to -1 (0xffffffffffffffff). The call to fls64(*region_start ^ (region_end - 1)) then evaluates to 64, leading to region_width = 63 and a shift by 64 (1ull << 64), which is undefined behavior in C and corrupts the lockaddr hardware register. > + > switch (op->flags & DRM_PANTHOR_VM_BIND_OP_TYPE_MASK) { > case DRM_PANTHOR_VM_BIND_OP_TYPE_MAP: > if (!(op->flags & DRM_PANTHOR_VM_BIND_OP_MAP_SPARSE)) { -- Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1
