All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/22] udf: Fix couple of preallocation related bugs
@ 2023-01-24 12:17 Jan Kara
  2023-01-24 12:17 ` [PATCH 01/22] udf: Unify types in anchor block detection Jan Kara
                   ` (21 more replies)
  0 siblings, 22 replies; 23+ messages in thread
From: Jan Kara @ 2023-01-24 12:17 UTC (permalink / raw)
  To: linux-fsdevel; +Cc: Jan Kara

Hello,

fsx has revealed a couple of bugs related to preallocation handling in UDF.
Firstly, we were not properly cleaning up preallocation for files that
were dirtied via mmap, secondly, we didn't discard preallocation in some
cases when expanding file with a hole which later led to confusion and
data corruption. As part of these changes we start allocating blocks on
page fault time instead of at writeback time and we also cleanup block mapping
interfaces in UDF.

The patches are based on top of my for_next branch:

git://git.kernel.org/pub/scm/linux/kernel/git/jack/linux-fs for_next

I plan to queue these patches to my tree.

								Honza

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [PATCH 01/22] udf: Unify types in anchor block detection
  2023-01-24 12:17 [PATCH 0/22] udf: Fix couple of preallocation related bugs Jan Kara
@ 2023-01-24 12:17 ` Jan Kara
  2023-01-24 12:17 ` [PATCH 02/22] udf: Drop VARCONV support Jan Kara
                   ` (20 subsequent siblings)
  21 siblings, 0 replies; 23+ messages in thread
From: Jan Kara @ 2023-01-24 12:17 UTC (permalink / raw)
  To: linux-fsdevel; +Cc: Jan Kara

When detecting last recorded block and from it derived anchor block
position, we were mixing unsigned long, u32, and sector_t types. Since
udf supports only 32-bit block numbers this is harmless but sometimes
makes things awkward. Convert everything to udf_pblk_t and also handle
the situation when block device size would not fit into udf_pblk_t.

Signed-off-by: Jan Kara <jack@suse.cz>
---
 fs/udf/lowlevel.c | 7 +++++--
 fs/udf/super.c    | 4 ++--
 fs/udf/udfdecl.h  | 2 +-
 3 files changed, 8 insertions(+), 5 deletions(-)

diff --git a/fs/udf/lowlevel.c b/fs/udf/lowlevel.c
index 46d697172197..c87ed942d076 100644
--- a/fs/udf/lowlevel.c
+++ b/fs/udf/lowlevel.c
@@ -45,7 +45,7 @@ unsigned int udf_get_last_session(struct super_block *sb)
 	return 0;
 }
 
-unsigned long udf_get_last_block(struct super_block *sb)
+udf_pblk_t udf_get_last_block(struct super_block *sb)
 {
 	struct cdrom_device_info *cdi = disk_to_cdi(sb->s_bdev->bd_disk);
 	unsigned long lblock = 0;
@@ -54,8 +54,11 @@ unsigned long udf_get_last_block(struct super_block *sb)
 	 * The cdrom layer call failed or returned obviously bogus value?
 	 * Try using the device size...
 	 */
-	if (!cdi || cdrom_get_last_written(cdi, &lblock) || lblock == 0)
+	if (!cdi || cdrom_get_last_written(cdi, &lblock) || lblock == 0) {
+		if (sb_bdev_nr_blocks(sb) > ~(udf_pblk_t)0)
+			return 0;
 		lblock = sb_bdev_nr_blocks(sb);
+	}
 
 	if (lblock)
 		return lblock - 1;
diff --git a/fs/udf/super.c b/fs/udf/super.c
index 241b40e886b3..c756d903a862 100644
--- a/fs/udf/super.c
+++ b/fs/udf/super.c
@@ -1861,10 +1861,10 @@ static int udf_check_anchor_block(struct super_block *sb, sector_t block,
  * Returns < 0 on error, 0 on success. -EAGAIN is special - try next set
  * of anchors.
  */
-static int udf_scan_anchors(struct super_block *sb, sector_t *lastblock,
+static int udf_scan_anchors(struct super_block *sb, udf_pblk_t *lastblock,
 			    struct kernel_lb_addr *fileset)
 {
-	sector_t last[6];
+	udf_pblk_t last[6];
 	int i;
 	struct udf_sb_info *sbi = UDF_SB(sb);
 	int last_count = 0;
diff --git a/fs/udf/udfdecl.h b/fs/udf/udfdecl.h
index d35aa42bb577..eaf9e6fd201e 100644
--- a/fs/udf/udfdecl.h
+++ b/fs/udf/udfdecl.h
@@ -196,7 +196,7 @@ extern void udf_new_tag(char *, uint16_t, uint16_t, uint16_t, uint32_t, int);
 
 /* lowlevel.c */
 extern unsigned int udf_get_last_session(struct super_block *);
-extern unsigned long udf_get_last_block(struct super_block *);
+udf_pblk_t udf_get_last_block(struct super_block *);
 
 /* partition.c */
 extern uint32_t udf_get_pblock(struct super_block *, uint32_t, uint16_t,
-- 
2.35.3


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH 02/22] udf: Drop VARCONV support
  2023-01-24 12:17 [PATCH 0/22] udf: Fix couple of preallocation related bugs Jan Kara
  2023-01-24 12:17 ` [PATCH 01/22] udf: Unify types in anchor block detection Jan Kara
