Re: Possible video driver issue after main-n275966-d2a55e6a9348 -> main-n275975-5963423232e8

From: Mark Johnston <markj_at_freebsd.org>
Date: Fri, 21 Mar 2025 09:34:16 UTC
On Fri, Mar 21, 2025 at 01:56:01AM -0700, Gleb Smirnoff wrote:
> On Thu, Mar 20, 2025 at 07:52:19PM +0000, Bjoern A. Zeeb wrote:
> B> He's hitting a ... somewhere in i915kms.ko (here's the two instances I
> B> have):
> B> REDZONE: Buffer underflow detected. 16 bytes corrupted before 0xfffffe089bc65000 (262148 bytes allocated).
> B> REDZONE: Buffer underflow detected. 16 bytes corrupted before 0xfffffe08a7e70000 (262148 bytes allocated).
> 
> I looked a bit into the problem and it actually seems very trivial to me.
> Please re-check my observations.
> 
> A contigmalloc(9) allocation doesn't get redzone protection, see kern_malloc.c.
> But free(9) always does contigmalloc check.  This makes deprecation of
> contigfree(9) incompatible with redzone(9). And looks like
> 19df0c5abcb9d4e951e610b6de98d4d8a00bd5f9 is our first bump into this sad fact.

Can we not just add redzone padding to contigmalloc() allocations?
Compile-tested patch below:

diff --git a/sys/kern/kern_malloc.c b/sys/kern/kern_malloc.c
index b1347b15e651..0b76e633b04a 100644
--- a/sys/kern/kern_malloc.c
+++ b/sys/kern/kern_malloc.c
@@ -477,11 +477,18 @@ contigmalloc_size(uma_slab_t slab)
 }
 
 void *
-contigmalloc(unsigned long size, struct malloc_type *type, int flags,
+contigmalloc(unsigned long osize, struct malloc_type *type, int flags,
     vm_paddr_t low, vm_paddr_t high, unsigned long alignment,
     vm_paddr_t boundary)
 {
 	void *ret;
+	unsigned long size;
+
+#ifdef DEBUG_REDZONE
+	size = redzone_size_ntor(osize);
+#else
+	size = osize;
+#endif
 
 	ret = (void *)kmem_alloc_contig(size, flags, low, high, alignment,
 	    boundary, VM_MEMATTR_DEFAULT);
@@ -489,16 +496,26 @@ contigmalloc(unsigned long size, struct malloc_type *type, int flags,
 		/* Use low bits unused for slab pointers. */
 		vsetzoneslab((uintptr_t)ret, NULL, CONTIG_MALLOC_SLAB(size));
 		malloc_type_allocated(type, round_page(size));
+#ifdef DEBUG_REDZONE
+		ret = redzone_setup(ret, osize);
+#endif
 	}
 	return (ret);
 }
 
 void *
-contigmalloc_domainset(unsigned long size, struct malloc_type *type,
+contigmalloc_domainset(unsigned long osize, struct malloc_type *type,
     struct domainset *ds, int flags, vm_paddr_t low, vm_paddr_t high,
     unsigned long alignment, vm_paddr_t boundary)
 {
 	void *ret;
+	unsigned long size;
+
+#ifdef DEBUG_REDZONE
+	size = redzone_size_ntor(osize);
+#else
+	size = osize;
+#endif
 
 	ret = (void *)kmem_alloc_contig_domainset(ds, size, flags, low, high,
 	    alignment, boundary, VM_MEMATTR_DEFAULT);
@@ -506,6 +523,9 @@ contigmalloc_domainset(unsigned long size, struct malloc_type *type,
 		/* Use low bits unused for slab pointers. */
 		vsetzoneslab((uintptr_t)ret, NULL, CONTIG_MALLOC_SLAB(size));
 		malloc_type_allocated(type, round_page(size));
+#ifdef DEBUG_REDZONE
+		ret = redzone_setup(ret, osize);
+#endif
 	}
 	return (ret);
 }