linux-kernel - Re: [PATCH] fs: add might_sleep() annotation to iput() and more

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <4hczbhmpigwkkeeqaq2kgw4wumsbrlgis4ld7jz5lq2wdjsnzb@oolwamox5glq>
Date: Wed, 17 Sep 2025 18:26:00 +0200
From: Jan Kara <jack@...e.cz>
To: Max Kellermann <max.kellermann@...os.com>
Cc: Alexander Viro <viro@...iv.linux.org.uk>, 
	Christian Brauner <brauner@...nel.org>, Jan Kara <jack@...e.cz>, linux-fsdevel@...r.kernel.org, 
	linux-kernel@...r.kernel.org, Mateusz Guzik <mjguzik@...il.com>
Subject: Re: [PATCH] fs: add might_sleep() annotation to iput() and more

On Wed 17-09-25 17:36:31, Max Kellermann wrote:
> When iput() drops the reference counter to zero, it may sleep via
> inode_wait_for_writeback().  This happens rarely because it's usually
> the dcache which evicts inodes, but really iput() should only ever be
> called in contexts where sleeping is allowed.  This annotation allows
> finding buggy callers.
> 
> Additionally, this patch annotates a few low-level functions that can
> call iput() conditionally.
> 
> Cc: Mateusz Guzik <mjguzik@...il.com>
> Signed-off-by: Max Kellermann <max.kellermann@...os.com>

Looks sensible. Feel free to add:

Reviewed-by: Jan Kara <jack@...e.cz>

								Honza

> ---
> For discussion of a loosely-related Ceph deadlock bug, see:
>  https://lore.kernel.org/ceph-devel/CAKPOu+-xr+nQuzfjtQCgZCqPtec=8uQiz29H5+5AeFzTbp=1rw@mail.gmail.com/T/
>  https://lore.kernel.org/ceph-devel/CAGudoHF0+JfqxB_fQxeo7Pbadjq7UA1JFH4QmfFS1hDHunNmtw@mail.gmail.com/T/
> ---
>  fs/inode.c | 18 ++++++++++++++++++
>  1 file changed, 18 insertions(+)
> 
> diff --git a/fs/inode.c b/fs/inode.c
> index fc2edb5a4dbe..ec9339024ac3 100644
> --- a/fs/inode.c
> +++ b/fs/inode.c
> @@ -1279,6 +1279,8 @@ struct inode *inode_insert5(struct inode *inode, unsigned long hashval,
>  	struct hlist_head *head = inode_hashtable + hash(inode->i_sb, hashval);
>  	struct inode *old;
>  
> +	might_sleep();
> +
>  again:
>  	spin_lock(&inode_hash_lock);
>  	old = find_inode(inode->i_sb, head, test, data, true);
> @@ -1382,6 +1384,8 @@ struct inode *iget5_locked_rcu(struct super_block *sb, unsigned long hashval,
>  	struct hlist_head *head = inode_hashtable + hash(sb, hashval);
>  	struct inode *inode, *new;
>  
> +	might_sleep();
> +
>  again:
>  	inode = find_inode(sb, head, test, data, false);
>  	if (inode) {
> @@ -1422,6 +1426,9 @@ struct inode *iget_locked(struct super_block *sb, unsigned long ino)
>  {
>  	struct hlist_head *head = inode_hashtable + hash(sb, ino);
>  	struct inode *inode;
> +
> +	might_sleep();
> +
>  again:
>  	inode = find_inode_fast(sb, head, ino, false);
>  	if (inode) {
> @@ -1605,6 +1612,9 @@ struct inode *ilookup5(struct super_block *sb, unsigned long hashval,
>  		int (*test)(struct inode *, void *), void *data)
>  {
>  	struct inode *inode;
> +
> +	might_sleep();
> +
>  again:
>  	inode = ilookup5_nowait(sb, hashval, test, data);
>  	if (inode) {
> @@ -1630,6 +1640,9 @@ struct inode *ilookup(struct super_block *sb, unsigned long ino)
>  {
>  	struct hlist_head *head = inode_hashtable + hash(sb, ino);
>  	struct inode *inode;
> +
> +	might_sleep();
> +
>  again:
>  	inode = find_inode_fast(sb, head, ino, false);
>  
> @@ -1780,6 +1793,8 @@ int insert_inode_locked(struct inode *inode)
>  	ino_t ino = inode->i_ino;
>  	struct hlist_head *head = inode_hashtable + hash(sb, ino);
>  
> +	might_sleep();
> +
>  	while (1) {
>  		struct inode *old = NULL;
>  		spin_lock(&inode_hash_lock);
> @@ -1826,6 +1841,8 @@ int insert_inode_locked4(struct inode *inode, unsigned long hashval,
>  {
>  	struct inode *old;
>  
> +	might_sleep();
> +
>  	inode->i_state |= I_CREATING;
>  	old = inode_insert5(inode, hashval, test, NULL, data);
>  
> @@ -1908,6 +1925,7 @@ static void iput_final(struct inode *inode)
>   */
>  void iput(struct inode *inode)
>  {
> +	might_sleep();
>  	if (unlikely(!inode))
>  		return;
>  
> -- 
> 2.47.3
> 
-- 
Jan Kara <jack@...e.com>
SUSE Labs, CR