All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/3] udf: Fix some signed/unsigned conversion issues
@ 2017-10-12 13:48 Steve Magnani
  2017-10-12 13:48 ` [PATCH v2 1/3] udf: Fix 64-bit sign extension issues affecting blocks > 0x7FFFFFFF Steve Magnani
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Steve Magnani @ 2017-10-12 13:48 UTC (permalink / raw)
  To: Jan Kara, linux-kernel; +Cc: Steven J . Magnani

The UDF driver has several points at which conversion between unsigned and
signed types cause (or could cause) problems. On 64-bit systems,
conversion of block addresses larger than 0x7FFFFFFF (>= 1 TiB when
blocksize is 512 bytes) can involve undesired sign extension that corrupts
the block address value. This is known to cause the following problems:

* readdir() can fail on a directory containing File Identifiers residing
  above 0x7FFFFFFF. This manifests as a 'ls' command failing with EIO.

* FIBMAP on a file block located above 0x7FFFFFFF can return a negative
  value. The low 32 bits are correct, but applications that don't mask the
  high 32 bits of the result can perform incorrectly.

Additionally, large unsigned values can be printed as negative numbers,
for example:

  Partition (0 type 1511) starts at physical 460, block length -1779968542

Take care to use unsigned types to store UDF block addresses and to use
format specifiers that match the signedness of the values they are to
print.

Changes since V1:
* Separated printing fixes from sign extension fixes
* Implemented suggested udf_pblk_t typedef for representation of
  block addresses and use it in place of 'uint32_t' for changes in this
  patch series
* Converted some uint32_t block address variables in inode_getblk() to
  udf_pblk_t
* Fixed additional signed/unsigned type mismatches in:
  - udf_table_new_block()
  - udf_fileident_read()
  - udf_new_inode()
  - udf_split_extents()
  - udf_getblk()
  - udf_do_extend_file()
  - udf_bread()
  - udf_setsize()
  - udf_setup_indirect_aext()
  - udf_add_aext()
  - extent_trunc()

Signed-off-by: Steven J. Magnani <steve@digidescorp.com>

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

* [PATCH v2 1/3] udf: Fix 64-bit sign extension issues affecting blocks > 0x7FFFFFFF
  2017-10-12 13:48 [PATCH v2 0/3] udf: Fix some signed/unsigned conversion issues Steve Magnani
@ 2017-10-12 13:48 ` Steve Magnani
  2017-10-16 14:17   ` Jan Kara
  2017-10-12 13:48 ` [PATCH v2 2/3] udf: Fix signed/unsigned format specifiers Steve Magnani
  2017-10-12 13:48 ` [PATCH v2 3/3] udf: Fix some sign-conversion warnings Steve Magnani
  2 siblings, 1 reply; 6+ messages in thread
From: Steve Magnani @ 2017-10-12 13:48 UTC (permalink / raw)
  To: Jan Kara, linux-kernel; +Cc: Steven J . Magnani

Large (> 1 TiB) UDF filesystems appear subject to several problems when
mounted on 64-bit systems:

* readdir() can fail on a directory containing File Identifiers residing
  above 0x7FFFFFFF. This manifests as a 'ls' command failing with EIO.

* FIBMAP on a file block located above 0x7FFFFFFF can return a negative
  value. The low 32 bits are correct, but applications that don't mask the
  high 32 bits of the result can perform incorrectly.

Per suggestion by Jan Kara, introduce a udf_pblk_t type for representation
of UDF block addresses. Ultimately, all driver functions that manipulate
UDF block addresses should use this type; for now, deployment is limited
to functions with actual or potential sign extension issues.

Changes to udf_readdir() and udf_block_map() address the issues noted
above; other changes address potential similar issues uncovered during
audit of the driver code.

Signed-off-by: Steven J. Magnani <steve@digidescorp.com>
---
diff -uprN a/fs/udf/balloc.c b/fs/udf/balloc.c
--- a/fs/udf/balloc.c	2017-10-07 16:46:37.694130197 -0500
+++ b/fs/udf/balloc.c	2017-10-11 13:08:47.969313878 -0500
@@ -218,16 +218,17 @@ out:
 	return alloc_count;
 }
 
-static int udf_bitmap_new_block(struct super_block *sb,
+static udf_pblk_t udf_bitmap_new_block(struct super_block *sb,
 				struct udf_bitmap *bitmap, uint16_t partition,
 				uint32_t goal, int *err)
 {
 	struct udf_sb_info *sbi = UDF_SB(sb);
-	int newbit, bit = 0, block, block_group, group_start;
+	int newbit, bit = 0;
+	udf_pblk_t block, block_group, group_start;
 	int end_goal, nr_groups, bitmap_nr, i;
 	struct buffer_head *bh = NULL;
 	char *ptr;
-	int newblock = 0;
+	udf_pblk_t newblock = 0;
 
 	*err = -ENOSPC;
 	mutex_lock(&sbi->s_alloc_mutex);
@@ -545,13 +546,14 @@ static int udf_table_prealloc_blocks(str
 	return alloc_count;
 }
 
-static int udf_table_new_block(struct super_block *sb,
+static udf_pblk_t udf_table_new_block(struct super_block *sb,
 			       struct inode *table, uint16_t partition,
 			       uint32_t goal, int *err)
 {
 	struct udf_sb_info *sbi = UDF_SB(sb);
 	uint32_t spread = 0xFFFFFFFF, nspread = 0xFFFFFFFF;
-	uint32_t newblock = 0, adsize;
+	udf_pblk_t newblock = 0;
+	uint32_t adsize;
 	uint32_t elen, goal_elen = 0;
 	struct kernel_lb_addr eloc, uninitialized_var(goal_eloc);
 	struct extent_position epos, goal_epos;
@@ -700,12 +702,12 @@ inline int udf_prealloc_blocks(struct su
 	return allocated;
 }
 
-inline int udf_new_block(struct super_block *sb,
+inline udf_pblk_t udf_new_block(struct super_block *sb,
 			 struct inode *inode,
 			 uint16_t partition, uint32_t goal, int *err)
 {
 	struct udf_part_map *map = &UDF_SB(sb)->s_partmaps[partition];
-	int block;
+	udf_pblk_t block;
 
 	if (map->s_partition_flags & UDF_PART_FLAG_UNALLOC_BITMAP)
 		block = udf_bitmap_new_block(sb,
diff -uprN a/fs/udf/dir.c b/fs/udf/dir.c
--- a/fs/udf/dir.c	2017-10-07 16:46:37.694130197 -0500
+++ b/fs/udf/dir.c	2017-10-11 08:01:39.376578063 -0500
@@ -43,7 +43,7 @@ static int udf_readdir(struct file *file
 	struct udf_fileident_bh fibh = { .sbh = NULL, .ebh = NULL};
 	struct fileIdentDesc *fi = NULL;
 	struct fileIdentDesc cfi;
-	int block, iblock;
+	udf_pblk_t block, iblock;
 	loff_t nf_pos;
 	int flen;
 	unsigned char *fname = NULL, *copy_name = NULL;
diff -uprN a/fs/udf/directory.c b/fs/udf/directory.c
--- a/fs/udf/directory.c	2017-10-07 16:46:37.694130197 -0500
+++ b/fs/udf/directory.c	2017-10-11 12:02:27.226674863 -0500
@@ -26,7 +26,8 @@ struct fileIdentDesc *udf_fileident_read
 					 sector_t *offset)
 {
 	struct fileIdentDesc *fi;
-	int i, num, block;
+	int i, num;
+	udf_pblk_t block;
 	struct buffer_head *tmp, *bha[16];
 	struct udf_inode_info *iinfo = UDF_I(dir);
 
diff -uprN a/fs/udf/ialloc.c b/fs/udf/ialloc.c
--- a/fs/udf/ialloc.c	2017-10-07 12:06:18.749230803 -0500
+++ b/fs/udf/ialloc.c	2017-10-11 12:25:21.255500184 -0500
@@ -50,7 +50,7 @@ struct inode *udf_new_inode(struct inode
 	struct super_block *sb = dir->i_sb;
 	struct udf_sb_info *sbi = UDF_SB(sb);
 	struct inode *inode;
-	int block;
+	udf_pblk_t block;
 	uint32_t start = UDF_I(dir)->i_location.logicalBlockNum;
 	struct udf_inode_info *iinfo;
 	struct udf_inode_info *dinfo = UDF_I(dir);
diff -uprN a/fs/udf/inode.c b/fs/udf/inode.c
--- a/fs/udf/inode.c	2017-10-07 16:46:37.694130197 -0500
+++ b/fs/udf/inode.c	2017-10-11 12:49:03.972233635 -0500
@@ -52,7 +52,7 @@ static int udf_alloc_i_data(struct inode
 static sector_t inode_getblk(struct inode *, sector_t, int *, int *);
 static int8_t udf_insert_aext(struct inode *, struct extent_position,
 			      struct kernel_lb_addr, uint32_t);
-static void udf_split_extents(struct inode *, int *, int, int,
+static void udf_split_extents(struct inode *, int *, int, udf_pblk_t,
 			      struct kernel_long_ad *, int *);
 static void udf_prealloc_extents(struct inode *, int, int,
 				 struct kernel_long_ad *, int *);
@@ -316,10 +316,10 @@ int udf_expand_file_adinicb(struct inode
 	return err;
 }
 
-struct buffer_head *udf_expand_dir_adinicb(struct inode *inode, int *block,
-					   int *err)
+struct buffer_head *udf_expand_dir_adinicb(struct inode *inode,
+					    udf_pblk_t *block, int *err)
 {
-	int newblock;
+	udf_pblk_t newblock;
 	struct buffer_head *dbh = NULL;
 	struct kernel_lb_addr eloc;
 	uint8_t alloctype;
@@ -446,7 +446,7 @@ abort:
 	return err;
 }
 
-static struct buffer_head *udf_getblk(struct inode *inode, long block,
+static struct buffer_head *udf_getblk(struct inode *inode, udf_pblk_t block,
 				      int create, int *err)
 {
 	struct buffer_head *bh;
@@ -663,11 +663,11 @@ static sector_t inode_getblk(struct inod
 	struct kernel_lb_addr eloc, tmpeloc;
 	int c = 1;
 	loff_t lbcount = 0, b_off = 0;
-	uint32_t newblocknum, newblock;
+	udf_pblk_t newblocknum, newblock;
 	sector_t offset = 0;
 	int8_t etype;
 	struct udf_inode_info *iinfo = UDF_I(inode);
-	int goal = 0, pgoal = iinfo->i_location.logicalBlockNum;
+	udf_pblk_t goal = 0, pgoal = iinfo->i_location.logicalBlockNum;
 	int lastblock = 0;
 	bool isBeyondEOF;
 
@@ -879,8 +879,8 @@ out_free:
 }
 
 static void udf_split_extents(struct inode *inode, int *c, int offset,
-			      int newblocknum, struct kernel_long_ad *laarr,
-			      int *endnum)
+			       udf_pblk_t newblocknum,
+			       struct kernel_long_ad *laarr, int *endnum)
 {
 	unsigned long blocksize = inode->i_sb->s_blocksize;
 	unsigned char blocksize_bits = inode->i_sb->s_blocksize_bits;
@@ -1166,7 +1166,7 @@ static void udf_update_extents(struct in
 	}
 }
 
-struct buffer_head *udf_bread(struct inode *inode, int block,
+struct buffer_head *udf_bread(struct inode *inode, udf_pblk_t block,
 			      int create, int *err)
 {
 	struct buffer_head *bh = NULL;
@@ -1852,7 +1852,7 @@ struct inode *__udf_iget(struct super_bl
 	return inode;
 }
 
-int udf_setup_indirect_aext(struct inode *inode, int block,
+int udf_setup_indirect_aext(struct inode *inode, udf_pblk_t block,
 			    struct extent_position *epos)
 {
 	struct super_block *sb = inode->i_sb;
@@ -1994,7 +1994,7 @@ int udf_add_aext(struct inode *inode, st
 
 	if (epos->offset + (2 * adsize) > sb->s_blocksize) {
 		int err;
-		int new_block;
+		udf_pblk_t new_block;
 
 		new_block = udf_new_block(sb, NULL,
 					  epos->block.partitionReferenceNum,
@@ -2076,7 +2076,7 @@ int8_t udf_next_aext(struct inode *inode
 
 	while ((etype = udf_current_aext(inode, epos, eloc, elen, inc)) ==
 	       (EXT_NEXT_EXTENT_ALLOCDECS >> 30)) {
-		int block;
+		udf_pblk_t block;
 
 		if (++indirections > UDF_MAX_INDIR_EXTS) {
 			udf_err(inode->i_sb,
@@ -2289,13 +2289,13 @@ int8_t inode_bmap(struct inode *inode, s
 	return etype;
 }
 
-long udf_block_map(struct inode *inode, sector_t block)
+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 = {};
-	int ret;
+	udf_pblk_t ret;
 
 	down_read(&UDF_I(inode)->i_data_sem);
 
diff -uprN a/fs/udf/misc.c b/fs/udf/misc.c
--- a/fs/udf/misc.c	2017-10-07 16:46:37.694130197 -0500
+++ b/fs/udf/misc.c	2017-10-11 08:01:39.380578114 -0500
@@ -28,7 +28,7 @@
 #include "udf_i.h"
 #include "udf_sb.h"
 
-struct buffer_head *udf_tgetblk(struct super_block *sb, int block)
+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));
@@ -36,7 +36,7 @@ struct buffer_head *udf_tgetblk(struct s
 		return sb_getblk(sb, block);
 }
 
-struct buffer_head *udf_tread(struct super_block *sb, int 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));
diff -uprN a/fs/udf/namei.c b/fs/udf/namei.c
--- a/fs/udf/namei.c	2017-10-07 16:46:37.694130197 -0500
+++ b/fs/udf/namei.c	2017-10-11 13:12:15.668773392 -0500
@@ -164,7 +164,8 @@ static struct fileIdentDesc *udf_find_en
 {
 	struct fileIdentDesc *fi = NULL;
 	loff_t f_pos;
-	int block, flen;
+	udf_pblk_t block;
+	int flen;
 	unsigned char *fname = NULL, *copy_name = NULL;
 	unsigned char *nameptr;
 	uint8_t lfi;
@@ -352,7 +353,7 @@ static struct fileIdentDesc *udf_add_ent
 	int nfidlen;
 	uint8_t lfi;
 	uint16_t liu;
-	int block;
+	udf_pblk_t block;
 	struct kernel_lb_addr eloc;
 	uint32_t elen = 0;
 	sector_t offset;
@@ -749,7 +750,7 @@ static int empty_dir(struct inode *dir)
 	struct udf_fileident_bh fibh;
 	loff_t f_pos;
 	loff_t size = udf_ext0_offset(dir) + dir->i_size;
-	int block;
+	udf_pblk_t block;
 	struct kernel_lb_addr eloc;
 	uint32_t elen;
 	sector_t offset;
@@ -913,7 +914,7 @@ static int udf_symlink(struct inode *dir
 	int eoffset, elen = 0;
 	uint8_t *ea;
 	int err;
-	int block;
+	udf_pblk_t block;
 	unsigned char *name = NULL;
 	int namelen;
 	struct udf_inode_info *iinfo;
diff -uprN a/fs/udf/super.c b/fs/udf/super.c
--- a/fs/udf/super.c	2017-10-07 16:46:37.698130278 -0500
+++ b/fs/udf/super.c	2017-10-11 08:01:39.380578114 -0500
@@ -2389,7 +2389,7 @@ static unsigned int udf_count_free_bitma
 	struct buffer_head *bh = NULL;
 	unsigned int accum = 0;
 	int index;
-	int block = 0, newblock;
+	udf_pblk_t block = 0, newblock;
 	struct kernel_lb_addr loc;
 	uint32_t bytes;
 	uint8_t *ptr;
diff -uprN a/fs/udf/truncate.c b/fs/udf/truncate.c
--- a/fs/udf/truncate.c	2017-10-07 12:06:18.749230803 -0500
+++ b/fs/udf/truncate.c	2017-10-11 11:14:47.404965456 -0500
@@ -31,9 +31,9 @@ static void extent_trunc(struct inode *i
 			 uint32_t nelen)
 {
 	struct kernel_lb_addr neloc = {};
-	int last_block = (elen + inode->i_sb->s_blocksize - 1) >>
+	udf_pblk_t last_block = (elen + inode->i_sb->s_blocksize - 1) >>
 		inode->i_sb->s_blocksize_bits;
-	int first_block = (nelen + inode->i_sb->s_blocksize - 1) >>
+	udf_pblk_t first_block = (nelen + inode->i_sb->s_blocksize - 1) >>
 		inode->i_sb->s_blocksize_bits;
 
 	if (nelen) {
@@ -48,7 +48,7 @@ static void extent_trunc(struct inode *i
 
 	if (elen != nelen) {
 		udf_write_aext(inode, epos, &neloc, nelen, 0);
-		if (last_block - first_block > 0) {
+		if (last_block > first_block) {
 			if (etype == (EXT_RECORDED_ALLOCATED >> 30))
 				mark_inode_dirty(inode);
 
diff -uprN a/fs/udf/udfdecl.h b/fs/udf/udfdecl.h
--- a/fs/udf/udfdecl.h	2017-10-07 16:46:37.698130278 -0500
+++ b/fs/udf/udfdecl.h	2017-10-11 10:22:31.131668281 -0500
@@ -73,6 +73,8 @@ static inline size_t udf_ext0_offset(str
 /* computes tag checksum */
 u8 udf_tag_checksum(const struct tag *t);
 
+typedef uint32_t udf_pblk_t;
+
 struct dentry;
 struct inode;
 struct task_struct;
@@ -144,15 +146,17 @@ static inline struct inode *udf_iget(str
 	return __udf_iget(sb, ino, false);
 }
 extern int udf_expand_file_adinicb(struct inode *);
-extern struct buffer_head *udf_expand_dir_adinicb(struct inode *, int *, int *);
-extern struct buffer_head *udf_bread(struct inode *, int, int, int *);
+extern struct buffer_head *udf_expand_dir_adinicb(struct inode *inode,
+						  udf_pblk_t *block, int *err);
+extern struct buffer_head *udf_bread(struct inode *inode, udf_pblk_t block,
+				      int create, int *err);
 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 long udf_block_map(struct inode *, sector_t);
+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, int block,
+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,
 			  struct kernel_lb_addr *eloc, uint32_t elen, int inc);
@@ -168,8 +172,9 @@ extern int8_t udf_current_aext(struct in
 			       struct kernel_lb_addr *, uint32_t *, int);
 
 /* misc.c */
-extern struct buffer_head *udf_tgetblk(struct super_block *, int);
-extern struct buffer_head *udf_tread(struct super_block *, int);
+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,
@@ -228,8 +233,8 @@ extern void udf_free_blocks(struct super
 			    struct kernel_lb_addr *, uint32_t, uint32_t);
 extern int udf_prealloc_blocks(struct super_block *, struct inode *, uint16_t,
 			       uint32_t, uint32_t);
-extern int udf_new_block(struct super_block *, struct inode *, uint16_t,
-			 uint32_t, int *);
+extern udf_pblk_t udf_new_block(struct super_block *sb, struct inode *inode,
+				 uint16_t partition, uint32_t goal, int *err);
 
 /* directory.c */
 extern struct fileIdentDesc *udf_fileident_read(struct inode *, loff_t *,

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

* [PATCH v2 2/3] udf: Fix signed/unsigned format specifiers
  2017-10-12 13:48 [PATCH v2 0/3] udf: Fix some signed/unsigned conversion issues Steve Magnani
  2017-10-12 13:48 ` [PATCH v2 1/3] udf: Fix 64-bit sign extension issues affecting blocks > 0x7FFFFFFF Steve Magnani
