Prefer recently introduced 'memvalue()' over an ad-hoc 'suffix_kstrtoint()' and 'suffix_kstrtoull()' to parse and basically validate the values passed via 'logbsize', 'allocsize', and 'max_atomic_write' mount options, and reject non-power-of-two values passed via the first and second one early in 'xfs_fs_parse_param()' rather than in 'xfs_fs_validate_params()'.
Signed-off-by: Dmitry Antipov <[email protected]> --- v3: adjust to match new 'memvalue()' syntax v2: rely on 'memvalue()' as (well, IIUC) suggested by Christoph and handle both 'logbsize' and 'allocsize' in 'xfs_fs_parse_param()' --- fs/xfs/xfs_super.c | 127 +++++++++++---------------------------------- 1 file changed, 29 insertions(+), 98 deletions(-) diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c index bc71aa9dcee8..4707cd3acf73 100644 --- a/fs/xfs/xfs_super.c +++ b/fs/xfs/xfs_super.c @@ -1319,77 +1319,6 @@ static const struct super_operations xfs_super_operations = { .show_stats = xfs_fs_show_stats, }; -static int -suffix_kstrtoint( - const char *s, - unsigned int base, - int *res) -{ - int last, shift_left_factor = 0, _res; - char *value; - int ret = 0; - - value = kstrdup(s, GFP_KERNEL); - if (!value) - return -ENOMEM; - - last = strlen(value) - 1; - if (value[last] == 'K' || value[last] == 'k') { - shift_left_factor = 10; - value[last] = '\0'; - } - if (value[last] == 'M' || value[last] == 'm') { - shift_left_factor = 20; - value[last] = '\0'; - } - if (value[last] == 'G' || value[last] == 'g') { - shift_left_factor = 30; - value[last] = '\0'; - } - - if (kstrtoint(value, base, &_res)) - ret = -EINVAL; - kfree(value); - *res = _res << shift_left_factor; - return ret; -} - -static int -suffix_kstrtoull( - const char *s, - unsigned int base, - unsigned long long *res) -{ - int last, shift_left_factor = 0; - unsigned long long _res; - char *value; - int ret = 0; - - value = kstrdup(s, GFP_KERNEL); - if (!value) - return -ENOMEM; - - last = strlen(value) - 1; - if (value[last] == 'K' || value[last] == 'k') { - shift_left_factor = 10; - value[last] = '\0'; - } - if (value[last] == 'M' || value[last] == 'm') { - shift_left_factor = 20; - value[last] = '\0'; - } - if (value[last] == 'G' || value[last] == 'g') { - shift_left_factor = 30; - value[last] = '\0'; - } - - if (kstrtoull(value, base, &_res)) - ret = -EINVAL; - kfree(value); - *res = _res << shift_left_factor; - return ret; -} - static inline void xfs_fs_warn_deprecated( struct fs_context *fc, @@ -1427,8 +1356,8 @@ xfs_fs_parse_param( { struct xfs_mount *parsing_mp = fc->s_fs_info; struct fs_parse_result result; - int size = 0; - int opt; + int opt, ret; + unsigned long long val; BUILD_BUG_ON(XFS_QFLAGS_MNTOPTS & XFS_MOUNT_QUOTA_ALL); @@ -1444,8 +1373,19 @@ xfs_fs_parse_param( parsing_mp->m_logbufs = result.uint_32; return 0; case Opt_logbsize: - if (suffix_kstrtoint(param->string, 10, &parsing_mp->m_logbsize)) + ret = memvalue(param->string, &val); + if (ret) + return ret; + if (val != 0 && + (val < XLOG_MIN_RECORD_BSIZE || + val > XLOG_MAX_RECORD_BSIZE || + !is_power_of_2(val))) { + xfs_warn(parsing_mp, + "invalid logbsize %llu: not a power-of-two in [%u..%u]", + val, XLOG_MIN_RECORD_BSIZE, XLOG_MAX_RECORD_BSIZE); return -EINVAL; + } + parsing_mp->m_logbsize = val; return 0; case Opt_logdev: kfree(parsing_mp->m_logname); @@ -1460,9 +1400,18 @@ xfs_fs_parse_param( return -ENOMEM; return 0; case Opt_allocsize: - if (suffix_kstrtoint(param->string, 10, &size)) + ret = memvalue(param->string, &val); + if (ret) + return ret; + if (val < (1ULL << XFS_MIN_IO_LOG) || + val > (1ULL << XFS_MAX_IO_LOG) || + !is_power_of_2(val)) { + xfs_warn(parsing_mp, + "invalid allocsize %llu: not a power-of-two in [%u..%u]", + val, 1 << XFS_MIN_IO_LOG, 1 << XFS_MAX_IO_LOG); return -EINVAL; - parsing_mp->m_allocsize_log = ffs(size) - 1; + } + parsing_mp->m_allocsize_log = ffs(val) - 1; parsing_mp->m_features |= XFS_FEAT_ALLOCSIZE; return 0; case Opt_grpid: @@ -1570,12 +1519,13 @@ xfs_fs_parse_param( parsing_mp->m_features |= XFS_FEAT_NOLIFETIME; return 0; case Opt_max_atomic_write: - if (suffix_kstrtoull(param->string, 10, - &parsing_mp->m_awu_max_bytes)) { + ret = memvalue(param->string, &val); + if (ret) { xfs_warn(parsing_mp, "max atomic write size must be positive integer"); - return -EINVAL; + return ret; } + parsing_mp->m_awu_max_bytes = val; return 0; default: xfs_warn(parsing_mp, "unknown mount option [%s].", param->key); @@ -1629,25 +1579,6 @@ xfs_fs_validate_params( return -EINVAL; } - if (mp->m_logbsize != -1 && - mp->m_logbsize != 0 && - (mp->m_logbsize < XLOG_MIN_RECORD_BSIZE || - mp->m_logbsize > XLOG_MAX_RECORD_BSIZE || - !is_power_of_2(mp->m_logbsize))) { - xfs_warn(mp, - "invalid logbufsize: %d [not 16k,32k,64k,128k or 256k]", - mp->m_logbsize); - return -EINVAL; - } - - if (xfs_has_allocsize(mp) && - (mp->m_allocsize_log > XFS_MAX_IO_LOG || - mp->m_allocsize_log < XFS_MIN_IO_LOG)) { - xfs_warn(mp, "invalid log iosize: %d [not %d-%d]", - mp->m_allocsize_log, XFS_MIN_IO_LOG, XFS_MAX_IO_LOG); - return -EINVAL; - } - return 0; } -- 2.52.0
