Re: [syzbot] possible deadlock in mnt_want_write (2)

[Mimi Zohar]

Hi Hillf,g

On Wed, 2022-07-06 at 20:10 +0800, Hillf Danton wrote:
> On Tue, 05 Jul 2022 08:53:15 -0400 Mimi Zohar wrote:
> > 
> > Thank you for the reproducer.  This seems to be a similar false
> > positive as was discussed:
> > https://lore.kernel.org/linux-unionfs/000000000000c5b77105b4c3546e@xxxxxxxxxx/
> > 
> > thanks,
> > 
> 
> Hi Mimi
> 
> Please pick up the patch attached if it makes sense to you.

The patch itself looks good, but missing from the patch description is
an indication that the lockdep warning is a false positive.  Perhaps
add a "Suggested-by" line crediting Amir.   I'd appreciate your posting
the patch on the mailing list.

thanks!

Mimi

> From: Hillf Danton <hdanton@xxxxxxxx>
> Subject: [PATCH] integrity: lockdep annotate of iint->mutex
> 
> This fixes a reported lockdep splat
> 
>         CPU0                    CPU1
>         ----                    ----
> 	lock(&iint->mutex);
> 				lock(sb_writers#4);
> 				lock(&iint->mutex);
> 	lock(sb_writers#4);
>  
> 	*** DEADLOCK ***
> 
> using the method in 4eae06de482b annotating OVL_I(inode)->lock.
> 
> Links: https://lore.kernel.org/linux-unionfs/CAOQ4uxjk4XYuwz5HCmN-Ge=Ld=tM1f7ZxVrd5U1AC2Wisc9MTA@xxxxxxxxxxxxxx/
> Reported-and-tested-by: syzbot <syzbot+b42fe626038981fb7bfa@xxxxxxxxxxxxxxxxxxxxxxxxx>
> Cc: Mimi Zohar <zohar@xxxxxxxxxxxxx>
> Cc: Amir Goldstein <amir73il@xxxxxxxxx>
> Signed-off-by: Hillf Danton <hdanton@xxxxxxxx>
> ---
> 
> --- a/security/integrity/iint.c 
> +++ b/security/integrity/iint.c 
> @@ -85,6 +85,17 @@ static void iint_free(struct integrity_i
>  	kmem_cache_free(iint_cache, iint);
>  }
>  
> +static void iint_annotate_mutex_key(struct integrity_iint_cache *iint, struct inode *inode)
> +{
> +#ifdef CONFIG_LOCKDEP
> +	static struct lock_class_key iint_mutex_key[FILESYSTEM_MAX_STACK_DEPTH];
> +
> +	int depth = inode->i_sb->s_stack_depth;
> +
> +	lockdep_set_class(&iint->mutex, &iint_mutex_key[depth]);
> +#endif
> +}
> +
>  /**
>   * integrity_inode_get - find or allocate an iint associated with an inode
>   * @inode: pointer to the inode
> @@ -114,6 +125,8 @@ struct integrity_iint_cache *integrity_i
>  	if (!iint)
>  		return NULL;
>  
> +	iint_annotate_mutex_key(iint, inode);
> +
>  	write_lock(&integrity_iint_lock);
>  
>  	p = &integrity_iint_tree.rb_node;
> --