Skip to content

Commit 41e76c6

Browse files
nickdesaulniersaxboe
authored andcommitted
nbd: use shifts rather than multiplies
commit fad7cd3 ("nbd: add the check to prevent overflow in __nbd_ioctl()") raised an issue from the fallback helpers added in commit f090782 ("compiler.h: enable builtin overflow checkers and add fallback code") ERROR: modpost: "__divdi3" [drivers/block/nbd.ko] undefined! As Stephen Rothwell notes: The added check_mul_overflow() call is being passed 64 bit values. COMPILER_HAS_GENERIC_BUILTIN_OVERFLOW is not set for this build (see include/linux/overflow.h). Specifically, the helpers for checking whether the results of a multiplication overflowed (__unsigned_mul_overflow, __signed_add_overflow) use the division operator when !COMPILER_HAS_GENERIC_BUILTIN_OVERFLOW. This is problematic for 64b operands on 32b hosts. This was fixed upstream by commit 76ae847 ("Documentation: raise minimum supported version of GCC to 5.1") which is not suitable to be backported to stable. Further, __builtin_mul_overflow() would emit a libcall to a compiler-rt-only symbol when compiling with clang < 14 for 32b targets. ld.lld: error: undefined symbol: __mulodi4 In order to keep stable buildable with GCC 4.9 and clang < 14, modify struct nbd_config to instead track the number of bits of the block size; reconstructing the block size using runtime checked shifts that are not problematic for those compilers and in a ways that can be backported to stable. In nbd_set_size, we do validate that the value of blksize must be a power of two (POT) and is in the range of [512, PAGE_SIZE] (both inclusive). This does modify the debugfs interface. Cc: [email protected] Cc: Arnd Bergmann <[email protected]> Cc: Rasmus Villemoes <[email protected]> Link: ClangBuiltLinux#1438 Link: https://lore.kernel.org/all/[email protected]/ Link: https://lore.kernel.org/stable/CAHk-=whiQBofgis_rkniz8GBP9wZtSZdcDEffgSLO62BUGV3gg@mail.gmail.com/ Reported-by: Naresh Kamboju <[email protected]> Reported-by: Nathan Chancellor <[email protected]> Reported-by: Stephen Rothwell <[email protected]> Suggested-by: Kees Cook <[email protected]> Suggested-by: Linus Torvalds <[email protected]> Suggested-by: Pavel Machek <[email protected]> Signed-off-by: Nick Desaulniers <[email protected]> Reviewed-by: Josef Bacik <[email protected]> Link: https://lore.kernel.org/r/[email protected] Signed-off-by: Jens Axboe <[email protected]>
1 parent ebc69e8 commit 41e76c6

File tree

1 file changed

+17
-12
lines changed

1 file changed

+17
-12
lines changed

drivers/block/nbd.c

Lines changed: 17 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -97,13 +97,18 @@ struct nbd_config {
9797

9898
atomic_t recv_threads;
9999
wait_queue_head_t recv_wq;
100-
loff_t blksize;
100+
unsigned int blksize_bits;
101101
loff_t bytesize;
102102
#if IS_ENABLED(CONFIG_DEBUG_FS)
103103
struct dentry *dbg_dir;
104104
#endif
105105
};
106106

107+
static inline unsigned int nbd_blksize(struct nbd_config *config)
108+
{
109+
return 1u << config->blksize_bits;
110+
}
111+
107112
struct nbd_device {
108113
struct blk_mq_tag_set tag_set;
109114

@@ -146,7 +151,7 @@ static struct dentry *nbd_dbg_dir;
146151

147152
#define NBD_MAGIC 0x68797548
148153

149-
#define NBD_DEF_BLKSIZE 1024
154+
#define NBD_DEF_BLKSIZE_BITS 10
150155

151156
static unsigned int nbds_max = 16;
152157
static int max_part = 16;
@@ -317,12 +322,12 @@ static int nbd_set_size(struct nbd_device *nbd, loff_t bytesize,
317322
loff_t blksize)
318323
{
319324
if (!blksize)
320-
blksize = NBD_DEF_BLKSIZE;
325+
blksize = 1u << NBD_DEF_BLKSIZE_BITS;
321326
if (blksize < 512 || blksize > PAGE_SIZE || !is_power_of_2(blksize))
322327
return -EINVAL;
323328

324329
nbd->config->bytesize = bytesize;
325-
nbd->config->blksize = blksize;
330+
nbd->config->blksize_bits = __ffs(blksize);
326331

327332
if (!nbd->task_recv)
328333
return 0;
@@ -1337,7 +1342,7 @@ static int nbd_start_device(struct nbd_device *nbd)
13371342
args->index = i;
13381343
queue_work(nbd->recv_workq, &args->work);
13391344
}
1340-
return nbd_set_size(nbd, config->bytesize, config->blksize);
1345+
return nbd_set_size(nbd, config->bytesize, nbd_blksize(config));
13411346
}
13421347

