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