@ 2017-10-12 13:48 ` Steve Magnani
  2017-10-12 13:48 ` [PATCH v2 3/3] udf: Fix some sign-conversion warnings Steve Magnani
  2 siblings, 0 replies; 6+ messages in thread
From: Steve Magnani @ 2017-10-12 13:48 UTC (permalink / raw)
  To: Jan Kara, linux-kernel; +Cc: Steven J . Magnani

Fix problems noted in compilion with -Wformat=2 -Wformat-signedness.
In particular, a mismatch between the signedness of a value and the
signedness of its format specifier can result in unsigned values being
printed as negative numbers, e.g.:

  Partition (0 type 1511) starts at physical 460, block length -1779968542

...which occurs when mounting a large (> 1 TiB) UDF partition.

Changes since V1:
* Fixed additional issues noted in udf_bitmap_free_blocks(),
  udf_get_fileident(), udf_show_options()

Signed-off-by: Steven J. Magnani <steve@digidescorp.com>
---
diff -uprN a/fs/udf/balloc.c b/fs/udf/balloc.c
--- a/fs/udf/balloc.c	2017-10-11 15:18:22.039284808 -0500
+++ b/fs/udf/balloc.c	2017-10-11 15:18:45.259250760 -0500
@@ -58,7 +58,7 @@ static int __load_block_bitmap(struct su
 	int nr_groups = bitmap->s_nr_groups;
 
 	if (block_group >= nr_groups) {
-		udf_debug("block_group (%d) > nr_groups (%d)\n",
+		udf_debug("block_group (%u) > nr_groups (%d)\n",
 			  block_group, nr_groups);
 	}
 
@@ -122,7 +122,7 @@ static void udf_bitmap_free_blocks(struc
 	partmap = &sbi->s_partmaps[bloc->partitionReferenceNum];
 	if (bloc->logicalBlockNum + count < count ||
 	    (bloc->logicalBlockNum + count) > partmap->s_partition_len) {
-		udf_debug("%d < %d || %d + %d > %d\n",
+		udf_debug("%u < %d || %u + %u > %u\n",
 			  bloc->logicalBlockNum, 0,
 			  bloc->logicalBlockNum, count,
 			  partmap->s_partition_len);
@@ -151,9 +151,9 @@ static void udf_bitmap_free_blocks(struc
 		bh = bitmap->s_block_bitmap[bitmap_nr];
 		for (i = 0; i < count; i++) {
 			if (udf_set_bit(bit + i, bh->b_data)) {
-				udf_debug("bit %ld already set\n", bit + i);
+				udf_debug("bit %lu already set\n", bit + i);
 				udf_debug("byte=%2x\n",
-					  ((char *)bh->b_data)[(bit + i) >> 3]);
+					  ((__u8 *)bh->b_data)[(bit + i) >> 3]);
 			}
 		}
 		udf_add_free_space(sb, sbi->s_partition, count);

@@ -363,7 +363,7 @@ static void udf_table_free_blocks(struct
 	partmap = &sbi->s_partmaps[bloc->partitionReferenceNum];
 	if (bloc->logicalBlockNum + count < count ||
 	    (bloc->logicalBlockNum + count) > partmap->s_partition_len) {
-		udf_debug("%d < %d || %d + %d > %d\n",
+		udf_debug("%u < %d || %u + %u > %u\n",
 			  bloc->logicalBlockNum, 0,
 			  bloc->logicalBlockNum, count,
 			  partmap->s_partition_len);
@@ -516,7 +516,7 @@ static int udf_table_prealloc_blocks(str
 
 	while (first_block != eloc.logicalBlockNum &&
 	       (etype = udf_next_aext(table, &epos, &eloc, &elen, 1)) != -1) {
-		udf_debug("eloc=%d, elen=%d, first_block=%d\n",
+		udf_debug("eloc=%u, elen=%u, first_block=%u\n",
 			  eloc.logicalBlockNum, elen, first_block);
 		; /* empty loop body */
 	}
diff -uprN a/fs/udf/directory.c b/fs/udf/directory.c
--- a/fs/udf/directory.c	2017-10-11 15:03:12.300741772 -0500
+++ b/fs/udf/directory.c	2017-10-11 15:18:45.255250766 -0500
@@ -176,7 +176,7 @@ struct fileIdentDesc *udf_get_fileident(
 	if (fi->descTag.tagIdent != cpu_to_le16(TAG_IDENT_FID)) {
 		udf_debug("0x%x != TAG_IDENT_FID\n",
 			  le16_to_cpu(fi->descTag.tagIdent));
-		udf_debug("offset: %u sizeof: %lu bufsize: %u\n",
+		udf_debug("offset: %d sizeof: %lu bufsize: %d\n",
 			  *offset, (unsigned long)sizeof(struct fileIdentDesc),
 			  bufsize);
 		return NULL;
diff -uprN a/fs/udf/inode.c b/fs/udf/inode.c
--- a/fs/udf/inode.c	2017-10-11 15:18:22.039284808 -0500
+++ b/fs/udf/inode.c	2017-10-11 15:18:45.251250771 -0500
@@ -1278,14 +1278,14 @@ static int udf_read_inode(struct inode *
 
 reread:
 	if (iloc->partitionReferenceNum >= sbi->s_partitions) {
-		udf_debug("partition reference: %d > logical volume partitions: %d\n",
+		udf_debug("partition reference: %u > logical volume partitions: %u\n",
 			  iloc->partitionReferenceNum, sbi->s_partitions);
 		return -EIO;
 	}
 
 	if (iloc->logicalBlockNum >=
 	    sbi->s_partmaps[iloc->partitionReferenceNum].s_partition_len) {
-		udf_debug("block=%d, partition=%d out of range\n",
+		udf_debug("block=%u, partition=%u out of range\n",
 			  iloc->logicalBlockNum, iloc->partitionReferenceNum);
 		return -EIO;
 	}
@@ -1304,13 +1304,13 @@ reread:
 	 */
 	bh = udf_read_ptagged(inode->i_sb, iloc, 0, &ident);
 	if (!bh) {
-		udf_err(inode->i_sb, "(ino %ld) failed !bh\n", inode->i_ino);
+		udf_err(inode->i_sb, "(ino %lu) failed !bh\n", inode->i_ino);
 		return -EIO;
 	}
 
 	if (ident != TAG_IDENT_FE && ident != TAG_IDENT_EFE &&
 	    ident != TAG_IDENT_USE) {
-		udf_err(inode->i_sb, "(ino %ld) failed ident=%d\n",
+		udf_err(inode->i_sb, "(ino %lu) failed ident=%u\n",
 			inode->i_ino, ident);
 		goto out;
 	}
@@ -1346,7 +1346,7 @@ reread:
 		}
 		brelse(ibh);
 	} else if (fe->icbTag.strategyType != cpu_to_le16(4)) {
-		udf_err(inode->i_sb, "unsupported strategy type: %d\n",
+		udf_err(inode->i_sb, "unsupported strategy type: %u\n",
 			le16_to_cpu(fe->icbTag.strategyType));
 		goto out;
 	}
@@ -1547,7 +1547,7 @@ reread:
 		udf_debug("METADATA BITMAP FILE-----\n");
 		break;
 	default:
-		udf_err(inode->i_sb, "(ino %ld) failed unknown file type=%d\n",
+		udf_err(inode->i_sb, "(ino %lu) failed unknown file type=%u\n",
 			inode->i_ino, fe->icbTag.fileType);
 		goto out;
 	}
@@ -2091,7 +2091,7 @@ int8_t udf_next_aext(struct inode *inode
 		block = udf_get_lb_pblock(inode->i_sb, &epos->block, 0);
 		epos->bh = udf_tread(inode->i_sb, block);
 		if (!epos->bh) {
-			udf_debug("reading block %d failed!\n", block);
+			udf_debug("reading block %u failed!\n", block);
 			return -1;
 		}
 	}
@@ -2146,7 +2146,7 @@ int8_t udf_current_aext(struct inode *in
 		*elen = le32_to_cpu(lad->extLength) & UDF_EXTENT_LENGTH_MASK;
 		break;
 	default:
-		udf_debug("alloc_type = %d unsupported\n", iinfo->i_alloc_type);
+		udf_debug("alloc_type = %u unsupported\n", iinfo->i_alloc_type);
 		return -1;
 	}
 
diff -uprN a/fs/udf/misc.c b/fs/udf/misc.c
--- a/fs/udf/misc.c	2017-10-11 15:18:22.039284808 -0500
+++ b/fs/udf/misc.c	2017-10-11 15:18:45.251250771 -0500
@@ -209,7 +209,7 @@ struct buffer_head *udf_read_tagged(stru
 
 	bh = udf_tread(sb, block);
 	if (!bh) {
-		udf_err(sb, "read failed, block=%u, location=%d\n",
+		udf_err(sb, "read failed, block=%u, location=%u\n",
 			block, location);
 		return NULL;
 	}
@@ -247,7 +247,7 @@ struct buffer_head *udf_read_tagged(stru
 					le16_to_cpu(tag_p->descCRCLength)))
 		return bh;
 
-	udf_debug("Crc failure block %d: crc = %d, crclen = %d\n", block,
+	udf_debug("Crc failure block %u: crc = %u, crclen = %u\n", block,
 		  le16_to_cpu(tag_p->descCRC),
 		  le16_to_cpu(tag_p->descCRCLength));
 error_out:
diff -uprN a/fs/udf/namei.c b/fs/udf/namei.c
--- a/fs/udf/namei.c	2017-10-11 15:18:22.039284808 -0500
+++ b/fs/udf/namei.c	2017-10-11 15:18:45.251250771 -0500
@@ -840,7 +840,7 @@ static int udf_rmdir(struct inode *dir,
 	if (retval)
 		goto end_rmdir;
 	if (inode->i_nlink != 2)
-		udf_warn(inode->i_sb, "empty directory has nlink != 2 (%d)\n",
+		udf_warn(inode->i_sb, "empty directory has nlink != 2 (%u)\n",
 			 inode->i_nlink);
 	clear_nlink(inode);
 	inode->i_size = 0;
@@ -882,7 +882,7 @@ static int udf_unlink(struct inode *dir,
 		goto end_unlink;
 
 	if (!inode->i_nlink) {
-		udf_debug("Deleting nonexistent file (%lu), %d\n",
+		udf_debug("Deleting nonexistent file (%lu), %u\n",
 			  inode->i_ino, inode->i_nlink);
 		set_nlink(inode, 1);
 	}
diff -uprN a/fs/udf/partition.c b/fs/udf/partition.c
--- a/fs/udf/partition.c	2017-10-11 15:18:22.039284808 -0500
+++ b/fs/udf/partition.c	2017-10-11 15:18:45.267250749 -0500
@@ -32,7 +32,7 @@ uint32_t udf_get_pblock(struct super_blo
 	struct udf_sb_info *sbi = UDF_SB(sb);
 	struct udf_part_map *map;
 	if (partition >= sbi->s_partitions) {
-		udf_debug("block=%d, partition=%d, offset=%d: invalid partition\n",
+		udf_debug("block=%u, partition=%u, offset=%u: invalid partition\n",
 			  block, partition, offset);
 		return 0xFFFFFFFF;
 	}
@@ -59,7 +59,7 @@ uint32_t udf_get_pblock_virt15(struct su
 	vdata = &map->s_type_specific.s_virtual;
 
 	if (block > vdata->s_num_entries) {
-		udf_debug("Trying to access block beyond end of VAT (%d max %d)\n",
+		udf_debug("Trying to access block beyond end of VAT (%u max %u)\n",
 			  block, vdata->s_num_entries);
 		return 0xFFFFFFFF;
 	}
@@ -83,7 +83,7 @@ uint32_t udf_get_pblock_virt15(struct su
 
 	bh = sb_bread(sb, loc);
 	if (!bh) {
-		udf_debug("get_pblock(UDF_VIRTUAL_MAP:%p,%d,%d) VAT: %d[%d]\n",
+		udf_debug("get_pblock(UDF_VIRTUAL_MAP:%p,%u,%u) VAT: %u[%u]\n",
 			  sb, block, partition, loc, index);
 		return 0xFFFFFFFF;
 	}
diff -uprN a/fs/udf/super.c b/fs/udf/super.c
--- a/fs/udf/super.c	2017-10-11 15:18:22.043284803 -0500
+++ b/fs/udf/super.c	2017-10-11 15:18:45.267250749 -0500
@@ -366,7 +366,7 @@ static int udf_show_options(struct seq_f
 	if (sbi->s_dmode != UDF_INVALID_MODE)
 		seq_printf(seq, ",dmode=%ho", sbi->s_dmode);
 	if (UDF_QUERY_FLAG(sb, UDF_FLAG_SESSION_SET))
-		seq_printf(seq, ",session=%u", sbi->s_session);
+		seq_printf(seq, ",session=%d", sbi->s_session);
 	if (UDF_QUERY_FLAG(sb, UDF_FLAG_LASTBLOCK_SET))
 		seq_printf(seq, ",lastblock=%u", sbi->s_last_block);
 	if (sbi->s_anchor != 0)
@@ -705,7 +705,7 @@ static loff_t udf_check_vsd(struct super
 
 	sector += (sbi->s_session << sb->s_blocksize_bits);
 
-	udf_debug("Starting at sector %u (%ld byte sectors)\n",
+	udf_debug("Starting at sector %u (%lu byte sectors)\n",
 		  (unsigned int)(sector >> sb->s_blocksize_bits),
 		  sb->s_blocksize);
 	/* Process the sequence (if applicable). The hard limit on the sector
@@ -868,7 +868,7 @@ static int udf_find_fileset(struct super
 
 	if ((fileset->logicalBlockNum != 0xFFFFFFFF ||
 	     fileset->partitionReferenceNum != 0xFFFF) && bh) {
-		udf_debug("Fileset at block=%d, partition=%d\n",
+		udf_debug("Fileset at block=%u, partition=%u\n",
 			  fileset->logicalBlockNum,
 			  fileset->partitionReferenceNum);
 
@@ -981,14 +981,14 @@ static int udf_load_metadata_files(struc
 	mdata->s_phys_partition_ref = type1_index;
 
 	/* metadata address */
-	udf_debug("Metadata file location: block = %d part = %d\n",
+	udf_debug("Metadata file location: block = %u part = %u\n",
 		  mdata->s_meta_file_loc, mdata->s_phys_partition_ref);
 
 	fe = udf_find_metadata_inode_efe(sb, mdata->s_meta_file_loc,
 					 mdata->s_phys_partition_ref);
 	if (IS_ERR(fe)) {
 		/* mirror file entry */
-		udf_debug("Mirror metadata file location: block = %d part = %d\n",
+		udf_debug("Mirror metadata file location: block = %u part = %u\n",
 			  mdata->s_mirror_file_loc, mdata->s_phys_partition_ref);
 
 		fe = udf_find_metadata_inode_efe(sb, mdata->s_mirror_file_loc,
@@ -1012,7 +1012,7 @@ static int udf_load_metadata_files(struc
 		addr.logicalBlockNum = mdata->s_bitmap_file_loc;
 		addr.partitionReferenceNum = mdata->s_phys_partition_ref;
 
-		udf_debug("Bitmap file location: block = %d part = %d\n",
+		udf_debug("Bitmap file location: block = %u part = %u\n",
 			  addr.logicalBlockNum, addr.partitionReferenceNum);
 
 		fe = udf_iget_special(sb, &addr);
@@ -1042,7 +1042,7 @@ static void udf_load_fileset(struct supe
 
 	UDF_SB(sb)->s_serial_number = le16_to_cpu(fset->descTag.tagSerialNum);
 
-	udf_debug("Rootdir at block=%d, partition=%d\n",
+	udf_debug("Rootdir at block=%u, partition=%u\n",
 		  root->logicalBlockNum, root->partitionReferenceNum);
 }
 
@@ -1097,7 +1097,7 @@ static int udf_fill_partdesc_info(struct
 	if (p->accessType == cpu_to_le32(PD_ACCESS_TYPE_OVERWRITABLE))
 		map->s_partition_flags |= UDF_PART_FLAG_OVERWRITABLE;
 
-	udf_debug("Partition (%d type %x) starts at physical %d, block length %d\n",
+	udf_debug("Partition (%d type %x) starts at physical %u, block length %u\n",
 		  p_index, map->s_partition_type,
 		  map->s_partition_root, map->s_partition_len);
 
@@ -1122,7 +1122,7 @@ static int udf_fill_partdesc_info(struct
 		}
 		map->s_uspace.s_table = inode;
 		map->s_partition_flags |= UDF_PART_FLAG_UNALLOC_TABLE;
-		udf_debug("unallocSpaceTable (part %d) @ %ld\n",
+		udf_debug("unallocSpaceTable (part %d) @ %lu\n",
 			  p_index, map->s_uspace.s_table->i_ino);
 	}
 
@@ -1134,7 +1134,7 @@ static int udf_fill_partdesc_info(struct
 		bitmap->s_extPosition = le32_to_cpu(
 				phd->unallocSpaceBitmap.extPosition);
 		map->s_partition_flags |= UDF_PART_FLAG_UNALLOC_BITMAP;
-		udf_debug("unallocSpaceBitmap (part %d) @ %d\n",
+		udf_debug("unallocSpaceBitmap (part %d) @ %u\n",
 			  p_index, bitmap->s_extPosition);
 	}
 
@@ -1157,7 +1157,7 @@ static int udf_fill_partdesc_info(struct
 		}
 		map->s_fspace.s_table = inode;
 		map->s_partition_flags |= UDF_PART_FLAG_FREED_TABLE;
-		udf_debug("freedSpaceTable (part %d) @ %ld\n",
+		udf_debug("freedSpaceTable (part %d) @ %lu\n",
 			  p_index, map->s_fspace.s_table->i_ino);
 	}
 
@@ -1169,7 +1169,7 @@ static int udf_fill_partdesc_info(struct
 		bitmap->s_extPosition = le32_to_cpu(
 				phd->freedSpaceBitmap.extPosition);
 		map->s_partition_flags |= UDF_PART_FLAG_FREED_BITMAP;
-		udf_debug("freedSpaceBitmap (part %d) @ %d\n",
+		udf_debug("freedSpaceBitmap (part %d) @ %u\n",
 			  p_index, bitmap->s_extPosition);
 	}
 	return 0;
@@ -1282,7 +1282,7 @@ static int udf_load_partdesc(struct supe
 	/* First scan for TYPE1 and SPARABLE partitions */
 	for (i = 0; i < sbi->s_partitions; i++) {
 		map = &sbi->s_partmaps[i];
-		udf_debug("Searching map: (%d == %d)\n",
+		udf_debug("Searching map: (%u == %u)\n",
 			  map->s_partition_num, partitionNumber);
 		if (map->s_partition_num == partitionNumber &&
 		    (map->s_partition_type == UDF_TYPE1_MAP15 ||
@@ -1291,7 +1291,7 @@ static int udf_load_partdesc(struct supe
 	}
 
 	if (i >= sbi->s_partitions) {
-		udf_debug("Partition (%d) not found in partition map\n",
+		udf_debug("Partition (%u) not found in partition map\n",
 			  partitionNumber);
 		ret = 0;
 		goto out_bh;
@@ -1483,7 +1483,7 @@ static int udf_load_logicalvol(struct su
 				struct metadataPartitionMap *mdm =
 						(struct metadataPartitionMap *)
 						&(lvd->partitionMaps[offset]);
-				udf_debug("Parsing Logical vol part %d type %d  id=%s\n",
+				udf_debug("Parsing Logical vol part %d type %u  id=%s\n",
 					  i, type, UDF_ID_METADATA);
 
 				map->s_partition_type = UDF_METADATA_MAP25;
@@ -1505,17 +1505,17 @@ static int udf_load_logicalvol(struct su
 				udf_debug("Metadata Ident suffix=0x%x\n",
 					  le16_to_cpu(*(__le16 *)
 						      mdm->partIdent.identSuffix));
-				udf_debug("Metadata part num=%d\n",
+				udf_debug("Metadata part num=%u\n",
 					  le16_to_cpu(mdm->partitionNum));
-				udf_debug("Metadata part alloc unit size=%d\n",
+				udf_debug("Metadata part alloc unit size=%u\n",
 					  le32_to_cpu(mdm->allocUnitSize));
-				udf_debug("Metadata file loc=%d\n",
+				udf_debug("Metadata file loc=%u\n",
 					  le32_to_cpu(mdm->metadataFileLoc));
-				udf_debug("Mirror file loc=%d\n",
+				udf_debug("Mirror file loc=%u\n",
 					  le32_to_cpu(mdm->metadataMirrorFileLoc));
-				udf_debug("Bitmap file loc=%d\n",
+				udf_debug("Bitmap file loc=%u\n",
 					  le32_to_cpu(mdm->metadataBitmapFileLoc));
-				udf_debug("Flags: %d %d\n",
+				udf_debug("Flags: %d %u\n",
 					  mdata->s_flags, mdm->flags);
 			} else {
 				udf_debug("Unknown ident: %s\n",
@@ -1525,7 +1525,7 @@ static int udf_load_logicalvol(struct su
 			map->s_volumeseqnum = le16_to_cpu(upm2->volSeqNum);
 			map->s_partition_num = le16_to_cpu(upm2->partitionNum);
 		}
-		udf_debug("Partition (%d:%d) type %d on volume %d\n",
+		udf_debug("Partition (%d:%u) type %u on volume %u\n",
 			  i, map->s_partition_num, type, map->s_volumeseqnum);
 	}
 
@@ -1533,7 +1533,7 @@ static int udf_load_logicalvol(struct su
 		struct long_ad *la = (struct long_ad *)&(lvd->logicalVolContentsUse[0]);
 
 		*fileset = lelb_to_cpu(la->extLocation);
-		udf_debug("FileSet found in LogicalVolDesc at block=%d, partition=%d\n",
+		udf_debug("FileSet found in LogicalVolDesc at block=%u, partition=%u\n",
 			  fileset->logicalBlockNum,
 			  fileset->partitionReferenceNum);
 	}
@@ -2159,7 +2159,7 @@ static int udf_fill_super(struct super_b
 			ret = udf_load_vrs(sb, &uopt, silent, &fileset);
 			if (ret < 0) {
 				if (!silent && ret != -EACCES) {
-					pr_notice("Scanning with blocksize %d failed\n",
+					pr_notice("Scanning with blocksize %u failed\n",
 						  uopt.blocksize);
 				}
 				brelse(sbi->s_lvid_bh);
@@ -2184,7 +2184,7 @@ static int udf_fill_super(struct super_b
 		goto error_out;
 	}
 
-	udf_debug("Lastblock=%d\n", sbi->s_last_block);
+	udf_debug("Lastblock=%u\n", sbi->s_last_block);
 
 	if (sbi->s_lvid_bh) {
 		struct logicalVolIntegrityDescImpUse *lvidiu =
@@ -2255,7 +2255,7 @@ static int udf_fill_super(struct super_b
 	/* perhaps it's not extensible enough, but for now ... */
 	inode = udf_iget(sb, &rootdir);
 	if (IS_ERR(inode)) {
-		udf_err(sb, "Error in udf_iget, block=%d, partition=%d\n",
+		udf_err(sb, "Error in udf_iget, block=%u, partition=%u\n",
 		       rootdir.logicalBlockNum, rootdir.partitionReferenceNum);
 		ret = PTR_ERR(inode);
 		goto error_out;
diff -uprN a/fs/udf/unicode.c b/fs/udf/unicode.c
--- a/fs/udf/unicode.c	2017-10-11 15:18:22.043284803 -0500
+++ b/fs/udf/unicode.c	2017-10-11 15:18:45.251250771 -0500
@@ -200,7 +200,7 @@ static int udf_name_from_CS0(uint8_t *st
 	cmp_id = ocu[0];
 	if (cmp_id != 8 && cmp_id != 16) {
 		memset(str_o, 0, str_max_len);
-		pr_err("unknown compression code (%d)\n", cmp_id);
+		pr_err("unknown compression code (%u)\n", cmp_id);
 		return -EINVAL;
 	}
 	u_ch = cmp_id >> 3;

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

* [PATCH v2 3/3] udf: Fix some sign-conversion warnings
  2017-10-12 13:48 [PATCH v2 0/3] udf: Fix some signed/unsigned conversion issues Steve Magnani
  2017-10-12 13:48 ` [PATCH v2 1/3] udf: Fix 64-bit sign extension issues affecting blocks > 0x7FFFFFFF Steve Magnani
  2017-10-12 13:48 ` [PATCH v2 2/3] udf: Fix signed/unsigned format specifiers Steve Magnani
@ 2017-10-12 13:48 ` Steve Magnani
  2 siblings, 0 replies; 6+ messages in thread
From: Steve Magnani @ 2017-10-12 13:48 UTC (permalink / raw)
  To: Jan Kara, linux-kernel; +Cc: Steven J . Magnani

Fix some warnings that appear when compiling with -Wconversion.
A sub-optimal choice of variable type leads to warnings about
conversion in both directions between unsigned and signed.

Signed-off-by: Steven J. Magnani <steve@digidescorp.com>
---
diff -uprN a/fs/udf/directory.c b/fs/udf/directory.c
--- a/fs/udf/directory.c	2017-10-07 16:46:37.694130197 -0500
+++ b/fs/udf/directory.c	2017-10-11 12:02:27.226674863 -0500
@@ -52,7 +52,7 @@ struct fileIdentDesc *udf_fileident_read
 	}
 
 	if (fibh->eoffset == dir->i_sb->s_blocksize) {
-		int lextoffset = epos->offset;
+		uint32_t lextoffset = epos->offset;
 		unsigned char blocksize_bits = dir->i_sb->s_blocksize_bits;
 
 		if (udf_next_aext(dir, epos, eloc, elen, 1) !=
@@ -111,7 +111,7 @@ struct fileIdentDesc *udf_fileident_read
 		memcpy((uint8_t *)cfi, (uint8_t *)fi,
 		       sizeof(struct fileIdentDesc));
 	} else if (fibh->eoffset > dir->i_sb->s_blocksize) {
-		int lextoffset = epos->offset;
+		uint32_t lextoffset = epos->offset;
 
 		if (udf_next_aext(dir, epos, eloc, elen, 1) !=
 		    (EXT_RECORDED_ALLOCATED >> 30))
diff -uprN a/fs/udf/inode.c b/fs/udf/inode.c
--- a/fs/udf/inode.c	2017-10-07 16:46:37.694130197 -0500
+++ b/fs/udf/inode.c	2017-10-11 12:49:03.972233635 -0500
@@ -480,7 +480,7 @@ static int udf_do_extend_file(struct ino
 	int count = 0, fake = !(last_ext->extLength & UDF_EXTENT_LENGTH_MASK);
 	struct super_block *sb = inode->i_sb;
 	struct kernel_lb_addr prealloc_loc = {};
-	int prealloc_len = 0;
+	uint32_t prealloc_len = 0;
 	struct udf_inode_info *iinfo;
 	int err;
 
@@ -1193,7 +1193,7 @@ int udf_setsize(struct inode *inode, lof
 {
 	int err;
 	struct udf_inode_info *iinfo;
-	int bsize = i_blocksize(inode);
+	unsigned int bsize = i_blocksize(inode);
 
 	if (!(S_ISREG(inode->i_mode) || S_ISDIR(inode->i_mode) ||
 	      S_ISLNK(inode->i_mode)))

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

* Re: [PATCH v2 1/3] udf: Fix 64-bit sign extension issues affecting blocks > 0x7FFFFFFF
  2017-10-12 13:48 ` [PATCH v2 1/3] udf: Fix 64-bit sign extension issues affecting blocks > 0x7FFFFFFF Steve Magnani
@ 2017-10-16 14:17   ` Jan Kara
  2017-10-16 19:20     ` Steve Magnani
  0 siblings, 1 reply; 6+ messages in thread
From: Jan Kara @ 2017-10-16 14:17 UTC (permalink / raw)
  To: Steve Magnani; +Cc: Jan Kara, linux-kernel, Steven J . Magnani

On Thu 12-10-17 08:48:40, Steve Magnani wrote:
> Large (> 1 TiB) UDF filesystems appear subject to several problems when
> mounted on 64-bit systems:
> 
> * readdir() can fail on a directory containing File Identifiers residing
>   above 0x7FFFFFFF. This manifests as a 'ls' command failing with EIO.
> 
> * FIBMAP on a file block located above 0x7FFFFFFF can return a negative
>   value. The low 32 bits are correct, but applications that don't mask the
>   high 32 bits of the result can perform incorrectly.
> 
> Per suggestion by Jan Kara, introduce a udf_pblk_t type for representation
> of UDF block addresses. Ultimately, all driver functions that manipulate
> UDF block addresses should use this type; for now, deployment is limited
> to functions with actual or potential sign extension issues.
> 
> Changes to udf_readdir() and udf_block_map() address the issues noted
> above; other changes address potential similar issues uncovered during
> audit of the driver code.
> 
> Signed-off-by: Steven J. Magnani <steve@digidescorp.com>

Thanks for the patch. Two small comments below.

> ---
> diff -uprN a/fs/udf/balloc.c b/fs/udf/balloc.c
> --- a/fs/udf/balloc.c	2017-10-07 16:46:37.694130197 -0500
> +++ b/fs/udf/balloc.c	2017-10-11 13:08:47.969313878 -0500
> @@ -218,16 +218,17 @@ out:
>  	return alloc_count;
>  }
>  
> -static int udf_bitmap_new_block(struct super_block *sb,
> +static udf_pblk_t udf_bitmap_new_block(struct super_block *sb,
>  				struct udf_bitmap *bitmap, uint16_t partition,
>  				uint32_t goal, int *err)
>  {
>  	struct udf_sb_info *sbi = UDF_SB(sb);
> -	int newbit, bit = 0, block, block_group, group_start;
> +	int newbit, bit = 0;
> +	udf_pblk_t block, block_group, group_start;

Only 'block' should be udf_pblk_t here. group_start is just an offset of
bitmap start in a block and block_group is how many bitmap blocks in a
bitmap we need to skip - neither of these is a physical block number and
neither has a problem with int type...

> diff -uprN a/fs/udf/truncate.c b/fs/udf/truncate.c
> --- a/fs/udf/truncate.c	2017-10-07 12:06:18.749230803 -0500
> +++ b/fs/udf/truncate.c	2017-10-11 11:14:47.404965456 -0500
> @@ -31,9 +31,9 @@ static void extent_trunc(struct inode *i
>  			 uint32_t nelen)
>  {
>  	struct kernel_lb_addr neloc = {};
> -	int last_block = (elen + inode->i_sb->s_blocksize - 1) >>
> +	udf_pblk_t last_block = (elen + inode->i_sb->s_blocksize - 1) >>
>  		inode->i_sb->s_blocksize_bits;
> -	int first_block = (nelen + inode->i_sb->s_blocksize - 1) >>
> +	udf_pblk_t first_block = (nelen + inode->i_sb->s_blocksize - 1) >>
>  		inode->i_sb->s_blocksize_bits;
>  
>  	if (nelen) {

These two are actually logical file offsets, not physical block numbers.
And since extent length is u32 and we divide it by block size, they cannot
really overflow. So just leave this place as is.

If you agree with these changes, I'll update your patch and merge it to my
tree (along with the other two changes which look good to me).

								Honza
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

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

* Re: [PATCH v2 1/3] udf: Fix 64-bit sign extension issues affecting blocks > 0x7FFFFFFF
  2017-10-16 14:17   ` Jan Kara
@ 2017-10-16 19:20     ` Steve Magnani
  0 siblings, 0 replies; 6+ messages in thread
From: Steve Magnani @ 2017-10-16 19:20 UTC (permalink / raw)
  To: Jan Kara; +Cc: Jan Kara, linux-kernel, Steven J . Magnani

Jan -

On 10/16/2017 09:17 AM, Jan Kara wrote:
> ...
> If you agree with these changes, I'll update your patch and merge it to my
> tree (along with the other two changes which look good to me).
>
>
I think your suggestions are OK. In an ideal world we could work to 
reduce the many signed/unsigned conversions, which make me dizzy. I 
think the ones that really cause problems are large unsigned int values 
which are converted to int and then either to sector_t (except on 32-bit 
systems without LBDAF) or to unsigned long on 64-bit systems.
------------------------------------------------------------------------
  Steven J. Magnani               "I claim this network for MARS!
  www.digidescorp.com              Earthling, return my space modulator!"

  #include <standard.disclaimer>

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

end of thread, other threads:[~2017-10-16 19:20 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-10-12 13:48 [PATCH v2 0/3] udf: Fix some signed/unsigned conversion issues Steve Magnani
2017-10-12 13:48 ` [PATCH v2 1/3] udf: Fix 64-bit sign extension issues affecting blocks > 0x7FFFFFFF Steve Magnani
2017-10-16 14:17   ` Jan Kara
2017-10-16 19:20     ` Steve Magnani
2017-10-12 13:48 ` [PATCH v2 2/3] udf: Fix signed/unsigned format specifiers Steve Magnani
2017-10-12 13:48 ` [PATCH v2 3/3] udf: Fix some sign-conversion warnings Steve Magnani

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.