Re: kernel panic: corrupted stack end in dput

[Al Viro]

On Tue, Jul 02, 2019 at 05:21:26PM +0800, Hillf Danton wrote:

> Hello,

> --- a/fs/dcache.c
> +++ b/fs/dcache.c
> @@ -673,14 +673,11 @@ static struct dentry *dentry_kill(struct dentry *dentry)
> 	if (!IS_ROOT(dentry)) {
> 		parent = dentry->d_parent;
> 		if (unlikely(!spin_trylock(&parent->d_lock))) {
> -			parent = __lock_parent(dentry);
> -			if (likely(inode || !dentry->d_inode))
> -				goto got_locks;
> -			/* negative that became positive */
> -			if (parent)
> -				spin_unlock(&parent->d_lock);
> -			inode = dentry->d_inode;
> -			goto slow_positive;
> +			/*
> +			 * fine if peer is busy either populating or
> +			 * cleaning up parent
> +			 */
> +			parent = NULL;
> 		}
> 	}
> 	__dentry_kill(dentry);

This is very much *NOT* fine.
	1) trylock can fail from any number of reasons, starting
with "somebody is going through the hash chain doing a lookup on
something completely unrelated"
	2) whoever had been holding the lock and whatever they'd
been doing might be over right after we get the return value from
spin_trylock().
	3) even had that been really somebody adding children in
the same parent *AND* even if they really kept doing that, rather
than unlocking and buggering off, would you care to explain why
dentry_unlist() called by __dentry_kill() and removing the victim
from the list of children would be safe to do in parallel with that?

NAK, in case it's not obvious from the above.