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
