* [PATCH] udf: Fix 64-bit sign extension issues affecting blocks > 0x7FFFFFFF
@ 2017-10-09 15:04 Steve Magnani
2017-10-10 7:33 ` Jan Kara
0 siblings, 1 reply; 4+ messages in thread
From: Steve Magnani @ 2017-10-09 15:04 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.
* Unsigned values > 0x7FFFFFFF are output as negative numbers in some
driver printks, e.g.:
Partition (0 type 1511) starts at physical 460, block length -1779968542
Take care to use "%u" when printing unsigned values and to use unsigned
types to store UDF block addresses.
Signed-off-by: Steven J. Magnani <steve@digidescorp.com>
---
Index: balloc.c
===================================================================
--- a/fs/udf/balloc.c (revision 26779)
+++ b/fs/udf/balloc.c (working copy)
@@ -58,7 +58,7 @@
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 @@
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,7 +151,7 @@
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]);
}
@@ -218,16 +218,17 @@
return alloc_count;
}
-static int udf_bitmap_new_block(struct super_block *sb,
+static uint32_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;
+ uint32_t block, block_group, group_start;
int end_goal, nr_groups, bitmap_nr, i;
struct buffer_head *bh = NULL;
char *ptr;
- int newblock = 0;
+ uint32_t newblock = 0;
*err = -ENOSPC;
mutex_lock(&sbi->s_alloc_mutex);
@@ -362,7 +363,7 @@
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);
@@ -515,7 +516,7 @@
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 */
}
@@ -700,12 +701,12 @@
return allocated;
}
-inline int udf_new_block(struct super_block *sb,
+inline uint32_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;
+ uint32_t block;
if (map->s_partition_flags & UDF_PART_FLAG_UNALLOC_BITMAP)
block = udf_bitmap_new_block(sb,
Index: dir.c
===================================================================
--- a/fs/udf/dir.c (revision 26779)
+++ b/fs/udf/dir.c (working copy)
@@ -43,7 +43,7 @@
struct udf_fileident_bh fibh = { .sbh = NULL, .ebh = NULL};
struct fileIdentDesc *fi = NULL;
struct fileIdentDesc cfi;
- int block, iblock;
+ uint32_t block, iblock;
loff_t nf_pos;
int flen;
unsigned char *fname = NULL, *copy_name = NULL;
Index: directory.c
===================================================================
--- a/fs/udf/directory.c (revision 26779)
+++ b/fs/udf/directory.c (working copy)
@@ -26,7 +26,8 @@
sector_t *offset)
{
struct fileIdentDesc *fi;
- int i, num, block;
+ int i, num;
+ uint32_t block;
struct buffer_head *tmp, *bha[16];
struct udf_inode_info *iinfo = UDF_I(dir);
Index: inode.c
===================================================================
--- a/fs/udf/inode.c (revision 26779)
+++ b/fs/udf/inode.c (working copy)
@@ -316,10 +316,10 @@
return err;
}
-struct buffer_head *udf_expand_dir_adinicb(struct inode *inode, int *block,
+struct buffer_head *udf_expand_dir_adinicb(struct inode *inode, uint32_t *block,
int *err)
{
- int newblock;
+ uint32_t newblock;
struct buffer_head *dbh = NULL;
struct kernel_lb_addr eloc;
uint8_t alloctype;
@@ -667,7 +667,7 @@
sector_t offset = 0;
int8_t etype;
struct udf_inode_info *iinfo = UDF_I(inode);
- int goal = 0, pgoal = iinfo->i_location.logicalBlockNum;
+ uint32_t goal = 0, pgoal = iinfo->i_location.logicalBlockNum;
int lastblock = 0;
bool isBeyondEOF;
@@ -1278,7 +1278,7 @@
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;
}
@@ -1285,7 +1285,7 @@
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 @@
*/
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 @@
}
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 @@
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;
}
@@ -2076,7 +2076,7 @@
while ((etype = udf_current_aext(inode, epos, eloc, elen, inc)) ==
(EXT_NEXT_EXTENT_ALLOCDECS >> 30)) {
- int block;
+ uint32_t block;
if (++indirections > UDF_MAX_INDIR_EXTS) {
udf_err(inode->i_sb,
@@ -2091,7 +2091,7 @@
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 @@
*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;
}
@@ -2289,13 +2289,13 @@
return etype;
}
-long udf_block_map(struct inode *inode, sector_t block)
+uint32_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;
+ uint32_t ret;
down_read(&UDF_I(inode)->i_data_sem);
Index: misc.c
===================================================================
--- a/fs/udf/misc.c (revision 26779)
+++ b/fs/udf/misc.c (working copy)
@@ -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, uint32_t block)
{
if (UDF_QUERY_FLAG(sb, UDF_FLAG_VARCONV))
return sb_getblk(sb, udf_fixed_to_variable(block));
@@ -36,7 +36,7 @@
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, uint32_t block)
{
if (UDF_QUERY_FLAG(sb, UDF_FLAG_VARCONV))
return sb_bread(sb, udf_fixed_to_variable(block));
@@ -209,7 +209,7 @@
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 @@
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:
Index: namei.c
===================================================================
--- a/fs/udf/namei.c (revision 26779)
+++ b/fs/udf/namei.c (working copy)
@@ -164,7 +164,7 @@
{
struct fileIdentDesc *fi = NULL;
loff_t f_pos;
- int block, flen;
+ int flen;
unsigned char *fname = NULL, *copy_name = NULL;
unsigned char *nameptr;
uint8_t lfi;
@@ -171,7 +171,7 @@
uint16_t liu;
loff_t size;
struct kernel_lb_addr eloc;
- uint32_t elen;
+ uint32_t block, elen;
sector_t offset;
struct extent_position epos = {};
struct udf_inode_info *dinfo = UDF_I(dir);
@@ -352,7 +352,7 @@
int nfidlen;
uint8_t lfi;
uint16_t liu;
- int block;
+ uint32_t block;
struct kernel_lb_addr eloc;
uint32_t elen = 0;
sector_t offset;
@@ -749,7 +749,7 @@
struct udf_fileident_bh fibh;
loff_t f_pos;
loff_t size = udf_ext0_offset(dir) + dir->i_size;
- int block;
+ uint32_t block;
struct kernel_lb_addr eloc;
uint32_t elen;
sector_t offset;
@@ -839,7 +839,7 @@
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;
@@ -881,7 +881,7 @@
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);
}
@@ -913,7 +913,7 @@
int eoffset, elen = 0;
uint8_t *ea;
int err;
- int block;
+ uint32_t block;
unsigned char *name = NULL;
int namelen;
struct udf_inode_info *iinfo;
Index: partition.c
===================================================================
--- a/fs/udf/partition.c (revision 26779)
+++ b/fs/udf/partition.c (working copy)
@@ -32,7 +32,7 @@
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 @@
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 @@
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;
}
Index: super.c
===================================================================
--- a/fs/udf/super.c (revision 26779)
+++ b/fs/udf/super.c (working copy)
@@ -705,7 +705,7 @@
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
@@ -721,7 +721,7 @@
for (; !nsr02 && !nsr03 && sector < VSD_MAX_SECTOR_OFFSET;
sector += sectorsize) {
/* Read a block */
- bh = udf_tread(sb, sector >> sb->s_blocksize_bits);
+ bh = udf_tread(sb, (uint32_t)(sector >> sb->s_blocksize_bits));
if (!bh)
break;
@@ -868,7 +868,7 @@
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,7 +981,7 @@
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,
@@ -988,7 +988,7 @@
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 @@
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 @@
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 @@
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 @@
}
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 @@
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 @@
}
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 @@
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 @@
/* 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 @@
}
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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
/* 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;
@@ -2389,7 +2389,7 @@
struct buffer_head *bh = NULL;
unsigned int accum = 0;
int index;
- int block = 0, newblock;
+ uint32_t block = 0, newblock;
struct kernel_lb_addr loc;
uint32_t bytes;
uint8_t *ptr;
Index: udfdecl.h
===================================================================
--- a/fs/udf/udfdecl.h (revision 26779)
+++ b/fs/udf/udfdecl.h (working copy)
@@ -144,12 +144,13 @@
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_expand_dir_adinicb(struct inode *inode,
+ uint32_t *block, int *err);
extern struct buffer_head *udf_bread(struct inode *, int, int, int *);
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 uint32_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,
@@ -168,8 +169,8 @@
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, uint32_t block);
+extern struct buffer_head *udf_tread(struct super_block *sb, uint32_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 +229,8 @@
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 uint32_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 *,
Index: unicode.c
===================================================================
--- a/fs/udf/unicode.c (revision 26779)
+++ b/fs/udf/unicode.c (working copy)
@@ -200,7 +200,7 @@
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] 4+ messages in thread
* Re: [PATCH] udf: Fix 64-bit sign extension issues affecting blocks > 0x7FFFFFFF
2017-10-09 15:04 [PATCH] udf: Fix 64-bit sign extension issues affecting blocks > 0x7FFFFFFF Steve Magnani
@ 2017-10-10 7:33 ` Jan Kara
2017-10-11 3:30 ` Steve Magnani
0 siblings, 1 reply; 4+ messages in thread
From: Jan Kara @ 2017-10-10 7:33 UTC (permalink / raw)
To: Steve Magnani; +Cc: Jan Kara, linux-kernel, Steven J . Magnani
On Mon 09-10-17 10:04:52, 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.
>
> * Unsigned values > 0x7FFFFFFF are output as negative numbers in some
> driver printks, e.g.:
> Partition (0 type 1511) starts at physical 460, block length -1779968542
>
> Take care to use "%u" when printing unsigned values and to use unsigned
> types to store UDF block addresses.
>
> Signed-off-by: Steven J. Magnani <steve@digidescorp.com>
Thanks for looking into this and for the patch! However the patch seems to
be mixing two changes into one which I'd prefer to be separate patches:
1) Changes so that physical block numbers are stored in uint32_t (and
accompanying format string changes). Also when doing this, could you please
create a dedicated type like
typedef uint32_t udf_pblk_t;
and use it instead of uint32_t? That way it would be cleaner what's going
on. Thanks!
2) Changes fixing signedness in various format strings for various types -
put these in a separare patch please.
> --- a/fs/udf/balloc.c (revision 26779)
> +++ b/fs/udf/balloc.c (working copy)
...
> @@ -151,7 +151,7 @@
> 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);
This change looks wrong - bit and i are signed. However they are ints, not
longs, so that should indeed be fixed.
Honza
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] udf: Fix 64-bit sign extension issues affecting blocks > 0x7FFFFFFF
2017-10-10 7:33 ` Jan Kara
@ 2017-10-11 3:30 ` Steve Magnani
2017-10-11 8:27 ` Jan Kara
0 siblings, 1 reply; 4+ messages in thread
From: Steve Magnani @ 2017-10-11 3:30 UTC (permalink / raw)
To: Jan Kara; +Cc: Jan Kara, linux-kernel, Steven J . Magnani
Jan -
On 10/10/2017 02:33 AM, Jan Kara wrote:
> On Mon 09-10-17 10:04:52, Steve Magnani wrote:
>
> ...the patch seems to be mixing two changes into one which I'd prefer to be
> separate patches:
>
> 1) Changes so that physical block numbers are stored in uint32_t (and
> accompanying format string changes). Also when doing this, could you please
> create a dedicated type like
>
> typedef uint32_t udf_pblk_t;
>
> and use it instead of uint32_t? That way it would be cleaner what's going
> on. Thanks!
I agree with this in principle and in fact do something like it in my
application code for just that reason. But, doing a complete job of this
in the driver would increase the scope far beyond what is needed to fix
the bugs I see and beyond what I am able to support. Would it be
acceptable to limit usage of this type to a subset of the places it
could ultimately be used? (Example: use it in udf_readdir(), which has a
bug requiring a type change, but not necessarily in udf_read_tagged(),
which doesn't).
> 2) Changes fixing signedness in various format strings for various types -
> put these in a separare patch please.
Sure - but would you be opposed to putting _all_ of the format string
changes in that patch? There are some format string changes (i.e., in
unicode.c) that obviously don't have anything to do with block numbers,
but I think almost all of the rest do. It gets a little murky when block
numbers or counts are used in calculations. The unifying idea behind all
the format string changes is preventing sign extension from causing
unsigned values to be printed as negative, so on that basis I think an
argument can be made that they all "go" together.
>> --- a/fs/udf/balloc.c (revision 26779)
>> +++ b/fs/udf/balloc.c (working copy)
> ...
>> @@ -151,7 +151,7 @@
>> 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);
> This change looks wrong - bit and i are signed. However they are ints, not
> longs, so that should indeed be fixed.
'bit' and 'i' are ints in the function _below_ this change, but unsigned
long within this function. So I think this is correct.
Regards,
------------------------------------------------------------------------
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] 4+ messages in thread
* Re: [PATCH] udf: Fix 64-bit sign extension issues affecting blocks > 0x7FFFFFFF
2017-10-11 3:30 ` Steve Magnani
@ 2017-10-11 8:27 ` Jan Kara
0 siblings, 0 replies; 4+ messages in thread
From: Jan Kara @ 2017-10-11 8:27 UTC (permalink / raw)
To: Steve Magnani; +Cc: Jan Kara, Jan Kara, linux-kernel, Steven J . Magnani
On Tue 10-10-17 22:30:30, Steve Magnani wrote:
> Jan -
>
> On 10/10/2017 02:33 AM, Jan Kara wrote:
> >On Mon 09-10-17 10:04:52, Steve Magnani wrote:
> >
> >...the patch seems to be mixing two changes into one which I'd prefer to be
> > separate patches:
> >
> >1) Changes so that physical block numbers are stored in uint32_t (and
> >accompanying format string changes). Also when doing this, could you please
> >create a dedicated type like
> >
> >typedef uint32_t udf_pblk_t;
> >
> >and use it instead of uint32_t? That way it would be cleaner what's going
> >on. Thanks!
> I agree with this in principle and in fact do something like it in my
> application code for just that reason. But, doing a complete job of this in
> the driver would increase the scope far beyond what is needed to fix the
> bugs I see and beyond what I am able to support. Would it be acceptable to
> limit usage of this type to a subset of the places it could ultimately be
> used? (Example: use it in udf_readdir(), which has a bug requiring a type
> change, but not necessarily in udf_read_tagged(), which doesn't).
Yeah, I'd be fine with that. It's better to start the conversion only in
some places than to not start it at all.
> >2) Changes fixing signedness in various format strings for various types -
> >put these in a separare patch please.
> Sure - but would you be opposed to putting _all_ of the format string
> changes in that patch? There are some format string changes (i.e., in
> unicode.c) that obviously don't have anything to do with block numbers, but
> I think almost all of the rest do. It gets a little murky when block numbers
> or counts are used in calculations. The unifying idea behind all the format
> string changes is preventing sign extension from causing unsigned values to
> be printed as negative, so on that basis I think an argument can be made
> that they all "go" together.
Yeah, all the format string changes can be in one patch. Those are
generally of the type "fix up format string to match passed argument"
anyway.
> >>--- a/fs/udf/balloc.c (revision 26779)
> >>+++ b/fs/udf/balloc.c (working copy)
> >...
> >>@@ -151,7 +151,7 @@
> >> 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);
> >This change looks wrong - bit and i are signed. However they are ints, not
> >longs, so that should indeed be fixed.
> 'bit' and 'i' are ints in the function _below_ this change, but unsigned
> long within this function. So I think this is correct.
Ah, right. I got confused.
Honza
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2017-10-11 8:27 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-10-09 15:04 [PATCH] udf: Fix 64-bit sign extension issues affecting blocks > 0x7FFFFFFF Steve Magnani
2017-10-10 7:33 ` Jan Kara
2017-10-11 3:30 ` Steve Magnani
2017-10-11 8:27 ` 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.