13431348
static int nbd_start_device_ioctl(struct nbd_device *nbd, struct block_device *bdev)
@@ -1406,11 +1411,11 @@ static int __nbd_ioctl(struct block_device *bdev, struct nbd_device *nbd,
14061411
case NBD_SET_BLKSIZE:
14071412
return nbd_set_size(nbd, config->bytesize, arg);
14081413
case NBD_SET_SIZE:
1409-
return nbd_set_size(nbd, arg, config->blksize);
1414+
return nbd_set_size(nbd, arg, nbd_blksize(config));
14101415
case NBD_SET_SIZE_BLOCKS:
1411-
if (check_mul_overflow((loff_t)arg, config->blksize, &bytesize))
1416+
if (check_shl_overflow(arg, config->blksize_bits, &bytesize))
14121417
return -EINVAL;
1413-
return nbd_set_size(nbd, bytesize, config->blksize);
1418+
return nbd_set_size(nbd, bytesize, nbd_blksize(config));
14141419
case NBD_SET_TIMEOUT:
14151420
nbd_set_cmd_timeout(nbd, arg);
14161421
return 0;
@@ -1476,7 +1481,7 @@ static struct nbd_config *nbd_alloc_config(void)
14761481
atomic_set(&config->recv_threads, 0);
14771482
init_waitqueue_head(&config->recv_wq);
14781483
init_waitqueue_head(&config->conn_wait);
1479-
config->blksize = NBD_DEF_BLKSIZE;
1484+
config->blksize_bits = NBD_DEF_BLKSIZE_BITS;
14801485
atomic_set(&config->live_connections, 0);
14811486
try_module_get(THIS_MODULE);
14821487
return config;
@@ -1604,7 +1609,7 @@ static int nbd_dev_dbg_init(struct nbd_device *nbd)
16041609
debugfs_create_file("tasks", 0444, dir, nbd, &nbd_dbg_tasks_fops);
16051610
debugfs_create_u64("size_bytes", 0444, dir, &config->bytesize);
16061611
debugfs_create_u32("timeout", 0444, dir, &nbd->tag_set.timeout);
1607-
debugfs_create_u64("blocksize", 0444, dir, &config->blksize);
1612+
debugfs_create_u32("blocksize_bits", 0444, dir, &config->blksize_bits);
16081613
debugfs_create_file("flags", 0444, dir, nbd, &nbd_dbg_flags_fops);
16091614

16101615
return 0;
@@ -1826,7 +1831,7 @@ nbd_device_policy[NBD_DEVICE_ATTR_MAX + 1] = {
18261831
static int nbd_genl_size_set(struct genl_info *info, struct nbd_device *nbd)
18271832
{
18281833
struct nbd_config *config = nbd->config;
1829-
u64 bsize = config->blksize;
1834+
u64 bsize = nbd_blksize(config);
18301835
u64 bytes = config->bytesize;
18311836

18321837
if (info->attrs[NBD_ATTR_SIZE_BYTES])
@@ -1835,7 +1840,7 @@ static int nbd_genl_size_set(struct genl_info *info, struct nbd_device *nbd)
18351840
if (info->attrs[NBD_ATTR_BLOCK_SIZE_BYTES])
18361841
bsize = nla_get_u64(info->attrs[NBD_ATTR_BLOCK_SIZE_BYTES]);
18371842

1838-
if (bytes != config->bytesize || bsize != config->blksize)
1843+
if (bytes != config->bytesize || bsize != nbd_blksize(config))
18391844
return nbd_set_size(nbd, bytes, bsize);
18401845
return 0;
18411846
}

0 commit comments

Comments
 (0)