[PATCH 0/2] udf: Correct lock ordering in udf_setsize()

[PATCH 0/2] udf: Correct lock ordering in udf_setsize()

[Jan Kara]

Hello!

Syzbot has reported a lockdep warning in udf_setsize(). After some analysis
this is actually harmless but this series fixes the lockdep false positive
error. I plan to merge these patches through my tree.

								Honza

[PATCH 1/2] udf: Drop pointless IS_IMMUTABLE and IS_APPEND check

[Jan Kara]

udf_setsize() checks for IS_IMMUTABLE and IS_APPEND flags. This is
however pointless as UDF does not have capability to store these flags
and never allows to set them. Furthermore this is the only place in UDF
code that was actually checking these flags. Remove the pointless check.

Signed-off-by: Jan Kara <jack@suse.cz>
---
 fs/udf/inode.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/fs/udf/inode.c b/fs/udf/inode.c
index 2f831a3a91af..04b0e62cf73f 100644
--- a/fs/udf/inode.c
+++ b/fs/udf/inode.c
@@ -1248,8 +1248,6 @@ int udf_setsize(struct inode *inode, loff_t newsize)
 	if (!(S_ISREG(inode->i_mode) || S_ISDIR(inode->i_mode) ||
 	      S_ISLNK(inode->i_mode)))
 		return -EINVAL;
-	if (IS_APPEND(inode) || IS_IMMUTABLE(inode))
-		return -EPERM;
 
 	filemap_invalidate_lock(inode->i_mapping);
 	iinfo = UDF_I(inode);
-- 
2.35.3

[PATCH 2/2] udf: Fix lock ordering in udf_evict_inode()

[Jan Kara]

udf_evict_inode() calls udf_setsize() to truncate deleted inode.
However inode deletion through udf_evict_inode() can happen from inode
reclaim context and udf_setsize() grabs mapping->invalidate_lock which
isn't generally safe to acquire from fs reclaim context since we
allocate pages under mapping->invalidate_lock for example in a page
fault path.  This is however not a real deadlock possibility as by the
time udf_evict_inode() is called, nobody can be accessing the inode,
even less work with its page cache. So this is just a lockdep triggering
false positive. Fix the problem by moving mapping->invalidate_lock
locking outsize of udf_setsize() into udf_setattr() as grabbing
mapping->invalidate_lock from udf_evict_inode() is pointless.

Reported-by: syzbot+0333a6f4b88bcd68a62f@syzkaller.appspotmail.com
Fixes: b9a861fd527a ("udf: Protect truncate and file type conversion with invalidate_lock")
Signed-off-by: Jan Kara <jack@suse.cz>
---
 fs/udf/file.c  |  2 ++
 fs/udf/inode.c | 11 ++++-------
 2 files changed, 6 insertions(+), 7 deletions(-)

diff --git a/fs/udf/file.c b/fs/udf/file.c
index 0ceac4b5937c..94daaaf76f71 100644
--- a/fs/udf/file.c
+++ b/fs/udf/file.c
@@ -232,7 +232,9 @@ static int udf_setattr(struct mnt_idmap *idmap, struct dentry *dentry,
 
 	if ((attr->ia_valid & ATTR_SIZE) &&
 	    attr->ia_size != i_size_read(inode)) {
+		filemap_invalidate_lock(inode->i_mapping);
 		error = udf_setsize(inode, attr->ia_size);
+		filemap_invalidate_unlock(inode->i_mapping);
 		if (error)
 			return error;
 	}
diff --git a/fs/udf/inode.c b/fs/udf/inode.c
index 04b0e62cf73f..87002ecd79df 100644
--- a/fs/udf/inode.c
+++ b/fs/udf/inode.c
@@ -1249,7 +1249,6 @@ int udf_setsize(struct inode *inode, loff_t newsize)
 	      S_ISLNK(inode->i_mode)))
 		return -EINVAL;
 
-	filemap_invalidate_lock(inode->i_mapping);
 	iinfo = UDF_I(inode);
 	if (newsize > inode->i_size) {
 		if (iinfo->i_alloc_type == ICBTAG_FLAG_AD_IN_ICB) {
@@ -1262,11 +1261,11 @@ int udf_setsize(struct inode *inode, loff_t newsize)
 			}
 			err = udf_expand_file_adinicb(inode);
 			if (err)
-				goto out_unlock;
+				return err;
 		}
 		err = udf_extend_file(inode, newsize);
 		if (err)
-			goto out_unlock;
+			return err;
 set_size:
 		truncate_setsize(inode, newsize);
 	} else {
@@ -1284,14 +1283,14 @@ int udf_setsize(struct inode *inode, loff_t newsize)
 		err = block_truncate_page(inode->i_mapping, newsize,
 					  udf_get_block);
 		if (err)
-			goto out_unlock;
+			return err;
 		truncate_setsize(inode, newsize);
 		down_write(&iinfo->i_data_sem);
 		udf_clear_extent_cache(inode);
 		err = udf_truncate_extents(inode);
 		up_write(&iinfo->i_data_sem);
 		if (err)
-			goto out_unlock;
+			return err;
 	}
 update_time:
 	inode_set_mtime_to_ts(inode, inode_set_ctime_current(inode));
@@ -1299,8 +1298,6 @@ int udf_setsize(struct inode *inode, loff_t newsize)
 		udf_sync_inode(inode);
 	else
 		mark_inode_dirty(inode);
-out_unlock:
-	filemap_invalidate_unlock(inode->i_mapping);
 	return err;
 }
 
-- 
2.35.3