@ 2023-01-24 12:17 ` Jan Kara
  2023-01-24 12:17 ` [PATCH 03/22] udf: Move incrementing of goal block directly into inode_getblk() Jan Kara
                   ` (19 subsequent siblings)
  21 siblings, 0 replies; 23+ messages in thread
From: Jan Kara @ 2023-01-24 12:17 UTC (permalink / raw)
  To: linux-fsdevel; +Cc: Jan Kara

UDF was supporting a strange mode where the media was containing 7
blocks of unknown data for every 32 blocks of the filesystem. I have yet
to see the media that would need such conversion (maybe it comes from
packet writing times) and the conversions have been inconsistent in the
code. In particular any write will write to a wrong block and corrupt
the media. This is an indication and no user actually needs this so
let's just drop the support instead of trying to fix it.

Signed-off-by: Jan Kara <jack@suse.cz>
---
 fs/udf/balloc.c    |  2 +-
 fs/udf/directory.c |  4 ++--
 fs/udf/inode.c     | 11 ++++------
 fs/udf/misc.c      | 18 +----------------
 fs/udf/namei.c     |  4 ++--
 fs/udf/super.c     | 50 +++-------------------------------------------
 fs/udf/truncate.c  |  2 +-
 fs/udf/udf_sb.h    |  1 -
 fs/udf/udfdecl.h   |  6 ------
 9 files changed, 14 insertions(+), 84 deletions(-)

diff --git a/fs/udf/balloc.c b/fs/udf/balloc.c
index 8e597db4d971..cdf90928b7f2 100644
--- a/fs/udf/balloc.c
+++ b/fs/udf/balloc.c
@@ -42,7 +42,7 @@ static int read_block_bitmap(struct super_block *sb,
 	loc.logicalBlockNum = bitmap->s_extPosition;
 	loc.partitionReferenceNum = UDF_SB(sb)->s_partition;
 
-	bh = udf_tread(sb, udf_get_lb_pblock(sb, &loc, block));
+	bh = sb_bread(sb, udf_get_lb_pblock(sb, &loc, block));
 	if (!bh)
 		retval = -EIO;
 
diff --git a/fs/udf/directory.c b/fs/udf/directory.c
index 0f3cc095b2a3..ae61814c195b 100644
--- a/fs/udf/directory.c
+++ b/fs/udf/directory.c
@@ -141,7 +141,7 @@ static void udf_readahead_dir(struct udf_fileident_iter *iter)
 	for (i = 0; i < ralen; i++) {
 		blk = udf_get_lb_pblock(iter->dir->i_sb, &iter->eloc,
 					iter->loffset + i);
-		tmp = udf_tgetblk(iter->dir->i_sb, blk);
+		tmp = sb_getblk(iter->dir->i_sb, blk);
 		if (tmp && !buffer_uptodate(tmp) && !buffer_locked(tmp))
 			bha[num++] = tmp;
 		else
@@ -160,7 +160,7 @@ static struct buffer_head *udf_fiiter_bread_blk(struct udf_fileident_iter *iter)
 
 	udf_readahead_dir(iter);
 	blk = udf_get_lb_pblock(iter->dir->i_sb, &iter->eloc, iter->loffset);
-	return udf_tread(iter->dir->i_sb, blk);
+	return sb_bread(iter->dir->i_sb, blk);
 }
 
 /*
diff --git a/fs/udf/inode.c b/fs/udf/inode.c
index a504c7650551..2b3fc897d1b3 100644
--- a/fs/udf/inode.c
+++ b/fs/udf/inode.c
@@ -1594,7 +1594,7 @@ static int udf_update_inode(struct inode *inode, int do_sync)
 	unsigned char blocksize_bits = inode->i_sb->s_blocksize_bits;
 	struct udf_inode_info *iinfo = UDF_I(inode);
 
-	bh = udf_tgetblk(inode->i_sb,
+	bh = sb_getblk(inode->i_sb,
 			udf_get_lb_pblock(inode->i_sb, &iinfo->i_location, 0));
 	if (!bh) {
 		udf_debug("getblk failure\n");
@@ -1854,7 +1854,7 @@ int udf_setup_indirect_aext(struct inode *inode, udf_pblk_t block,
 	neloc.logicalBlockNum = block;
 	neloc.partitionReferenceNum = epos->block.partitionReferenceNum;
 
-	bh = udf_tgetblk(sb, udf_get_lb_pblock(sb, &neloc, 0));
+	bh = sb_getblk(sb, udf_get_lb_pblock(sb, &neloc, 0));
 	if (!bh)
 		return -EIO;
 	lock_buffer(bh);
@@ -2071,7 +2071,7 @@ int8_t udf_next_aext(struct inode *inode, struct extent_position *epos,
 		epos->offset = sizeof(struct allocExtDesc);
 		brelse(epos->bh);
 		block = udf_get_lb_pblock(inode->i_sb, &epos->block, 0);
-		epos->bh = udf_tread(inode->i_sb, block);
+		epos->bh = sb_bread(inode->i_sb, block);
 		if (!epos->bh) {
 			udf_debug("reading block %u failed!\n", block);
 			return -1;
@@ -2292,8 +2292,5 @@ udf_pblk_t udf_block_map(struct inode *inode, sector_t block)
 	up_read(&UDF_I(inode)->i_data_sem);
 	brelse(epos.bh);
 
-	if (UDF_QUERY_FLAG(inode->i_sb, UDF_FLAG_VARCONV))
-		return udf_fixed_to_variable(ret);
-	else
-		return ret;
+	return ret;
 }
diff --git a/fs/udf/misc.c b/fs/udf/misc.c
index 1614d308d0f0..3777468d06ce 100644
--- a/fs/udf/misc.c
+++ b/fs/udf/misc.c
@@ -28,22 +28,6 @@
 #include "udf_i.h"
 #include "udf_sb.h"
 
-struct buffer_head *udf_tgetblk(struct super_block *sb, udf_pblk_t block)
-{
-	if (UDF_QUERY_FLAG(sb, UDF_FLAG_VARCONV))
-		return sb_getblk(sb, udf_fixed_to_variable(block));
-	else
-		return sb_getblk(sb, block);
-}
-
-struct buffer_head *udf_tread(struct super_block *sb, udf_pblk_t block)
-{
-	if (UDF_QUERY_FLAG(sb, UDF_FLAG_VARCONV))
-		return sb_bread(sb, udf_fixed_to_variable(block));
-	else
-		return sb_bread(sb, block);
-}
-
 struct genericFormat *udf_add_extendedattr(struct inode *inode, uint32_t size,
 					   uint32_t type, uint8_t loc)
 {
@@ -216,7 +200,7 @@ struct buffer_head *udf_read_tagged(struct super_block *sb, uint32_t block,
 	if (block == 0xFFFFFFFF)
 		return NULL;
 
-	bh = udf_tread(sb, block);
+	bh = sb_bread(sb, block);
 	if (!bh) {
 		udf_err(sb, "read failed, block=%u, location=%u\n",
 			block, location);
diff --git a/fs/udf/namei.c b/fs/udf/namei.c
index f8091aa22fcd..49fab30afff3 100644
--- a/fs/udf/namei.c
+++ b/fs/udf/namei.c
@@ -171,7 +171,7 @@ static struct buffer_head *udf_expand_dir_adinicb(struct inode *inode,
 				0);
 	if (!newblock)
 		return NULL;
-	dbh = udf_tgetblk(inode->i_sb, newblock);
+	dbh = sb_getblk(inode->i_sb, newblock);
 	if (!dbh)
 		return NULL;
 	lock_buffer(dbh);
@@ -619,7 +619,7 @@ static int udf_symlink(struct user_namespace *mnt_userns, struct inode *dir,
 		block = udf_get_pblock(sb, block,
 				iinfo->i_location.partitionReferenceNum,
 				0);
-		epos.bh = udf_tgetblk(sb, block);
+		epos.bh = sb_getblk(sb, block);
 		if (unlikely(!epos.bh)) {
 			err = -ENOMEM;
 			udf_free_blocks(sb, inode, &eloc, 0, 1);
diff --git a/fs/udf/super.c b/fs/udf/super.c
index c756d903a862..58a3148173ac 100644
--- a/fs/udf/super.c
+++ b/fs/udf/super.c
@@ -734,7 +734,7 @@ static int udf_check_vsd(struct super_block *sb)
 	 * added */
 	for (; !nsr && sector < VSD_MAX_SECTOR_OFFSET; sector += sectorsize) {
 		/* Read a block */
-		bh = udf_tread(sb, sector >> sb->s_blocksize_bits);
+		bh = sb_bread(sb, sector >> sb->s_blocksize_bits);
 		if (!bh)
 			break;
 
@@ -1839,10 +1839,6 @@ static int udf_check_anchor_block(struct super_block *sb, sector_t block,
 	uint16_t ident;
 	int ret;
 
-	if (UDF_QUERY_FLAG(sb, UDF_FLAG_VARCONV) &&
-	    udf_fixed_to_variable(block) >= sb_bdev_nr_blocks(sb))
-		return -EAGAIN;
-
 	bh = udf_read_tagged(sb, block, block, &ident);
 	if (!bh)
 		return -EAGAIN;
@@ -1924,46 +1920,6 @@ static int udf_scan_anchors(struct super_block *sb, udf_pblk_t *lastblock,
 	return udf_check_anchor_block(sb, sbi->s_session + 512, fileset);
 }
 
-/*
- * Find an anchor volume descriptor and load Volume Descriptor Sequence from
- * area specified by it. The function expects sbi->s_lastblock to be the last
- * block on the media.
- *
- * Return <0 on error, 0 if anchor found. -EAGAIN is special meaning anchor
- * was not found.
- */
-static int udf_find_anchor(struct super_block *sb,
-			   struct kernel_lb_addr *fileset)
-{
-	struct udf_sb_info *sbi = UDF_SB(sb);
-	sector_t lastblock = sbi->s_last_block;
-	int ret;
-
-	ret = udf_scan_anchors(sb, &lastblock, fileset);
-	if (ret != -EAGAIN)
-		goto out;
-
-	/* No anchor found? Try VARCONV conversion of block numbers */
-	UDF_SET_FLAG(sb, UDF_FLAG_VARCONV);
-	lastblock = udf_variable_to_fixed(sbi->s_last_block);
-	/* Firstly, we try to not convert number of the last block */
-	ret = udf_scan_anchors(sb, &lastblock, fileset);
-	if (ret != -EAGAIN)
-		goto out;
-
-	lastblock = sbi->s_last_block;
-	/* Secondly, we try with converted number of the last block */
-	ret = udf_scan_anchors(sb, &lastblock, fileset);
-	if (ret < 0) {
-		/* VARCONV didn't help. Clear it. */
-		UDF_CLEAR_FLAG(sb, UDF_FLAG_VARCONV);
-	}
-out:
-	if (ret == 0)
-		sbi->s_last_block = lastblock;
-	return ret;
-}
-
 /*
  * Check Volume Structure Descriptor, find Anchor block and load Volume
  * Descriptor Sequence.
@@ -2004,7 +1960,7 @@ static int udf_load_vrs(struct super_block *sb, struct udf_options *uopt,
 
 	/* Look for anchor block and load Volume Descriptor Sequence */
 	sbi->s_anchor = uopt->anchor;
-	ret = udf_find_anchor(sb, fileset);
+	ret = udf_scan_anchors(sb, &sbi->s_last_block, fileset);
 	if (ret < 0) {
 		if (!silent && ret == -EAGAIN)
 			udf_warn(sb, "No anchor found\n");
@@ -2455,7 +2411,7 @@ static unsigned int udf_count_free_bitmap(struct super_block *sb,
 		if (bytes) {
 			brelse(bh);
 			newblock = udf_get_lb_pblock(sb, &loc, ++block);
-			bh = udf_tread(sb, newblock);
+			bh = sb_bread(sb, newblock);
 			if (!bh) {
 				udf_debug("read failed\n");
 				goto out;
diff --git a/fs/udf/truncate.c b/fs/udf/truncate.c
index 036ebd892b85..3d2cfc7a1449 100644
--- a/fs/udf/truncate.c
+++ b/fs/udf/truncate.c
@@ -240,7 +240,7 @@ int udf_truncate_extents(struct inode *inode)
 			brelse(epos.bh);
 			epos.offset = sizeof(struct allocExtDesc);
 			epos.block = eloc;
-			epos.bh = udf_tread(sb,
+			epos.bh = sb_bread(sb,
 					udf_get_lb_pblock(sb, &eloc, 0));
 			/* Error reading indirect block? */
 			if (!epos.bh)
diff --git a/fs/udf/udf_sb.h b/fs/udf/udf_sb.h
index 6bccff3c70f5..9af6ff7f9747 100644
--- a/fs/udf/udf_sb.h
+++ b/fs/udf/udf_sb.h
@@ -23,7 +23,6 @@
 #define UDF_FLAG_STRICT			5
 #define UDF_FLAG_UNDELETE		6
 #define UDF_FLAG_UNHIDE			7
-#define UDF_FLAG_VARCONV		8
 #define UDF_FLAG_UID_FORGET     11    /* save -1 for uid to disk */
 #define UDF_FLAG_GID_FORGET     12
 #define UDF_FLAG_UID_SET	13
diff --git a/fs/udf/udfdecl.h b/fs/udf/udfdecl.h
index eaf9e6fd201e..88667a80795d 100644
--- a/fs/udf/udfdecl.h
+++ b/fs/udf/udfdecl.h
@@ -34,9 +34,6 @@ extern __printf(3, 4) void _udf_warn(struct super_block *sb,
 #define udf_debug(fmt, ...)					\
 	pr_debug("%s:%d:%s: " fmt, __FILE__, __LINE__, __func__, ##__VA_ARGS__)
 
-#define udf_fixed_to_variable(x) ( ( ( (x) >> 5 ) * 39 ) + ( (x) & 0x0000001F ) )
-#define udf_variable_to_fixed(x) ( ( ( (x) / 39 ) << 5 ) + ( (x) % 39 ) )
-
 #define UDF_EXTENT_LENGTH_MASK	0x3FFFFFFF
 #define UDF_EXTENT_FLAG_MASK	0xC0000000
 
@@ -179,9 +176,6 @@ extern int8_t udf_current_aext(struct inode *, struct extent_position *,
 extern void udf_update_extra_perms(struct inode *inode, umode_t mode);
 
 /* misc.c */
-extern struct buffer_head *udf_tgetblk(struct super_block *sb,
-					udf_pblk_t block);
-extern struct buffer_head *udf_tread(struct super_block *sb, udf_pblk_t block);
 extern struct genericFormat *udf_add_extendedattr(struct inode *, uint32_t,
 						  uint32_t, uint8_t);
 extern struct genericFormat *udf_get_extendedattr(struct inode *, uint32_t,
-- 
2.35.3


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH 03/22] udf: Move incrementing of goal block directly into inode_getblk()
  2023-01-24 12:17 [PATCH 0/22] udf: Fix couple of preallocation related bugs Jan Kara
  2023-01-24 12:17 ` [PATCH 01/22] udf: Unify types in anchor block detection Jan Kara
  2023-01-24 12:17 ` [PATCH 02/22] udf: Drop VARCONV support Jan Kara
@ 2023-01-24 12:17 ` Jan Kara
  2023-01-24 12:17 ` [PATCH 04/22] udf: Factor out block mapping into udf_map_block() Jan Kara
                   ` (18 subsequent siblings)
  21 siblings, 0 replies; 23+ messages in thread
From: Jan Kara @ 2023-01-24 12:17 UTC (permalink / raw)
  To: linux-fsdevel; +Cc: Jan Kara

inode_getblk() sets goal block for the next allocation to the currently
allocated block. This is obviously one less than what the goal block
should be which we fixup in udf_get_block(). Just set the right goal
block directly in inode_getblk().

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

