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

On Fri, 21 Mar 2025, Mark Johnston wrote:

> On Fri, Mar 21, 2025 at 02:50:58AM -0700, Gleb Smirnoff wrote:
>> On Fri, Mar 21, 2025 at 05:34:16AM -0400, Mark Johnston wrote:
>> M> On Fri, Mar 21, 2025 at 01:56:01AM -0700, Gleb Smirnoff wrote:
>> M> > On Thu, Mar 20, 2025 at 07:52:19PM +0000, Bjoern A. Zeeb wrote:
>> M> > B> He's hitting a ... somewhere in i915kms.ko (here's the two instances I
>> M> > B> have):
>> M> > B> REDZONE: Buffer underflow detected. 16 bytes corrupted before 0xfffffe089bc65000 (262148 bytes allocated).
>> M> > B> REDZONE: Buffer underflow detected. 16 bytes corrupted before 0xfffffe08a7e70000 (262148 bytes allocated).
>> M> >
>> M> > I looked a bit into the problem and it actually seems very trivial to me.
>> M> > Please re-check my observations.
>> M> >
>> M> > A contigmalloc(9) allocation doesn't get redzone protection, see kern_malloc.c.
>> M> > But free(9) always does contigmalloc check.  This makes deprecation of
>> M> > contigfree(9) incompatible with redzone(9). And looks like
>> M> > 19df0c5abcb9d4e951e610b6de98d4d8a00bd5f9 is our first bump into this sad fact.
>> M>
>> M> Can we not just add redzone padding to contigmalloc() allocations?
>>
>> I was about to suggest that, but was afraid it is too naive :) But
>> if that works, why not?  We probably should document that for
>> contigmalloc() the redzone would provide protection of the virtual
>> space, but not the physical.
>
> I'm not sure what you mean by this?  As implemented, the patch
> effectively rounds up the allocation size, so the redzone will also be
> physically contiguous.  Though, I see now that this will result in an
> non-page-aligned allocation, which callers of contigmalloc() might
> not tolerate...
>
> Actually, for malloc_large() and contigmalloc() allocations it's
> probably a bit easier to just provide guard pages around the
> allocation, like we do for kernel stacks.  That is, if the caller asks
> for N pages, then allocate N+2 pages of virtual address space and back
> pages [1, N] with physical memory.  Then any overflow will trap at the
> site of the overflow, which is probably more useful than what
> redzone(9).  Actually, KASAN provides the same checking, but currently
> we don't pad allocations when KASAN is enabled.

I like the idea given contigmalloc will always round up to PAGE_SIZE
anyway.  Problem with contigmalloc is that you have to meet the
alignment requirement, etc. on [1,N] then. Does that make it more
tricky?

/bz

-- 
Bjoern A. Zeeb                                                     r15:7