Messages in this thread |  | | Date | Mon, 6 Apr 2026 21:28:29 +0100 | | From | Al Viro <> | | Subject | Re: [RFC] [PATCH] Fix warning at fs/dcache.c:430 dentry_free |
| |
On Mon, Apr 06, 2026 at 09:07:33PM +0100, Al Viro wrote: > On Mon, Apr 06, 2026 at 09:52:16PM +0200, Helge Deller wrote: > > The debian buildd servers for the parisc architecture crash reproduceably when > > building the webkit2gtk debian package, shortly after having shown the warning > > below. > > > > This patch keeps the lock of the dentry up until when the dentry is given back > > to the cache and after having freed the "external dentry name". > > > > I'm not sure if this patch is really correct, but it seems to have fixed the > > problem, although more testing is needed. > > Hard NAK. You are turning every place that grabs ->d_lock on a dentry scheduled > for freeing (like, say it, any RCU pathwalk trying to check if the end result can > be grabbed) into a UAF. > > Do you have a better localized reproducer?
BTW, could you reproduce it on viro/vfs.git #work.dcache-busy-wait? It's possible that changes in there might accidentally fix that, and if they did it would narrow the things down a lot.
Some invariants that ought to hold: 1) dentry_free() should never be called without DCACHE_DENTRY_KILLED 2) DCACHE_DENTRY_KILLED should never be set on positive dentries 3) DCACHE_DENTRY_KILLED | DCACHE_PAR_LOOKUP is only possible for dentries that had never been inserted into ->d_in_lookup_hash 4) dentry with DCACHE_DENTRY_KILLED should never become positive
Could you turn that WARN_ON(!hlist_unhashed(&dentry->d_alias)); in whatever you'd been testing into if (WARN_ON(!hlist_unhashed(&dentry->d_alias))) printk(KERN_ERR "->d_inode = %p, ->d_flags = %x", dentry->d_inode, dentry->d_flags); and see what it shows? That's a separate from #work.dcache-busy-wait test - please, do that one on the tree where you'd seen the original bug.
|  |