diff --git a/fs/udf/inode.c b/fs/udf/inode.c
index 2b3fc897d1b3..ff414fff354a 100644
--- a/fs/udf/inode.c
+++ b/fs/udf/inode.c
@@ -339,11 +339,6 @@ static int udf_get_block(struct inode *inode, sector_t block,
 	iinfo = UDF_I(inode);
 
 	down_write(&iinfo->i_data_sem);
-	if (block == iinfo->i_next_alloc_block + 1) {
-		iinfo->i_next_alloc_block++;
-		iinfo->i_next_alloc_goal++;
-	}
-
 	/*
 	 * Block beyond EOF and prealloc extents? Just discard preallocation
 	 * as it is not useful and complicates things.
@@ -812,8 +807,8 @@ static sector_t inode_getblk(struct inode *inode, sector_t block,
 		goto out_free;
 	}
 	*new = 1;
-	iinfo->i_next_alloc_block = block;
-	iinfo->i_next_alloc_goal = newblocknum;
+	iinfo->i_next_alloc_block = block + 1;
+	iinfo->i_next_alloc_goal = newblocknum + 1;
 	inode->i_ctime = current_time(inode);
 
 	if (IS_SYNC(inode))
-- 
2.35.3


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH 04/22] udf: Factor out block mapping into udf_map_block()
  2023-01-24 12:17 [PATCH 0/22] udf: Fix couple of preallocation related bugs Jan Kara
                   ` (2 preceding siblings ...)
  2023-01-24 12:17 ` [PATCH 03/22] udf: Move incrementing of goal block directly into inode_getblk() Jan Kara
@ 2023-01-24 12:17 ` Jan Kara
  2023-01-24 12:17 ` [PATCH 05/22] udf: Use udf_bread() in udf_get_pblock_virt15() Jan Kara
                   ` (17 subsequent siblings)
  21 siblings, 0 replies; 23+ messages in thread
From: Jan Kara @ 2023-01-24 12:17 UTC (permalink / raw)
  To: linux-fsdevel; +Cc: Jan Kara

Create new block mapping function udf_map_block() that takes new
udf_map_rq structure describing mapping request. We will convert other
places to use this function for block mapping.

Signed-off-by: Jan Kara <jack@suse.cz>
---
 fs/udf/inode.c   | 70 +++++++++++++++++++++++++++++++++---------------
 fs/udf/udfdecl.h |  1 +
 2 files changed, 49 insertions(+), 22 deletions(-)

diff --git a/fs/udf/inode.c b/fs/udf/inode.c
index ff414fff354a..53d2d8fef158 100644
--- a/fs/udf/inode.c
+++ b/fs/udf/inode.c
@@ -52,6 +52,8 @@
 #define FE_DELETE_PERMS	(FE_PERM_U_DELETE | FE_PERM_G_DELETE | \
 			 FE_PERM_O_DELETE)
 
+struct udf_map_rq;
+
 static umode_t udf_convert_permissions(struct fileEntry *);
 static int udf_update_inode(struct inode *, int);
 static int udf_sync_inode(struct inode *inode);
@@ -320,43 +322,67 @@ int udf_expand_file_adinicb(struct inode *inode)
 	return err;
 }
 
-static int udf_get_block(struct inode *inode, sector_t block,
-			 struct buffer_head *bh_result, int create)
+#define UDF_MAP_CREATE	0x01	/* Mapping can allocate new blocks */
+
+#define UDF_BLK_MAPPED	0x01	/* Block was successfully mapped */
+#define UDF_BLK_NEW	0x02	/* Block was freshly allocated */
+
+struct udf_map_rq {
+	sector_t lblk;
+	udf_pblk_t pblk;
+	int iflags;		/* UDF_MAP_ flags determining behavior */
+	int oflags;		/* UDF_BLK_ flags reporting results */
+};
+
+static int udf_map_block(struct inode *inode, struct udf_map_rq *map)
 {
 	int err, new;
-	sector_t phys = 0;
-	struct udf_inode_info *iinfo;
+	struct udf_inode_info *iinfo = UDF_I(inode);
 
-	if (!create) {
-		phys = udf_block_map(inode, block);
-		if (phys)
-			map_bh(bh_result, inode->i_sb, phys);
+	map->oflags = 0;
+	if (!(map->iflags & UDF_MAP_CREATE)) {
+		map->pblk = udf_block_map(inode, map->lblk);
+		if (map->pblk != 0)
+			map->oflags |= UDF_BLK_MAPPED;
 		return 0;
 	}
 
-	err = -EIO;
-	new = 0;
-	iinfo = UDF_I(inode);
-
 	down_write(&iinfo->i_data_sem);
 	/*
 	 * Block beyond EOF and prealloc extents? Just discard preallocation
 	 * as it is not useful and complicates things.
 	 */
-	if (((loff_t)block) << inode->i_blkbits > iinfo->i_lenExtents)
+	if (((loff_t)map->lblk) << inode->i_blkbits > iinfo->i_lenExtents)
 		udf_discard_prealloc(inode);
 	udf_clear_extent_cache(inode);
-	phys = inode_getblk(inode, block, &err, &new);
-	if (!phys)
-		goto abort;
-
+	map->pblk = inode_getblk(inode, map->lblk, &err, &new);
+	up_write(&iinfo->i_data_sem);
+	if (err)
+		return err;
+	map->oflags |= UDF_BLK_MAPPED;
 	if (new)
-		set_buffer_new(bh_result);
-	map_bh(bh_result, inode->i_sb, phys);
+		map->oflags |= UDF_BLK_NEW;
+	return 0;
+}
 
-abort:
-	up_write(&iinfo->i_data_sem);
-	return err;
+static int udf_get_block(struct inode *inode, sector_t block,
+			 struct buffer_head *bh_result, int create)
+{
+	int err;
+	struct udf_map_rq map = {
+		.lblk = block,
+		.iflags = create ? UDF_MAP_CREATE : 0,
+	};
+
+	err = udf_map_block(inode, &map);
+	if (err < 0)
+		return err;
+	if (map.oflags & UDF_BLK_MAPPED) {
+		map_bh(bh_result, inode->i_sb, map.pblk);
+		if (map.oflags & UDF_BLK_NEW)
+			set_buffer_new(bh_result);
+	}
+	return 0;
 }
 
 static struct buffer_head *udf_getblk(struct inode *inode, udf_pblk_t block,
diff --git a/fs/udf/udfdecl.h b/fs/udf/udfdecl.h
index 88667a80795d..d791458fe52a 100644
--- a/fs/udf/udfdecl.h
+++ b/fs/udf/udfdecl.h
@@ -138,6 +138,7 @@ static inline unsigned int udf_dir_entry_len(struct fileIdentDesc *cfi)
 
 /* file.c */
 extern long udf_ioctl(struct file *, unsigned int, unsigned long);
+
 /* inode.c */
 extern struct inode *__udf_iget(struct super_block *, struct kernel_lb_addr *,
 				bool hidden_inode);
-- 
2.35.3


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH 05/22] udf: Use udf_bread() in udf_get_pblock_virt15()
  2023-01-24 12:17 [PATCH 0/22] udf: Fix couple of preallocation related bugs Jan Kara
                   ` (3 preceding siblings ...)
  2023-01-24 12:17 ` [PATCH 04/22] udf: Factor out block mapping into udf_map_block() Jan Kara
@ 2023-01-24 12:17 ` Jan Kara
  2023-01-24 12:17 ` [PATCH 06/22] udf: Use udf_bread() in udf_load_vat() Jan Kara
                   ` (16 subsequent siblings)
  21 siblings, 0 replies; 23+ messages in thread
From: Jan Kara @ 2023-01-24 12:17 UTC (permalink / raw)
  To: linux-fsdevel; +Cc: Jan Kara

Use udf_bread() instead of mapping and reading buffer head manually in
udf_get_pblock_virt15().

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

diff --git a/fs/udf/partition.c b/fs/udf/partition.c
index 4cbf40575965..92765d2f6958 100644
--- a/fs/udf/partition.c
+++ b/fs/udf/partition.c
@@ -54,6 +54,7 @@ uint32_t udf_get_pblock_virt15(struct super_block *sb, uint32_t block,
 	struct udf_part_map *map;
 	struct udf_virtual_data *vdata;
 	struct udf_inode_info *iinfo = UDF_I(sbi->s_vat_inode);
+	int err;
 
 	map = &sbi->s_partmaps[partition];
 	vdata = &map->s_type_specific.s_virtual;
@@ -79,9 +80,7 @@ uint32_t udf_get_pblock_virt15(struct super_block *sb, uint32_t block,
 		index = vdata->s_start_offset / sizeof(uint32_t) + block;
 	}
 
-	loc = udf_block_map(sbi->s_vat_inode, newblock);
-
-	bh = sb_bread(sb, loc);
+	bh = udf_bread(sbi->s_vat_inode, newblock, 0, &err);
 	if (!bh) {
 		udf_debug("get_pblock(UDF_VIRTUAL_MAP:%p,%u,%u) VAT: %u[%u]\n",
 			  sb, block, partition, loc, index);
-- 
2.35.3


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH 06/22] udf: Use udf_bread() in udf_load_vat()
  2023-01-24 12:17 [PATCH 0/22] udf: Fix couple of preallocation related bugs Jan Kara
                   ` (4 preceding siblings ...)
  2023-01-24 12:17 ` [PATCH 05/22] udf: Use udf_bread() in udf_get_pblock_virt15() Jan Kara
@ 2023-01-24 12:17 ` Jan Kara
  2023-01-24 12:17 ` [PATCH 07/22] udf: Do not call udf_block_map() on ICB files Jan Kara
                   ` (15 subsequent siblings)
  21 siblings, 0 replies; 23+ messages in thread
From: Jan Kara @ 2023-01-24 12:17 UTC (permalink / raw)
  To: linux-fsdevel; +Cc: Jan Kara

Use udf_bread() instead of mapping and loadign buffer head manually in
udf_load_vat().

Signed-off-by: Jan Kara <jack@suse.cz>
---
 fs/udf/super.c | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/fs/udf/super.c b/fs/udf/super.c
index 58a3148173ac..df5287c5d659 100644
--- a/fs/udf/super.c
+++ b/fs/udf/super.c
@@ -1176,7 +1176,6 @@ static int udf_load_vat(struct super_block *sb, int p_index, int type1_index)
 	struct udf_part_map *map = &sbi->s_partmaps[p_index];
 	struct buffer_head *bh = NULL;
 	struct udf_inode_info *vati;
-	uint32_t pos;
 	struct virtualAllocationTable20 *vat20;
 	sector_t blocks = sb_bdev_nr_blocks(sb);
 
@@ -1198,10 +1197,14 @@ static int udf_load_vat(struct super_block *sb, int p_index, int type1_index)
 	} else if (map->s_partition_type == UDF_VIRTUAL_MAP20) {
 		vati = UDF_I(sbi->s_vat_inode);
 		if (vati->i_alloc_type != ICBTAG_FLAG_AD_IN_ICB) {
-			pos = udf_block_map(sbi->s_vat_inode, 0);
-			bh = sb_bread(sb, pos);
-			if (!bh)
-				return -EIO;
+			int err = 0;
+
+			bh = udf_bread(sbi->s_vat_inode, 0, 0, &err);
+			if (!bh) {
+				if (!err)
+					err = -EFSCORRUPTED;
+				return err;
+			}
 			vat20 = (struct virtualAllocationTable20 *)bh->b_data;
 		} else {
 			vat20 = (struct virtualAllocationTable20 *)
-- 
2.35.3


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH 07/22] udf: Do not call udf_block_map() on ICB files
  2023-01-24 12:17 [PATCH 0/22] udf: Fix couple of preallocation related bugs Jan Kara
                   ` (5 preceding siblings ...)
  2023-01-24 12:17 ` [PATCH 06/22] udf: Use udf_bread() in udf_load_vat() Jan Kara
@ 2023-01-24 12:17 ` Jan Kara
  2023-01-24 12:17 ` [PATCH 08/22] udf: Convert udf_symlink_filler() to use udf_bread() Jan Kara
                   ` (14 subsequent siblings)
  21 siblings, 0 replies; 23+ messages in thread
From: Jan Kara @ 2023-01-24 12:17 UTC (permalink / raw)
  To: linux-fsdevel; +Cc: Jan Kara

Currently udf_symlink_filler() called udf_block_map() even on files
which have data stored inside the ICB. This is invalid as we cannot map
blocks for such files (although so far the error got silently ignored).
The call happened because we could not call block mapping function once
we've acquired i_data_sem and determined whether the file has data
stored in the ICB. For symlinks the situation is luckily simple as they
get never modified so file type never changes once it is set. Hence we
can check the file type even without i_data_sem. Just drop the
i_data_sem locking and move block mapping to where it is needed.

Signed-off-by: Jan Kara <jack@suse.cz>
---
 fs/udf/symlink.c | 19 +++++++------------
 1 file changed, 7 insertions(+), 12 deletions(-)

diff --git a/fs/udf/symlink.c b/fs/udf/symlink.c
index f3642f9c23f8..451d6d6c701e 100644
--- a/fs/udf/symlink.c
+++ b/fs/udf/symlink.c
@@ -109,27 +109,24 @@ static int udf_symlink_filler(struct file *file, struct folio *folio)
 	unsigned char *symlink;
 	int err;
 	unsigned char *p = page_address(page);
-	struct udf_inode_info *iinfo;
+	struct udf_inode_info *iinfo = UDF_I(inode);
 	uint32_t pos;
 
 	/* We don't support symlinks longer than one block */
 	if (inode->i_size > inode->i_sb->s_blocksize) {
 		err = -ENAMETOOLONG;
-		goto out_unmap;
+		goto out_unlock;
 	}
 
-	iinfo = UDF_I(inode);
-	pos = udf_block_map(inode, 0);
-
-	down_read(&iinfo->i_data_sem);
 	if (iinfo->i_alloc_type == ICBTAG_FLAG_AD_IN_ICB) {
 		symlink = iinfo->i_data + iinfo->i_lenEAttr;
 	} else {
+		pos = udf_block_map(inode, 0);
 		bh = sb_bread(inode->i_sb, pos);
 
 		if (!bh) {
 			err = -EIO;
-			goto out_unlock_inode;
+			goto out_err;
 		}
 
 		symlink = bh->b_data;
@@ -138,17 +135,15 @@ static int udf_symlink_filler(struct file *file, struct folio *folio)
 	err = udf_pc_to_char(inode->i_sb, symlink, inode->i_size, p, PAGE_SIZE);
 	brelse(bh);
 	if (err)
-		goto out_unlock_inode;
+		goto out_err;
 
-	up_read(&iinfo->i_data_sem);
 	SetPageUptodate(page);
 	unlock_page(page);
 	return 0;
 
-out_unlock_inode:
-	up_read(&iinfo->i_data_sem);
+out_err:
 	SetPageError(page);
-out_unmap:
+out_unlock:
 	unlock_page(page);
 	return err;
 }
-- 
2.35.3


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH 08/22] udf: Convert udf_symlink_filler() to use udf_bread()
  2023-01-24 12:17 [PATCH 0/22] udf: Fix couple of preallocation related bugs Jan Kara
                   ` (6 preceding siblings ...)
  2023-01-24 12:17 ` [PATCH 07/22] udf: Do not call udf_block_map() on ICB files Jan Kara
@ 2023-01-24 12:17 ` Jan Kara
  2023-01-24 12:17 ` [PATCH 09/22] udf: Fold udf_block_map() into udf_map_block() Jan Kara
                   ` (13 subsequent siblings)
  21 siblings, 0 replies; 23+ messages in thread
From: Jan Kara @ 2023-01-24 12:17 UTC (permalink / raw)
  To: linux-fsdevel; +Cc: Jan Kara

Convert udf_symlink_filler() to use udf_bread() instead of mapping and
reading buffer head manually.

Signed-off-by: Jan Kara <jack@suse.cz>
---
 fs/udf/symlink.c | 11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/fs/udf/symlink.c b/fs/udf/symlink.c
index 451d6d6c701e..a6cabaa5f1c2 100644
--- a/fs/udf/symlink.c
+++ b/fs/udf/symlink.c
@@ -107,10 +107,9 @@ static int udf_symlink_filler(struct file *file, struct folio *folio)
 	struct inode *inode = page->mapping->host;
 	struct buffer_head *bh = NULL;
 	unsigned char *symlink;
-	int err;
+	int err = 0;
 	unsigned char *p = page_address(page);
 	struct udf_inode_info *iinfo = UDF_I(inode);
-	uint32_t pos;
 
 	/* We don't support symlinks longer than one block */
 	if (inode->i_size > inode->i_sb->s_blocksize) {
@@ -121,14 +120,12 @@ static int udf_symlink_filler(struct file *file, struct folio *folio)
 	if (iinfo->i_alloc_type == ICBTAG_FLAG_AD_IN_ICB) {
 		symlink = iinfo->i_data + iinfo->i_lenEAttr;
 	} else {
-		pos = udf_block_map(inode, 0);
-		bh = sb_bread(inode->i_sb, pos);
-
+		bh = udf_bread(inode, 0, 0, &err);
 		if (!bh) {
-			err = -EIO;
+			if (!err)
+				err = -EFSCORRUPTED;
 			goto out_err;
 		}
-
 		symlink = bh->b_data;
 	}
 
-- 
2.35.3


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH 09/22] udf: Fold udf_block_map() into udf_map_block()
  2023-01-24 12:17 [PATCH 0/22] udf: Fix couple of preallocation related bugs Jan Kara
                   ` (7 preceding siblings ...)
  2023-01-24 12:17 ` [PATCH 08/22] udf: Convert udf_symlink_filler() to use udf_bread() Jan Kara
@ 2023-01-24 12:17 ` Jan Kara
  2023-01-24 12:17 ` [PATCH 10/22] udf: Pass mapping request into inode_getblk() Jan Kara
                   ` (12 subsequent siblings)
  21 siblings, 0 replies; 23+ messages in thread
From: Jan Kara @ 2023-01-24 12:17 UTC (permalink / raw)
  To: linux-fsdevel; +Cc: Jan Kara

udf_block_map() has now only a single caller. Fold it there.

Signed-off-by: Jan Kara <jack@suse.cz>
---
 fs/udf/inode.c   | 38 ++++++++++++++------------------------
 fs/udf/udfdecl.h |  1 -
 2 files changed, 14 insertions(+), 25 deletions(-)

diff --git a/fs/udf/inode.c b/fs/udf/inode.c
index 53d2d8fef158..e098d69991d0 100644
--- a/fs/udf/inode.c
+++ b/fs/udf/inode.c
@@ -341,9 +341,21 @@ static int udf_map_block(struct inode *inode, struct udf_map_rq *map)
 
 	map->oflags = 0;
 	if (!(map->iflags & UDF_MAP_CREATE)) {
-		map->pblk = udf_block_map(inode, map->lblk);
-		if (map->pblk != 0)
+		struct kernel_lb_addr eloc;
+		uint32_t elen;
+		sector_t offset;
+		struct extent_position epos = {};
+
+		down_read(&iinfo->i_data_sem);
+		if (inode_bmap(inode, map->lblk, &epos, &eloc, &elen, &offset)
+				== (EXT_RECORDED_ALLOCATED >> 30)) {
+			map->pblk = udf_get_lb_pblock(inode->i_sb, &eloc,
+							offset);
 			map->oflags |= UDF_BLK_MAPPED;
+		}
+		up_read(&iinfo->i_data_sem);
+		brelse(epos.bh);
+
 		return 0;
 	}
 
@@ -2293,25 +2305,3 @@ int8_t inode_bmap(struct inode *inode, sector_t block,
 
 	return etype;
 }
-
-udf_pblk_t udf_block_map(struct inode *inode, sector_t block)
-{
-	struct kernel_lb_addr eloc;
-	uint32_t elen;
-	sector_t offset;
-	struct extent_position epos = {};
-	udf_pblk_t ret;
-
-	down_read(&UDF_I(inode)->i_data_sem);
-
-	if (inode_bmap(inode, block, &epos, &eloc, &elen, &offset) ==
-						(EXT_RECORDED_ALLOCATED >> 30))
-		ret = udf_get_lb_pblock(inode->i_sb, &eloc, offset);
-	else
-		ret = 0;
-
-	up_read(&UDF_I(inode)->i_data_sem);
-	brelse(epos.bh);
-
-	return ret;
-}
diff --git a/fs/udf/udfdecl.h b/fs/udf/udfdecl.h
index d791458fe52a..98b4d89b4368 100644
--- a/fs/udf/udfdecl.h
+++ b/fs/udf/udfdecl.h
@@ -158,7 +158,6 @@ extern struct buffer_head *udf_bread(struct inode *inode, udf_pblk_t block,
 extern int udf_setsize(struct inode *, loff_t);
 extern void udf_evict_inode(struct inode *);
 extern int udf_write_inode(struct inode *, struct writeback_control *wbc);
-extern udf_pblk_t udf_block_map(struct inode *inode, sector_t block);
 extern int8_t inode_bmap(struct inode *, sector_t, struct extent_position *,
 			 struct kernel_lb_addr *, uint32_t *, sector_t *);
 extern int udf_setup_indirect_aext(struct inode *inode, udf_pblk_t block,
-- 
2.35.3


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH 10/22] udf: Pass mapping request into inode_getblk()
  2023-01-24 12:17 [PATCH 0/22] udf: Fix couple of preallocation related bugs Jan Kara
                   ` (8 preceding siblings ...)
  2023-01-24 12:17 ` [PATCH 09/22] udf: Fold udf_block_map() into udf_map_block() Jan Kara
@ 2023-01-24 12:17 ` Jan Kara
  2023-01-24 12:17 ` [PATCH 11/22] udf: Add flag to disable block preallocation Jan Kara
                   ` (11 subsequent siblings)
  21 siblings, 0 replies; 23+ messages in thread
From: Jan Kara @ 2023-01-24 12:17 UTC (permalink / raw)
  To: linux-fsdevel; +Cc: Jan Kara

Pass struct udf_map_rq into inode_getblk() instead of unfolding it and
the putting the results back.

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

diff --git a/fs/udf/inode.c b/fs/udf/inode.c
index e098d69991d0..5c6725a5bb88 100644
--- a/fs/udf/inode.c
+++ b/fs/udf/inode.c
@@ -58,7 +58,7 @@ static umode_t udf_convert_permissions(struct fileEntry *);
 static int udf_update_inode(struct inode *, int);
 static int udf_sync_inode(struct inode *inode);
 static int udf_alloc_i_data(struct inode *inode, size_t size);
-static sector_t inode_getblk(struct inode *, sector_t, int *, int *);
+static int inode_getblk(struct inode *inode, struct udf_map_rq *map);
 static int udf_insert_aext(struct inode *, struct extent_position,
 			   struct kernel_lb_addr, uint32_t);
 static void udf_split_extents(struct inode *, int *, int, udf_pblk_t,
@@ -336,7 +336,7 @@ struct udf_map_rq {
 
 static int udf_map_block(struct inode *inode, struct udf_map_rq *map)
 {
-	int err, new;
+	int err;
 	struct udf_inode_info *iinfo = UDF_I(inode);
 
 	map->oflags = 0;
@@ -367,14 +367,9 @@ static int udf_map_block(struct inode *inode, struct udf_map_rq *map)
 	if (((loff_t)map->lblk) << inode->i_blkbits > iinfo->i_lenExtents)
 		udf_discard_prealloc(inode);
 	udf_clear_extent_cache(inode);
-	map->pblk = inode_getblk(inode, map->lblk, &err, &new);
+	err = inode_getblk(inode, map);
 	up_write(&iinfo->i_data_sem);
-	if (err)
-		return err;
-	map->oflags |= UDF_BLK_MAPPED;
-	if (new)
-		map->oflags |= UDF_BLK_NEW;
-	return 0;
+	return err;
 }
 
 static int udf_get_block(struct inode *inode, sector_t block,
@@ -627,8 +622,7 @@ static int udf_extend_file(struct inode *inode, loff_t newsize)
 	return err;
 }
 
-static sector_t inode_getblk(struct inode *inode, sector_t block,
-			     int *err, int *new)
+static int inode_getblk(struct inode *inode, struct udf_map_rq *map)
 {
 	struct kernel_long_ad laarr[EXTENT_MERGE_SIZE];
 	struct extent_position prev_epos, cur_epos, next_epos;
@@ -637,21 +631,20 @@ static sector_t inode_getblk(struct inode *inode, sector_t block,
 	struct kernel_lb_addr eloc, tmpeloc;
 	int c = 1;
 	loff_t lbcount = 0, b_off = 0;
-	udf_pblk_t newblocknum, newblock;
+	udf_pblk_t newblocknum;
 	sector_t offset = 0;
 	int8_t etype;
 	struct udf_inode_info *iinfo = UDF_I(inode);
 	udf_pblk_t goal = 0, pgoal = iinfo->i_location.logicalBlockNum;
 	int lastblock = 0;
 	bool isBeyondEOF;
+	int ret = 0;
 
-	*err = 0;
-	*new = 0;
 	prev_epos.offset = udf_file_entry_alloc_offset(inode);
 	prev_epos.block = iinfo->i_location;
 	prev_epos.bh = NULL;
 	cur_epos = next_epos = prev_epos;
-	b_off = (loff_t)block << inode->i_sb->s_blocksize_bits;
+	b_off = (loff_t)map->lblk << inode->i_sb->s_blocksize_bits;
 
 	/* find the extent which contains the block we are looking for.
 	   alternate between laarr[0] and laarr[1] for locations of the
@@ -715,13 +708,13 @@ static sector_t inode_getblk(struct inode *inode, sector_t block,
 				      inode->i_sb->s_blocksize);
 			udf_write_aext(inode, &cur_epos, &eloc, elen, 1);
 		}
-		newblock = udf_get_lb_pblock(inode->i_sb, &eloc, offset);
+		map->oflags = UDF_BLK_MAPPED;
+		map->pblk = udf_get_lb_pblock(inode->i_sb, &eloc, offset);
 		goto out_free;
 	}
 
 	/* Are we beyond EOF and preallocated extent? */
 	if (etype == -1) {
-		int ret;
 		loff_t hole_len;
 
 		isBeyondEOF = true;
@@ -741,11 +734,8 @@ static sector_t inode_getblk(struct inode *inode, sector_t block,
 		/* Create extents for the hole between EOF and offset */
 		hole_len = (loff_t)offset << inode->i_blkbits;
 		ret = udf_do_extend_file(inode, &prev_epos, laarr, hole_len);
-		if (ret < 0) {
-			*err = ret;
-			newblock = 0;
+		if (ret < 0)
 			goto out_free;
-		}
 		c = 0;
 		offset = 0;
 		count += ret;
@@ -795,7 +785,7 @@ static sector_t inode_getblk(struct inode *inode, sector_t block,
 	if ((laarr[c].extLength >> 30) == (EXT_NOT_RECORDED_ALLOCATED >> 30))
 		newblocknum = laarr[c].extLocation.logicalBlockNum + offset;
 	else { /* otherwise, allocate a new block */
-		if (iinfo->i_next_alloc_block == block)
+		if (iinfo->i_next_alloc_block == map->lblk)
 			goal = iinfo->i_next_alloc_goal;
 
 		if (!goal) {
@@ -805,12 +795,9 @@ static sector_t inode_getblk(struct inode *inode, sector_t block,
 
 		newblocknum = udf_new_block(inode->i_sb, inode,
 				iinfo->i_location.partitionReferenceNum,
-				goal, err);
-		if (!newblocknum) {
-			*err = -ENOSPC;
-			newblock = 0;
+				goal, &ret);
+		if (!newblocknum)
 			goto out_free;
-		}
 		if (isBeyondEOF)
 			iinfo->i_lenExtents += inode->i_sb->s_blocksize;
 	}
@@ -834,30 +821,31 @@ static sector_t inode_getblk(struct inode *inode, sector_t block,
 	/* write back the new extents, inserting new extents if the new number
 	 * of extents is greater than the old number, and deleting extents if
 	 * the new number of extents is less than the old number */
-	*err = udf_update_extents(inode, laarr, startnum, endnum, &prev_epos);
-	if (*err < 0)
+	ret = udf_update_extents(inode, laarr, startnum, endnum, &prev_epos);
+	if (ret < 0)
 		goto out_free;
 
-	newblock = udf_get_pblock(inode->i_sb, newblocknum,
+	map->pblk = udf_get_pblock(inode->i_sb, newblocknum,
 				iinfo->i_location.partitionReferenceNum, 0);
-	if (!newblock) {
-		*err = -EIO;
+	if (!map->pblk) {
+		ret = -EFSCORRUPTED;
 		goto out_free;
 	}
-	*new = 1;
-	iinfo->i_next_alloc_block = block + 1;
-	iinfo->i_next_alloc_goal = newblocknum + 1;
+	map->oflags = UDF_BLK_NEW | UDF_BLK_MAPPED;
+	iinfo->i_next_alloc_block = map->lblk + 1;
+	iinfo->i_next_alloc_goal = map->pblk + 1;
 	inode->i_ctime = current_time(inode);
 
 	if (IS_SYNC(inode))
 		udf_sync_inode(inode);
 	else
 		mark_inode_dirty(inode);
+	ret = 0;
 out_free:
 	brelse(prev_epos.bh);
 	brelse(cur_epos.bh);
 	brelse(next_epos.bh);
-	return newblock;
+	return ret;
 }
 
 static void udf_split_extents(struct inode *inode, int *c, int offset,
-- 
2.35.3


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH 11/22] udf: Add flag to disable block preallocation
  2023-01-24 12:17 [PATCH 0/22] udf: Fix couple of preallocation related bugs Jan Kara
                   ` (9 preceding siblings ...)
  2023-01-24 12:17 ` [PATCH 10/22] udf: Pass mapping request into inode_getblk() Jan Kara
@ 2023-01-24 12:17 ` Jan Kara
  2023-01-24 12:17 ` [PATCH 12/22] udf: Use udf_map_block() in udf_getblk() Jan Kara
                   ` (10 subsequent siblings)
  21 siblings, 0 replies; 23+ messages in thread
From: Jan Kara @ 2023-01-24 12:17 UTC (permalink / raw)
  To: linux-fsdevel; +Cc: Jan Kara

In some cases we don't want to create block preallocation when
allocating blocks. Add a flag to udf_map_rq controlling the behavior.

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

diff --git a/fs/udf/inode.c b/fs/udf/inode.c
index 5c6725a5bb88..daacb793f6f1 100644
--- a/fs/udf/inode.c
+++ b/fs/udf/inode.c
@@ -322,7 +322,8 @@ int udf_expand_file_adinicb(struct inode *inode)
 	return err;
 }
 
-#define UDF_MAP_CREATE	0x01	/* Mapping can allocate new blocks */
+#define UDF_MAP_CREATE		0x01	/* Mapping can allocate new blocks */
+#define UDF_MAP_NOPREALLOC	0x02	/* Do not preallocate blocks */
 
 #define UDF_BLK_MAPPED	0x01	/* Block was successfully mapped */
 #define UDF_BLK_NEW	0x02	/* Block was freshly allocated */
@@ -381,6 +382,14 @@ static int udf_get_block(struct inode *inode, sector_t block,
 		.iflags = create ? UDF_MAP_CREATE : 0,
 	};
 
+	/*
+	 * We preallocate blocks only for regular files. It also makes sense
+	 * for directories but there's a problem when to drop the
+	 * preallocation. We might use some delayed work for that but I feel
+	 * it's overengineering for a filesystem like UDF.
+	 */
+	if (!S_ISREG(inode->i_mode))
+		map.iflags |= UDF_MAP_NOPREALLOC;
 	err = udf_map_block(inode, &map);
 	if (err < 0)
 		return err;
@@ -808,11 +817,7 @@ static int inode_getblk(struct inode *inode, struct udf_map_rq *map)
 	 * block */
 	udf_split_extents(inode, &c, offset, newblocknum, laarr, &endnum);
 
-	/* We preallocate blocks only for regular files. It also makes sense
-	 * for directories but there's a problem when to drop the
-	 * preallocation. We might use some delayed work for that but I feel
-	 * it's overengineering for a filesystem like UDF. */
-	if (S_ISREG(inode->i_mode))
+	if (!(map->iflags & UDF_MAP_NOPREALLOC))
 		udf_prealloc_extents(inode, c, lastblock, laarr, &endnum);
 
 	/* merge any continuous blocks in laarr */
-- 
2.35.3


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH 12/22] udf: Use udf_map_block() in udf_getblk()
  2023-01-24 12:17 [PATCH 0/22] udf: Fix couple of preallocation related bugs Jan Kara
                   ` (10 preceding siblings ...)
  2023-01-24 12:17 ` [PATCH 11/22] udf: Add flag to disable block preallocation Jan Kara
@ 2023-01-24 12:17 ` Jan Kara
  2023-01-24 12:17 ` [PATCH 13/22] udf: Fold udf_getblk() into udf_bread() Jan Kara
                   ` (9 subsequent siblings)
  21 siblings, 0 replies; 23+ messages in thread
From: Jan Kara @ 2023-01-24 12:17 UTC (permalink / raw)
  To: linux-fsdevel; +Cc: Jan Kara

Use the new function udf_map_block() in udf_getblk().

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

diff --git a/fs/udf/inode.c b/fs/udf/inode.c
index daacb793f6f1..4e6b855ffd02 100644
--- a/fs/udf/inode.c
+++ b/fs/udf/inode.c
@@ -405,14 +405,15 @@ static struct buffer_head *udf_getblk(struct inode *inode, udf_pblk_t block,
 				      int create, int *err)
 {
 	struct buffer_head *bh;
-	struct buffer_head dummy;
-
-	dummy.b_state = 0;
-	dummy.b_blocknr = -1000;
-	*err = udf_get_block(inode, block, &dummy, create);
-	if (!*err && buffer_mapped(&dummy)) {
-		bh = sb_getblk(inode->i_sb, dummy.b_blocknr);
-		if (buffer_new(&dummy)) {
+	struct udf_map_rq map = {
+		.lblk = block,
+		.iflags = create ? UDF_MAP_CREATE : 0,
+	};
+
+	*err = udf_map_block(inode, &map);
+	if (!*err && map.oflags & UDF_BLK_MAPPED) {
+		bh = sb_getblk(inode->i_sb, map.pblk);
+		if (map.oflags & UDF_BLK_NEW) {
 			lock_buffer(bh);
 			memset(bh->b_data, 0x00, inode->i_sb->s_blocksize);
 			set_buffer_uptodate(bh);
-- 
2.35.3


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH 13/22] udf: Fold udf_getblk() into udf_bread()
  2023-01-24 12:17 [PATCH 0/22] udf: Fix couple of preallocation related bugs Jan Kara
                   ` (11 preceding siblings ...)
  2023-01-24 12:17 ` [PATCH 12/22] udf: Use udf_map_block() in udf_getblk() Jan Kara
@ 2023-01-24 12:17 ` Jan Kara
  2023-01-24 12:18 ` [PATCH 14/22] udf: Protect rename against modification of moved directory Jan Kara
                   ` (8 subsequent siblings)
  21 siblings, 0 replies; 23+ messages in thread
From: Jan Kara @ 2023-01-24 12:17 UTC (permalink / raw)
  To: linux-fsdevel; +Cc: Jan Kara

udf_getblk() has a single call site. Fold it there.

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

diff --git a/fs/udf/inode.c b/fs/udf/inode.c
index 4e6b855ffd02..4554d1e54eb3 100644
--- a/fs/udf/inode.c
+++ b/fs/udf/inode.c
@@ -401,31 +401,6 @@ static int udf_get_block(struct inode *inode, sector_t block,
 	return 0;
 }
 
-static struct buffer_head *udf_getblk(struct inode *inode, udf_pblk_t block,
-				      int create, int *err)
-{
-	struct buffer_head *bh;
-	struct udf_map_rq map = {
-		.lblk = block,
-		.iflags = create ? UDF_MAP_CREATE : 0,
-	};
-
-	*err = udf_map_block(inode, &map);
-	if (!*err && map.oflags & UDF_BLK_MAPPED) {
-		bh = sb_getblk(inode->i_sb, map.pblk);
-		if (map.oflags & UDF_BLK_NEW) {
-			lock_buffer(bh);
-			memset(bh->b_data, 0x00, inode->i_sb->s_blocksize);
-			set_buffer_uptodate(bh);
-			unlock_buffer(bh);
-			mark_buffer_dirty_inode(bh, inode);
-		}
-		return bh;
-	}
-
-	return NULL;
-}
-
 /* Extend the file with new blocks totaling 'new_block_bytes',
  * return the number of extents added
  */
@@ -1140,10 +1115,28 @@ struct buffer_head *udf_bread(struct inode *inode, udf_pblk_t block,
 			      int create, int *err)
 {
 	struct buffer_head *bh = NULL;
+	struct udf_map_rq map = {
+		.lblk = block,
+		.iflags = create ? UDF_MAP_CREATE : 0,
+	};
 
-	bh = udf_getblk(inode, block, create, err);
-	if (!bh)
+	*err = udf_map_block(inode, &map);
+	if (*err || !(map.oflags & UDF_BLK_MAPPED))
+		return NULL;
+	
+	bh = sb_getblk(inode->i_sb, map.pblk);
+	if (!bh) {
+		*err = -ENOMEM;
 		return NULL;
+	}
+	if (map.oflags & UDF_BLK_NEW) {
+		lock_buffer(bh);
+		memset(bh->b_data, 0x00, inode->i_sb->s_blocksize);
+		set_buffer_uptodate(bh);
+		unlock_buffer(bh);
+		mark_buffer_dirty_inode(bh, inode);
+		return bh;
+	}
 
 	if (bh_read(bh, 0) >= 0)
 		return bh;
-- 
2.35.3


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH 14/22] udf: Protect rename against modification of moved directory
  2023-01-24 12:17 [PATCH 0/22] udf: Fix couple of preallocation related bugs Jan Kara
                   ` (12 preceding siblings ...)
  2023-01-24 12:17 ` [PATCH 13/22] udf: Fold udf_getblk() into udf_bread() Jan Kara
@ 2023-01-24 12:18 ` Jan Kara
  2023-01-24 12:18 ` [PATCH 15/22] udf: Push i_data_sem locking into udf_expand_file_adinicb() Jan Kara
                   ` (7 subsequent siblings)
  21 siblings, 0 replies; 23+ messages in thread
From: Jan Kara @ 2023-01-24 12:18 UTC (permalink / raw)
  To: linux-fsdevel; +Cc: Jan Kara, syzbot+aebf90eea2671c43112a

When we are renaming a directory to a different directory, we need to
update '..' entry in the moved directory. However nothing prevents moved
directory from being modified and even converted from the in-ICB format
to the normal format which results in a crash. Fix the problem by
locking the moved directory.

Reported-by: syzbot+aebf90eea2671c43112a@syzkaller.appspotmail.com
Signed-off-by: Jan Kara <jack@suse.cz>
---
 fs/udf/namei.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/fs/udf/namei.c b/fs/udf/namei.c
index 49fab30afff3..1b0f4c600b63 100644
--- a/fs/udf/namei.c
+++ b/fs/udf/namei.c
@@ -782,6 +782,11 @@ static int udf_rename(struct user_namespace *mnt_userns, struct inode *old_dir,
 			if (!empty_dir(new_inode))
 				goto out_oiter;
 		}
+		/*
+		 * We need to protect against old_inode getting converted from
+		 * ICB to normal directory.
+		 */
+		inode_lock_nested(old_inode, I_MUTEX_NONDIR2);
 		retval = udf_fiiter_find_entry(old_inode, &dotdot_name,
 					       &diriter);
 		if (retval == -ENOENT) {
@@ -790,8 +795,10 @@ static int udf_rename(struct user_namespace *mnt_userns, struct inode *old_dir,
 				old_inode->i_ino);
 			retval = -EFSCORRUPTED;
 		}
-		if (retval)
+		if (retval) {
+			inode_unlock(old_inode);
 			goto out_oiter;
+		}
 		has_diriter = true;
 		tloc = lelb_to_cpu(diriter.fi.icb.extLocation);
 		if (udf_get_lb_pblock(old_inode->i_sb, &tloc, 0) !=
@@ -869,6 +876,7 @@ static int udf_rename(struct user_namespace *mnt_userns, struct inode *old_dir,
 			       udf_dir_entry_len(&diriter.fi));
 		udf_fiiter_write_fi(&diriter, NULL);
 		udf_fiiter_release(&diriter);
+		inode_unlock(old_inode);
 
 		inode_dec_link_count(old_dir);
 		if (new_inode)
@@ -880,8 +888,10 @@ static int udf_rename(struct user_namespace *mnt_userns, struct inode *old_dir,
 	}
 	return 0;
 out_oiter:
-	if (has_diriter)
+	if (has_diriter) {
 		udf_fiiter_release(&diriter);
+		inode_unlock(old_inode);
+	}
 	udf_fiiter_release(&oiter);
 
 	return retval;
-- 
2.35.3


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH 15/22] udf: Push i_data_sem locking into udf_expand_file_adinicb()
  2023-01-24 12:17 [PATCH 0/22] udf: Fix couple of preallocation related bugs Jan Kara
                   ` (13 preceding siblings ...)
  2023-01-24 12:18 ` [PATCH 14/22] udf: Protect rename against modification of moved directory Jan Kara
@ 2023-01-24 12:18 ` Jan Kara
  2023-01-24 12:18 ` [PATCH 16/22] udf: Push i_data_sem locking into udf_extend_file() Jan Kara
                   ` (6 subsequent siblings)
  21 siblings, 0 replies; 23+ messages in thread
From: Jan Kara @ 2023-01-24 12:18 UTC (permalink / raw)
  To: linux-fsdevel; +Cc: Jan Kara

The checks we do in udf_setsize() and udf_file_write_iter() are safe to
do only with i_rwsem locked as it stabilizes both file type and file
size. Hence we don't need to lock i_data_sem before we enter
udf_expand_file_adinicb() which simplifies the locking somewhat.

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

diff --git a/fs/udf/file.c b/fs/udf/file.c
index 8be51161f3e5..60524814c594 100644
--- a/fs/udf/file.c
+++ b/fs/udf/file.c
@@ -148,7 +148,6 @@ static ssize_t udf_file_write_iter(struct kiocb *iocb, struct iov_iter *from)
 	if (retval <= 0)
 		goto out;
 
-	down_write(&iinfo->i_data_sem);
 	if (iinfo->i_alloc_type == ICBTAG_FLAG_AD_IN_ICB &&
 	    inode->i_sb->s_blocksize < (udf_file_entry_alloc_offset(inode) +
 				 iocb->ki_pos + iov_iter_count(from))) {
@@ -158,15 +157,15 @@ static ssize_t udf_file_write_iter(struct kiocb *iocb, struct iov_iter *from)
 			udf_debug("udf_expand_adinicb: err=%d\n", err);
 			return err;
 		}
-	} else
-		up_write(&iinfo->i_data_sem);
+	}
 
 	retval = __generic_file_write_iter(iocb, from);
 out:
-	down_write(&iinfo->i_data_sem);
-	if (iinfo->i_alloc_type == ICBTAG_FLAG_AD_IN_ICB && retval > 0)
+	if (iinfo->i_alloc_type == ICBTAG_FLAG_AD_IN_ICB && retval > 0) {
+		down_write(&iinfo->i_data_sem);
 		iinfo->i_lenAlloc = inode->i_size;
-	up_write(&iinfo->i_data_sem);
+		up_write(&iinfo->i_data_sem);
+	}
 	inode_unlock(inode);
 
 	if (retval > 0) {
diff --git a/fs/udf/inode.c b/fs/udf/inode.c
index 4554d1e54eb3..b13c35335dd1 100644
--- a/fs/udf/inode.c
+++ b/fs/udf/inode.c
@@ -247,7 +247,6 @@ const struct address_space_operations udf_aops = {
 /*
  * Expand file stored in ICB to a normal one-block-file
  *
- * This function requires i_data_sem for writing and releases it.
  * This function requires i_mutex held
  */
 int udf_expand_file_adinicb(struct inode *inode)
@@ -259,6 +258,7 @@ int udf_expand_file_adinicb(struct inode *inode)
 
 	WARN_ON_ONCE(!inode_is_locked(inode));
 	if (!iinfo->i_lenAlloc) {
+		down_write(&iinfo->i_data_sem);
 		if (UDF_QUERY_FLAG(inode->i_sb, UDF_FLAG_USE_SHORT_AD))
 			iinfo->i_alloc_type = ICBTAG_FLAG_AD_SHORT;
 		else
@@ -269,11 +269,6 @@ int udf_expand_file_adinicb(struct inode *inode)
 		mark_inode_dirty(inode);
 		return 0;
 	}
-	/*
-	 * Release i_data_sem so that we can lock a page - page lock ranks
-	 * above i_data_sem. i_mutex still protects us against file changes.
-	 */
-	up_write(&iinfo->i_data_sem);
 
 	page = find_or_create_page(inode->i_mapping, 0, GFP_NOFS);
 	if (!page)
@@ -1160,19 +1155,18 @@ int udf_setsize(struct inode *inode, loff_t newsize)
 
 	iinfo = UDF_I(inode);
 	if (newsize > inode->i_size) {
-		down_write(&iinfo->i_data_sem);
 		if (iinfo->i_alloc_type == ICBTAG_FLAG_AD_IN_ICB) {
-			if (bsize <
+			if (bsize >=
 			    (udf_file_entry_alloc_offset(inode) + newsize)) {
-				err = udf_expand_file_adinicb(inode);
-				if (err)
-					return err;
 				down_write(&iinfo->i_data_sem);
-			} else {
 				iinfo->i_lenAlloc = newsize;
 				goto set_size;
 			}
+			err = udf_expand_file_adinicb(inode);
+			if (err)
+				return err;
 		}
+		down_write(&iinfo->i_data_sem);
 		err = udf_extend_file(inode, newsize);
 		if (err) {
 			up_write(&iinfo->i_data_sem);
-- 
2.35.3


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH 16/22] udf: Push i_data_sem locking into udf_extend_file()
  2023-01-24 12:17 [PATCH 0/22] udf: Fix couple of preallocation related bugs Jan Kara
                   ` (14 preceding siblings ...)
  2023-01-24 12:18 ` [PATCH 15/22] udf: Push i_data_sem locking into udf_expand_file_adinicb() Jan Kara
@ 2023-01-24 12:18 ` Jan Kara
  2023-01-24 12:18 ` [PATCH 17/22] udf: Simplify error handling in udf_file_write_iter() Jan Kara
                   ` (5 subsequent siblings)
  21 siblings, 0 replies; 23+ messages in thread
From: Jan Kara @ 2023-01-24 12:18 UTC (permalink / raw)
  To: linux-fsdevel; +Cc: Jan Kara

Push i_data_sem locking into udf_extend_file(). It somewhat simplifies
the code around it.

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

diff --git a/fs/udf/inode.c b/fs/udf/inode.c
index b13c35335dd1..3ffeb5651689 100644
--- a/fs/udf/inode.c
+++ b/fs/udf/inode.c
@@ -555,6 +555,7 @@ static int udf_extend_file(struct inode *inode, loff_t newsize)
 	else
 		BUG();
 
+	down_write(&iinfo->i_data_sem);
 	/*
 	 * When creating hole in file, just don't bother with preserving
 	 * preallocation. It likely won't be very useful anyway.
@@ -599,6 +600,7 @@ static int udf_extend_file(struct inode *inode, loff_t newsize)
 	err = 0;
 out:
 	brelse(epos.bh);
+	up_write(&iinfo->i_data_sem);
 	return err;
 }
 
@@ -1160,20 +1162,17 @@ int udf_setsize(struct inode *inode, loff_t newsize)
 			    (udf_file_entry_alloc_offset(inode) + newsize)) {
 				down_write(&iinfo->i_data_sem);
 				iinfo->i_lenAlloc = newsize;
+				up_write(&iinfo->i_data_sem);
 				goto set_size;
 			}
 			err = udf_expand_file_adinicb(inode);
 			if (err)
 				return err;
 		}
-		down_write(&iinfo->i_data_sem);
 		err = udf_extend_file(inode, newsize);
-		if (err) {
-			up_write(&iinfo->i_data_sem);
+		if (err)
 			return err;
-		}
 set_size:
-		up_write(&iinfo->i_data_sem);
 		truncate_setsize(inode, newsize);
 	} else {
 		if (iinfo->i_alloc_type == ICBTAG_FLAG_AD_IN_ICB) {
-- 
2.35.3


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH 17/22] udf: Simplify error handling in udf_file_write_iter()
  2023-01-24 12:17 [PATCH 0/22] udf: Fix couple of preallocation related bugs Jan Kara
                   ` (15 preceding siblings ...)
  2023-01-24 12:18 ` [PATCH 16/22] udf: Push i_data_sem locking into udf_extend_file() Jan Kara
@ 2023-01-24 12:18 ` Jan Kara
  2023-01-24 12:18 ` [PATCH 18/22] udf: Protect truncate and file type conversion with invalidate_lock Jan Kara
                   ` (4 subsequent siblings)
  21 siblings, 0 replies; 23+ messages in thread
From: Jan Kara @ 2023-01-24 12:18 UTC (permalink / raw)
  To: linux-fsdevel; +Cc: Jan Kara

When udf_expand_file_adinicb() fails, we can now use the standard exit
path instead of implementing our own.

Signed-off-by: Jan Kara <jack@suse.cz>
---
 fs/udf/file.c | 10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/fs/udf/file.c b/fs/udf/file.c
index 60524814c594..596d703fb6c8 100644
--- a/fs/udf/file.c
+++ b/fs/udf/file.c
@@ -140,7 +140,6 @@ static ssize_t udf_file_write_iter(struct kiocb *iocb, struct iov_iter *from)
 	struct file *file = iocb->ki_filp;
 	struct inode *inode = file_inode(file);
 	struct udf_inode_info *iinfo = UDF_I(inode);
-	int err;
 
 	inode_lock(inode);
 
@@ -151,12 +150,9 @@ static ssize_t udf_file_write_iter(struct kiocb *iocb, struct iov_iter *from)
 	if (iinfo->i_alloc_type == ICBTAG_FLAG_AD_IN_ICB &&
 	    inode->i_sb->s_blocksize < (udf_file_entry_alloc_offset(inode) +
 				 iocb->ki_pos + iov_iter_count(from))) {
-		err = udf_expand_file_adinicb(inode);
-		if (err) {
-			inode_unlock(inode);
-			udf_debug("udf_expand_adinicb: err=%d\n", err);
-			return err;
-		}
+		retval = udf_expand_file_adinicb(inode);
+		if (retval)
+			goto out;
 	}
 
 	retval = __generic_file_write_iter(iocb, from);
-- 
2.35.3


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH 18/22] udf: Protect truncate and file type conversion with invalidate_lock
  2023-01-24 12:17 [PATCH 0/22] udf: Fix couple of preallocation related bugs Jan Kara
                   ` (16 preceding siblings ...)
  2023-01-24 12:18 ` [PATCH 17/22] udf: Simplify error handling in udf_file_write_iter() Jan Kara
@ 2023-01-24 12:18 ` Jan Kara
  2023-01-24 12:18 ` [PATCH 19/22] udf: Allocate blocks on write page fault Jan Kara
                   ` (3 subsequent siblings)
  21 siblings, 0 replies; 23+ messages in thread
From: Jan Kara @ 2023-01-24 12:18 UTC (permalink / raw)
  To: linux-fsdevel; +Cc: Jan Kara

Protect truncate and file type conversion in udf_file_write_iter() with
invalidate lock. That will allow us to serialize these paths with page
faults so that the page fault can determine the file type in a racefree
way.

Signed-off-by: Jan Kara <jack@suse.cz>
---
 fs/udf/file.c  |  2 ++
 fs/udf/inode.c | 15 +++++++++------
 2 files changed, 11 insertions(+), 6 deletions(-)

diff --git a/fs/udf/file.c b/fs/udf/file.c
index 596d703fb6c8..cf050bdffd9e 100644
--- a/fs/udf/file.c
+++ b/fs/udf/file.c
@@ -150,7 +150,9 @@ static ssize_t udf_file_write_iter(struct kiocb *iocb, struct iov_iter *from)
 	if (iinfo->i_alloc_type == ICBTAG_FLAG_AD_IN_ICB &&
 	    inode->i_sb->s_blocksize < (udf_file_entry_alloc_offset(inode) +
 				 iocb->ki_pos + iov_iter_count(from))) {
+		filemap_invalidate_lock(inode->i_mapping);
 		retval = udf_expand_file_adinicb(inode);
+		filemap_invalidate_unlock(inode->i_mapping);
 		if (retval)
 			goto out;
 	}
diff --git a/fs/udf/inode.c b/fs/udf/inode.c
index 3ffeb5651689..7109adcceefe 100644
--- a/fs/udf/inode.c
+++ b/fs/udf/inode.c
@@ -1145,7 +1145,7 @@ struct buffer_head *udf_bread(struct inode *inode, udf_pblk_t block,
 
 int udf_setsize(struct inode *inode, loff_t newsize)
 {
-	int err;
+	int err = 0;
 	struct udf_inode_info *iinfo;
 	unsigned int bsize = i_blocksize(inode);
 
@@ -1155,6 +1155,7 @@ int udf_setsize(struct inode *inode, loff_t newsize)
 	if (IS_APPEND(inode) || IS_IMMUTABLE(inode))
 		return -EPERM;
 
+	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) {
@@ -1167,11 +1168,11 @@ int udf_setsize(struct inode *inode, loff_t newsize)
 			}
 			err = udf_expand_file_adinicb(inode);
 			if (err)
-				return err;
+				goto out_unlock;
 		}
 		err = udf_extend_file(inode, newsize);
 		if (err)
-			return err;
+			goto out_unlock;
 set_size:
 		truncate_setsize(inode, newsize);
 	} else {
@@ -1189,14 +1190,14 @@ int udf_setsize(struct inode *inode, loff_t newsize)
 		err = block_truncate_page(inode->i_mapping, newsize,
 					  udf_get_block);
 		if (err)
-			return err;
+			goto out_unlock;
 		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)
-			return err;
+			goto out_unlock;
 	}
 update_time:
 	inode->i_mtime = inode->i_ctime = current_time(inode);
@@ -1204,7 +1205,9 @@ int udf_setsize(struct inode *inode, loff_t newsize)
 		udf_sync_inode(inode);
 	else
 		mark_inode_dirty(inode);
-	return 0;
+out_unlock:
+	filemap_invalidate_unlock(inode->i_mapping);
+	return err;
 }
 
 /*
-- 
2.35.3


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH 19/22] udf: Allocate blocks on write page fault
  2023-01-24 12:17 [PATCH 0/22] udf: Fix couple of preallocation related bugs Jan Kara
                   ` (17 preceding siblings ...)
  2023-01-24 12:18 ` [PATCH 18/22] udf: Protect truncate and file type conversion with invalidate_lock Jan Kara
@ 2023-01-24 12:18 ` Jan Kara
  2023-01-24 12:18 ` [PATCH 20/22] udf: Do not allocate blocks on page writeback Jan Kara
                   ` (2 subsequent siblings)
  21 siblings, 0 replies; 23+ messages in thread
From: Jan Kara @ 2023-01-24 12:18 UTC (permalink / raw)
  To: linux-fsdevel; +Cc: Jan Kara

Currently if file with holes is mapped, udf allocates blocks for dirtied
pages during page writeback. This however creates problems when to
truncate final extent to proper size and currently we leave the last
extent untruncated which violates UDF standard. So allocate blocks on
write page fault instead. In that case the last extent gets truncated
the file is closed and everything is happy.

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

diff --git a/fs/udf/file.c b/fs/udf/file.c
index cf050bdffd9e..322115c8369d 100644
--- a/fs/udf/file.c
+++ b/fs/udf/file.c
@@ -134,6 +134,57 @@ const struct address_space_operations udf_adinicb_aops = {
 	.direct_IO	= udf_adinicb_direct_IO,
 };
 
+static vm_fault_t udf_page_mkwrite(struct vm_fault *vmf)
+{
+	struct vm_area_struct *vma = vmf->vma;
+	struct inode *inode = file_inode(vma->vm_file);
+	struct address_space *mapping = inode->i_mapping;
+	struct page *page = vmf->page;
+	loff_t size;
+	unsigned int end;
+	vm_fault_t ret = VM_FAULT_LOCKED;
+	int err;
+
+	sb_start_pagefault(inode->i_sb);
+	file_update_time(vma->vm_file);
+	filemap_invalidate_lock_shared(mapping);
+	lock_page(page);
+	size = i_size_read(inode);
+	if (page->mapping != inode->i_mapping || page_offset(page) >= size) {
+		unlock_page(page);
+		ret = VM_FAULT_NOPAGE;
+		goto out_unlock;
+	}
+	/* Space is already allocated for in-ICB file */
+	if (UDF_I(inode)->i_alloc_type == ICBTAG_FLAG_AD_IN_ICB)
+		goto out_dirty;
+	if (page->index == size >> PAGE_SHIFT)
+		end = size & ~PAGE_MASK;
+	else
+		end = PAGE_SIZE;
+	err = __block_write_begin(page, 0, end, udf_get_block);
+	if (!err)
+		err = block_commit_write(page, 0, end);
+	if (err < 0) {
+		unlock_page(page);
+		ret = block_page_mkwrite_return(err);
+		goto out_unlock;
+	}
+out_dirty:
+	set_page_dirty(page);
+	wait_for_stable_page(page);
+out_unlock:
+	filemap_invalidate_unlock_shared(mapping);
+	sb_end_pagefault(inode->i_sb);
+	return ret;
+}
+
+static const struct vm_operations_struct udf_file_vm_ops = {
+	.fault		= filemap_fault,
+	.map_pages	= filemap_map_pages,
+	.page_mkwrite	= udf_page_mkwrite,
+};
+
 static ssize_t udf_file_write_iter(struct kiocb *iocb, struct iov_iter *from)
 {
 	ssize_t retval;
@@ -238,11 +289,19 @@ static int udf_release_file(struct inode *inode, struct file *filp)
 	return 0;
 }
 
+static int udf_file_mmap(struct file *file, struct vm_area_struct *vma)
+{
+	file_accessed(file);
+	vma->vm_ops = &udf_file_vm_ops;
+
+	return 0;
+}
+
 const struct file_operations udf_file_operations = {
 	.read_iter		= generic_file_read_iter,
 	.unlocked_ioctl		= udf_ioctl,
 	.open			= generic_file_open,
-	.mmap			= generic_file_mmap,
+	.mmap			= udf_file_mmap,
 	.write_iter		= udf_file_write_iter,
 	.release		= udf_release_file,
 	.fsync			= generic_file_fsync,
diff --git a/fs/udf/inode.c b/fs/udf/inode.c
index 7109adcceefe..7fd0aa2439e9 100644
--- a/fs/udf/inode.c
+++ b/fs/udf/inode.c
@@ -68,7 +68,6 @@ static void udf_prealloc_extents(struct inode *, int, int,
 static void udf_merge_extents(struct inode *, struct kernel_long_ad *, int *);
 static int udf_update_extents(struct inode *, struct kernel_long_ad *, int,
 			      int, struct extent_position *);
-static int udf_get_block(struct inode *, sector_t, struct buffer_head *, int);
 
 static void __udf_clear_extent_cache(struct inode *inode)
 {
diff --git a/fs/udf/udfdecl.h b/fs/udf/udfdecl.h
index 98b4d89b4368..5ba59ab90d48 100644
--- a/fs/udf/udfdecl.h
+++ b/fs/udf/udfdecl.h
@@ -160,6 +160,7 @@ extern void udf_evict_inode(struct inode *);
 extern int udf_write_inode(struct inode *, struct writeback_control *wbc);
 extern int8_t inode_bmap(struct inode *, sector_t, struct extent_position *,
 			 struct kernel_lb_addr *, uint32_t *, sector_t *);
+int udf_get_block(struct inode *, sector_t, struct buffer_head *, int);
 extern int udf_setup_indirect_aext(struct inode *inode, udf_pblk_t block,
 				   struct extent_position *epos);
 extern int __udf_add_aext(struct inode *inode, struct extent_position *epos,
-- 
2.35.3


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH 20/22] udf: Do not allocate blocks on page writeback
  2023-01-24 12:17 [PATCH 0/22] udf: Fix couple of preallocation related bugs Jan Kara
                   ` (18 preceding siblings ...)
  2023-01-24 12:18 ` [PATCH 19/22] udf: Allocate blocks on write page fault Jan Kara
@ 2023-01-24 12:18 ` Jan Kara
  2023-01-24 12:18 ` [PATCH 21/22] udf: Fix file corruption when appending just after end of preallocated extent Jan Kara
  2023-01-24 12:18 ` [PATCH 22/22] udf: Fix off-by-one error when discarding preallocation Jan Kara
  21 siblings, 0 replies; 23+ messages in thread
From: Jan Kara @ 2023-01-24 12:18 UTC (permalink / raw)
  To: linux-fsdevel; +Cc: Jan Kara

Now when we allocate blocks on write page fault there should be no block
allocation happening on page writeback. So just ignore the 'create' flag
passed to udf_get_block(). Note that we can spot dirty buffers without
underlying blocks allocated in writeback when we race with expanding
truncate. However in that case these buffers do not contain valid data
so we can safely ignore them and we would just create ourselves problem
when to trim the tail extent.

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

diff --git a/fs/udf/inode.c b/fs/udf/inode.c
index 7fd0aa2439e9..8f55b37ddcad 100644
--- a/fs/udf/inode.c
+++ b/fs/udf/inode.c
@@ -68,6 +68,8 @@ static void udf_prealloc_extents(struct inode *, int, int,
 static void udf_merge_extents(struct inode *, struct kernel_long_ad *, int *);
 static int udf_update_extents(struct inode *, struct kernel_long_ad *, int,
 			      int, struct extent_position *);
+static int udf_get_block_wb(struct inode *inode, sector_t block,
+			    struct buffer_head *bh_result, int create);
 
 static void __udf_clear_extent_cache(struct inode *inode)
 {
@@ -186,7 +188,7 @@ static void udf_write_failed(struct address_space *mapping, loff_t to)
 static int udf_writepages(struct address_space *mapping,
 			struct writeback_control *wbc)
 {
-	return mpage_writepages(mapping, wbc, udf_get_block);
+	return mpage_writepages(mapping, wbc, udf_get_block_wb);
 }
 
 static int udf_read_folio(struct file *file, struct folio *folio)
@@ -367,23 +369,15 @@ static int udf_map_block(struct inode *inode, struct udf_map_rq *map)
 	return err;
 }
 
-static int udf_get_block(struct inode *inode, sector_t block,
-			 struct buffer_head *bh_result, int create)
+static int __udf_get_block(struct inode *inode, sector_t block,
+			   struct buffer_head *bh_result, int flags)
 {
 	int err;
 	struct udf_map_rq map = {
 		.lblk = block,
-		.iflags = create ? UDF_MAP_CREATE : 0,
+		.iflags = flags,
 	};
 
-	/*
-	 * We preallocate blocks only for regular files. It also makes sense
-	 * for directories but there's a problem when to drop the
-	 * preallocation. We might use some delayed work for that but I feel
-	 * it's overengineering for a filesystem like UDF.
-	 */
-	if (!S_ISREG(inode->i_mode))
-		map.iflags |= UDF_MAP_NOPREALLOC;
 	err = udf_map_block(inode, &map);
 	if (err < 0)
 		return err;
@@ -395,6 +389,34 @@ static int udf_get_block(struct inode *inode, sector_t block,
 	return 0;
 }
 
+int udf_get_block(struct inode *inode, sector_t block,
+		  struct buffer_head *bh_result, int create)
+{
+	int flags = create ? UDF_MAP_CREATE : 0;
+
+	/*
+	 * We preallocate blocks only for regular files. It also makes sense
+	 * for directories but there's a problem when to drop the
+	 * preallocation. We might use some delayed work for that but I feel
+	 * it's overengineering for a filesystem like UDF.
+	 */
+	if (!S_ISREG(inode->i_mode))
+		flags |= UDF_MAP_NOPREALLOC;
+	return __udf_get_block(inode, block, bh_result, flags);
+}
+
+/*
+ * We shouldn't be allocating blocks on page writeback since we allocate them
+ * on page fault. We can spot dirty buffers without allocated blocks though
+ * when truncate expands file. These however don't have valid data so we can
+ * safely ignore them. So never allocate blocks from page writeback.
+ */
+static int udf_get_block_wb(struct inode *inode, sector_t block,
+			    struct buffer_head *bh_result, int create)
+{
+	return __udf_get_block(inode, block, bh_result, 0);
+}
+
 /* Extend the file with new blocks totaling 'new_block_bytes',
  * return the number of extents added
  */
-- 
2.35.3


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH 21/22] udf: Fix file corruption when appending just after end of preallocated extent
  2023-01-24 12:17 [PATCH 0/22] udf: Fix couple of preallocation related bugs Jan Kara
                   ` (19 preceding siblings ...)
  2023-01-24 12:18 ` [PATCH 20/22] udf: Do not allocate blocks on page writeback Jan Kara
@ 2023-01-24 12:18 ` Jan Kara
  2023-01-24 12:18 ` [PATCH 22/22] udf: Fix off-by-one error when discarding preallocation Jan Kara
  21 siblings, 0 replies; 23+ messages in thread
From: Jan Kara @ 2023-01-24 12:18 UTC (permalink / raw)
  To: linux-fsdevel; +Cc: Jan Kara, stable

When we append new block just after the end of preallocated extent, the
code in inode_getblk() wrongly determined we're going to use the
preallocated extent which resulted in adding block into a wrong logical
offset in the file. Sequence like this manifests it:

xfs_io -f -c "pwrite 0x2cacf 0xd122" -c "truncate 0x2dd6f" \
  -c "pwrite 0x27fd9 0x69a9" -c "pwrite 0x32981 0x7244" <file>

The code that determined the use of preallocated extent is actually
stale because udf_do_extend_file() does not create preallocation anymore
so after calling that function we are sure there's no usable
preallocation. Just remove the faulty condition.

CC: stable@vger.kernel.org
Fixes: 16d055656814 ("udf: Discard preallocation before extending file with a hole")
Signed-off-by: Jan Kara <jack@suse.cz>
---
 fs/udf/inode.c | 24 +++++++++++-------------
 1 file changed, 11 insertions(+), 13 deletions(-)

diff --git a/fs/udf/inode.c b/fs/udf/inode.c
index 8f55b37ddcad..6826c2aa021f 100644
--- a/fs/udf/inode.c
+++ b/fs/udf/inode.c
@@ -742,19 +742,17 @@ static int inode_getblk(struct inode *inode, struct udf_map_rq *map)
 		c = 0;
 		offset = 0;
 		count += ret;
-		/* We are not covered by a preallocated extent? */
-		if ((laarr[0].extLength & UDF_EXTENT_FLAG_MASK) !=
-						EXT_NOT_RECORDED_ALLOCATED) {
-			/* Is there any real extent? - otherwise we overwrite
-			 * the fake one... */
-			if (count)
-				c = !c;
-			laarr[c].extLength = EXT_NOT_RECORDED_NOT_ALLOCATED |
-				inode->i_sb->s_blocksize;
-			memset(&laarr[c].extLocation, 0x00,
-				sizeof(struct kernel_lb_addr));
-			count++;
-		}
+		/*
+		 * Is there any real extent? - otherwise we overwrite the fake
+		 * one...
+		 */
+		if (count)
+			c = !c;
+		laarr[c].extLength = EXT_NOT_RECORDED_NOT_ALLOCATED |
+			inode->i_sb->s_blocksize;
+		memset(&laarr[c].extLocation, 0x00,
+			sizeof(struct kernel_lb_addr));
+		count++;
 		endnum = c + 1;
 		lastblock = 1;
 	} else {
-- 
2.35.3


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH 22/22] udf: Fix off-by-one error when discarding preallocation
  2023-01-24 12:17 [PATCH 0/22] udf: Fix couple of preallocation related bugs Jan Kara
                   ` (20 preceding siblings ...)
  2023-01-24 12:18 ` [PATCH 21/22] udf: Fix file corruption when appending just after end of preallocated extent Jan Kara
@ 2023-01-24 12:18 ` Jan Kara
  21 siblings, 0 replies; 23+ messages in thread
From: Jan Kara @ 2023-01-24 12:18 UTC (permalink / raw)
  To: linux-fsdevel; +Cc: Jan Kara, stable

The condition determining whether the preallocation can be used had
an off-by-one error so we didn't discard preallocation when new
allocation was just following it. This can then confuse code in
inode_getblk().

CC: stable@vger.kernel.org
Fixes: 16d055656814 ("udf: Discard preallocation before extending file with a hole")
Signed-off-by: Jan Kara <jack@suse.cz>
---
 fs/udf/inode.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/udf/inode.c b/fs/udf/inode.c
index 6826c2aa021f..15e0d9f23c06 100644
--- a/fs/udf/inode.c
+++ b/fs/udf/inode.c
@@ -361,7 +361,7 @@ static int udf_map_block(struct inode *inode, struct udf_map_rq *map)
 	 * Block beyond EOF and prealloc extents? Just discard preallocation
 	 * as it is not useful and complicates things.
 	 */
-	if (((loff_t)map->lblk) << inode->i_blkbits > iinfo->i_lenExtents)
+	if (((loff_t)map->lblk) << inode->i_blkbits >= iinfo->i_lenExtents)
 		udf_discard_prealloc(inode);
 	udf_clear_extent_cache(inode);
 	err = inode_getblk(inode, map);
-- 
2.35.3


^ permalink raw reply related	[flat|nested] 23+ messages in thread

end of thread, other threads:[~2023-01-24 12:18 UTC | newest]

Thread overview: 23+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-01-24 12:17 [PATCH 0/22] udf: Fix couple of preallocation related bugs Jan Kara
2023-01-24 12:17 ` [PATCH 01/22] udf: Unify types in anchor block detection Jan Kara
2023-01-24 12:17 ` [PATCH 02/22] udf: Drop VARCONV support Jan Kara
2023-01-24 12:17 ` [PATCH 03/22] udf: Move incrementing of goal block directly into inode_getblk() Jan Kara
2023-01-24 12:17 ` [PATCH 04/22] udf: Factor out block mapping into udf_map_block() Jan Kara
2023-01-24 12:17 ` [PATCH 05/22] udf: Use udf_bread() in udf_get_pblock_virt15() Jan Kara
2023-01-24 12:17 ` [PATCH 06/22] udf: Use udf_bread() in udf_load_vat() Jan Kara
2023-01-24 12:17 ` [PATCH 07/22] udf: Do not call udf_block_map() on ICB files Jan Kara
2023-01-24 12:17 ` [PATCH 08/22] udf: Convert udf_symlink_filler() to use udf_bread() Jan Kara
2023-01-24 12:17 ` [PATCH 09/22] udf: Fold udf_block_map() into udf_map_block() Jan Kara
2023-01-24 12:17 ` [PATCH 10/22] udf: Pass mapping request into inode_getblk() Jan Kara
2023-01-24 12:17 ` [PATCH 11/22] udf: Add flag to disable block preallocation Jan Kara
2023-01-24 12:17 ` [PATCH 12/22] udf: Use udf_map_block() in udf_getblk() Jan Kara
2023-01-24 12:17 ` [PATCH 13/22] udf: Fold udf_getblk() into udf_bread() Jan Kara
2023-01-24 12:18 ` [PATCH 14/22] udf: Protect rename against modification of moved directory Jan Kara
2023-01-24 12:18 ` [PATCH 15/22] udf: Push i_data_sem locking into udf_expand_file_adinicb() Jan Kara
2023-01-24 12:18 ` [PATCH 16/22] udf: Push i_data_sem locking into udf_extend_file() Jan Kara
2023-01-24 12:18 ` [PATCH 17/22] udf: Simplify error handling in udf_file_write_iter() Jan Kara
2023-01-24 12:18 ` [PATCH 18/22] udf: Protect truncate and file type conversion with invalidate_lock Jan Kara
2023-01-24 12:18 ` [PATCH 19/22] udf: Allocate blocks on write page fault Jan Kara
2023-01-24 12:18 ` [PATCH 20/22] udf: Do not allocate blocks on page writeback Jan Kara
2023-01-24 12:18 ` [PATCH 21/22] udf: Fix file corruption when appending just after end of preallocated extent Jan Kara
2023-01-24 12:18 ` [PATCH 22/22] udf: Fix off-by-one error when discarding preallocation Jan Kara

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.