Re: [PATCH] fs: fcntl_setlease defies lease_init assumptions

From: Linus Torvalds
Date: Sun May 07 2006 - 23:34:06 EST




On Mon, 8 May 2006, Daniel Hokka Zakrisson wrote:
>
> fcntl_setlease uses a struct file_lock on the stack to find the
> file_lock it's actually looking for. The problem with this approach is
> that lease_init will attempt to free the file_lock if the arg argument
> is invalid, causing kmem_cache_free to be called with the address of the
> on-stack file_lock.

Ok, I was actually really surprised that we'd ever allow a non-slab page
to be free'd as a slab or kmalloc allocation, without screaming very
loudly indeed. That implies a lack of some pretty fundamental sanity
checking by default in the slab layer (I suspect slab debugging turns it
on, but even without it, that's just nasty).

Can you see if this trivial patch at least causes a honking huge
"kernel BUG" message to be triggered quickly?

Trond wrote an alternate patch for the actual problem which I sent
separately, but it would probably be good to have more safety in the slab
layer by default regardless.

Linus

---
diff --git a/mm/slab.c b/mm/slab.c
index c32af7e..279ca59 100644
--- a/mm/slab.c
+++ b/mm/slab.c
@@ -597,6 +597,7 @@ static inline struct kmem_cache *page_ge
{
if (unlikely(PageCompound(page)))
page = (struct page *)page_private(page);
+ BUG_ON(!PageSlab(page));
return (struct kmem_cache *)page->lru.next;
}

@@ -609,6 +610,7 @@ static inline struct slab *page_get_slab
{
if (unlikely(PageCompound(page)))
page = (struct page *)page_private(page);
+ BUG_ON(!PageSlab(page));
return (struct slab *)page->lru.prev;
}

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/