Re: [PATCH RFC 2/5] mm/slab, kfence: support kfence objects in kfree_nolock()

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

New issues:
- [High] Double-freeing a KFENCE object via kfree_nolock() causes critical list corruption (llist cycle or rcu_head overwrite), turning a detectable bug into an unrecoverable crash.
- [Medium] kfree_nolock() silently bypasses KFENCE's invalid free detection for offset pointers, stripping the invalid offsets during deferral.

Pre-existing issues:
- [High] KFENCE fails to safely catch double-frees for SLAB_TYPESAFE_BY_RCU caches, corrupting RCU and freelists.
--

--- Patch [2]: [PATCH RFC 2/5] mm/slab, kfence: support kfence objects in kfree_nolock() ---
commit c37e10ff24a4e2a898c52d16f3e89a13a643b486
Author: Vlastimil Babka (SUSE) <[email protected]>

mm/slab, kfence: support kfence objects in kfree_nolock()

This commit adds support for freeing KFENCE objects via kfree_nolock(). It
defers the freeing to an irq_work context using an llist node, reusing the
rcu_head space in kfence_metadata.

> diff --git a/mm/kfence/core.c b/mm/kfence/core.c
> index 6577bd76954eb..42519d24687ff 100644
> --- a/mm/kfence/core.c
> +++ b/mm/kfence/core.c
> @@ -1271,6 +1271,20 @@ void __kfence_free(void *addr)

[Severity: High]
This is a pre-existing issue, but does __kfence_free() safely handle double
frees for SLAB_TYPESAFE_BY_RCU caches?

It appears that if an already-freed object is passed, it blindly calls
metadata_update_state() and call_rcu(&meta->rcu_head) without checking if
the object is already freed.

If the object is already in the RCU queue, this executes call_rcu() twice on
the same rcu_head, corrupting the RCU list. If it was already fully freed,
the state is forced back to RCU_FREEING, causing the subsequent callback to
bypass the double-free check and corrupt the freelist.

>  	}
>  }
>  
> +struct llist_node *kfence_obj_to_llnode(void *addr)
> +{
> +	struct kfence_metadata *meta = addr_to_metadata((unsigned long)addr);
> +
> +	return &meta->llnode;
> +}
> +
> +void *kfence_llnode_to_obj(struct llist_node *llnode)
> +{
> +	struct kfence_metadata *meta = container_of(llnode, struct kfence_metadata, llnode);
> +
> +	return (void *)meta->addr;
> +}

[Severity: Medium]
Does this sequence bypass KFENCE's invalid free detection?

When kfree_nolock(obj) receives an invalid KFENCE address, such as an offset
pointer or redzone address, defer_free_kfence() calls kfence_obj_to_llnode()
which drops the original invalid offset.

Later, kfence_llnode_to_obj() reconstructs the pointer strictly as meta->addr,
which is the valid base address.

This passes the reconstructed valid address to __kfence_free(), masking the
original invalid pointer and preventing KFENCE from reporting the bug.

[ ... ]

> diff --git a/mm/kfence/kfence.h b/mm/kfence/kfence.h
> index 1f618f9b0d12d..0fca1dc2c7941 100644
> --- a/mm/kfence/kfence.h
> +++ b/mm/kfence/kfence.h
> @@ -58,7 +58,10 @@ struct kfence_track {
>  /* KFENCE metadata per guarded allocation. */
>  struct kfence_metadata {
>  	struct list_head list __guarded_by(&kfence_freelist_lock);	/* Freelist node. */
> -	struct rcu_head rcu_head;	/* For delayed freeing. */
> +	union {
> +		struct rcu_head rcu_head;	/* For delayed freeing. */
> +		struct llist_node llnode;	/* For kfree_nolock(). */
> +	};

[Severity: High]
Can aliasing rcu_head and llnode risk list corruption on double frees?

If a KFENCE object is double-freed via kfree_nolock() before the pending
irq_work runs, llist_add() is called twice on the same llnode. This would
create a circular linked list and an infinite loop in IRQ context.

Additionally, for SLAB_TYPESAFE_BY_RCU caches, a double free via
kfree_nolock() during the RCU grace period will locklessly overwrite
meta->rcu_head.next via llist_add(), corrupting the active RCU callback list.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=2