* [PATCH 0/3] kstrtox: introduce memparse_safe()
@ 2023-12-23 9:58 Qu Wenruo
2023-12-23 9:58 ` [PATCH 1/3] kstrtox: introduce a safer version of memparse() Qu Wenruo
` (3 more replies)
0 siblings, 4 replies; 16+ messages in thread
From: Qu Wenruo @ 2023-12-23 9:58 UTC (permalink / raw
To: linux-btrfs, linux-kernel, akpm, christophe.jaillet,
andriy.shevchenko, David.Laight, ddiss
Function memparse() lacks error handling:
- If no valid number string at all
In that case @retptr would just be updated and return value would be
zero.
- No overflown detection
This applies to both the number string part, and the suffixes part.
And since we have no way to indicate errors, we can get weird results
like:
"25E" -> 10376293541461622784 (9E)
This is due to the fact that for "E" suffix, there is only 4 bits
left, and 25 with 60 bits left shift would lead to overflow.
(And decision to support for that "E" suffix is already cursed)
So here we introduce a safer version of it: memparse_safe(), and mark
the original one deprecated.
Unfortunately I didn't find a good way to mark it deprecated, as with
recent -Werror changes, '__deprecated' marco does not seem to warn
anymore.
The new helper has the following advantages:
- Better overflow and invalid string detection
The overflow detection is for both the numberic part, and with the
suffix. Thus above "25E" would be rejected correctly.
The invalid string part means if there is no valid number starts at
the buffer, we return -EINVAL.
- Allow caller to select the suffixes, and saner default ones
The new default one would be "KMGTP", without the cursed and overflow
prone "E".
Some older code like setup_elfcorehdr() would benefit from it, if the
code really wants to only allow "KMG" suffixes.
- Keep the old @retptr behavior
So the existing callers can easily migrate to the new one, without the
need to do extra strsep() work.
- Finally test cases
The test case would cover more things other than the existing kstrtox
tests:
* The @retptr behavior
Either for bad cases, which @retptr should not be touched,
or for good cases, the @retptr is properly advanced,
* Return value verification
Make sure we distinguish -EINVAL and -ERANGE correctly.
With the new helper, migrate btrfs to the interface, and since the
@retptr behavior is the same, we won't cause any behavior change.
Qu Wenruo (3):
kstrtox: introduce a safer version of memparse()
kstrtox: add unit tests for memparse_safe()
btrfs: migrate to the newer memparse_safe() helper
fs/btrfs/ioctl.c | 8 +-
fs/btrfs/super.c | 8 ++
fs/btrfs/sysfs.c | 14 ++-
include/linux/kernel.h | 8 +-
include/linux/kstrtox.h | 15 +++
lib/cmdline.c | 5 +-
lib/kstrtox.c | 96 ++++++++++++++++++
lib/test-kstrtox.c | 217 ++++++++++++++++++++++++++++++++++++++++
8 files changed, 364 insertions(+), 7 deletions(-)
--
2.43.0
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 1/3] kstrtox: introduce a safer version of memparse()
2023-12-23 9:58 [PATCH 0/3] kstrtox: introduce memparse_safe() Qu Wenruo
@ 2023-12-23 9:58 ` Qu Wenruo
2023-12-27 13:26 ` David Disseldorp
2023-12-23 9:58 ` [PATCH 2/3] kstrtox: add unit tests for memparse_safe() Qu Wenruo
` (2 subsequent siblings)
3 siblings, 1 reply; 16+ messages in thread
From: Qu Wenruo @ 2023-12-23 9:58 UTC (permalink / raw
To: linux-btrfs, linux-kernel, akpm, christophe.jaillet,
andriy.shevchenko, David.Laight, ddiss
[BUGS]
Function memparse() lacks error handling:
- If no valid number string at all
In that case @retptr would just be updated and return value would be
zero.
- No overflown detection
This applies to both the number string part, and the suffixes part.
And since we have no way to indicate errors, we can get weird results
like:
"25E" -> 10376293541461622784 (9E)
This is due to the fact that for "E" suffix, there is only 4 bits
left, and 25 with 60 bits left shift would lead to overflow.
[CAUSE]
The root cause is already mentioned in the comments of the function, the
usage of simple_strtoull() is the source of evil.
Furthermore the function prototype is no good either, just returning an
unsigned long long gives us no way to indicate an error.
[FIX]
Due to the prototype limits, we can not have a drop-in replacement for
memparse().
This patch can only help by introduce a new helper, memparse_safe(), and
mark the old memparse() deprecated.
The new memparse_safe() has the following improvement:
- Invalid string detection
If no number string can be detected at all, -EINVAL would be returned.
- Better overflow detection
Both the string part and the extra left shift would have overflow
detection.
Any overflow would result -ERANGE.
- Safer default suffix selection
The helper allows the caller to choose the suffixes that they want to
use.
But only "KMGTP" are recommended by default since the "E" leaves only
4 bits before overflow.
For those callers really know what they are doing, they can still
manually to include all suffixes.
Due to the prototype change, callers should migrate to the new one and
change their code and add extra error handling.
Signed-off-by: Qu Wenruo <wqu@suse.com>
---
include/linux/kernel.h | 8 +++-
include/linux/kstrtox.h | 15 +++++++
lib/cmdline.c | 5 ++-
lib/kstrtox.c | 96 +++++++++++++++++++++++++++++++++++++++++
4 files changed, 122 insertions(+), 2 deletions(-)
diff --git a/include/linux/kernel.h b/include/linux/kernel.h
index d9ad21058eed..b1b6da60ea43 100644
--- a/include/linux/kernel.h
+++ b/include/linux/kernel.h
@@ -201,7 +201,13 @@ void do_exit(long error_code) __noreturn;
extern int get_option(char **str, int *pint);
extern char *get_options(const char *str, int nints, int *ints);
-extern unsigned long long memparse(const char *ptr, char **retptr);
+
+/*
+ * DEPRECATED, lack of any kind of error handling.
+ *
+ * Use memparse_safe() from lib/kstrtox.c instead.
+ */
+extern __deprecated unsigned long long memparse(const char *ptr, char **retptr);
extern bool parse_option_str(const char *str, const char *option);
extern char *next_arg(char *args, char **param, char **val);
diff --git a/include/linux/kstrtox.h b/include/linux/kstrtox.h
index 7fcf29a4e0de..53a1e059dd31 100644
--- a/include/linux/kstrtox.h
+++ b/include/linux/kstrtox.h
@@ -9,6 +9,21 @@
int __must_check _kstrtoul(const char *s, unsigned int base, unsigned long *res);
int __must_check _kstrtol(const char *s, unsigned int base, long *res);
+enum memparse_suffix {
+ MEMPARSE_SUFFIX_K = 1 << 0,
+ MEMPARSE_SUFFIX_M = 1 << 1,
+ MEMPARSE_SUFFIX_G = 1 << 2,
+ MEMPARSE_SUFFIX_T = 1 << 3,
+ MEMPARSE_SUFFIX_P = 1 << 4,
+ MEMPARSE_SUFFIX_E = 1 << 5,
+};
+
+#define MEMPARSE_SUFFIXES_DEFAULT (MEMPARSE_SUFFIX_K | MEMPARSE_SUFFIX_M |\
+ MEMPARSE_SUFFIX_G | MEMPARSE_SUFFIX_T |\
+ MEMPARSE_SUFFIX_P)
+
+int __must_check memparse_safe(const char *s, enum memparse_suffix suffixes,
+ unsigned long long *res, char **retptr);
int __must_check kstrtoull(const char *s, unsigned int base, unsigned long long *res);
int __must_check kstrtoll(const char *s, unsigned int base, long long *res);
diff --git a/lib/cmdline.c b/lib/cmdline.c
index 90ed997d9570..d379157de349 100644
--- a/lib/cmdline.c
+++ b/lib/cmdline.c
@@ -139,12 +139,15 @@ char *get_options(const char *str, int nints, int *ints)
EXPORT_SYMBOL(get_options);
/**
- * memparse - parse a string with mem suffixes into a number
+ * memparse - DEPRECATED, parse a string with mem suffixes into a number
* @ptr: Where parse begins
* @retptr: (output) Optional pointer to next char after parse completes
*
+ * There is no way to handle errors, and no overflown detection and string
+ * sanity checks.
* Parses a string into a number. The number stored at @ptr is
* potentially suffixed with K, M, G, T, P, E.
+ *
*/
unsigned long long memparse(const char *ptr, char **retptr)
diff --git a/lib/kstrtox.c b/lib/kstrtox.c
index d586e6af5e5a..38c772097301 100644
--- a/lib/kstrtox.c
+++ b/lib/kstrtox.c
@@ -113,6 +113,102 @@ static int _kstrtoull(const char *s, unsigned int base, unsigned long long *res)
return 0;
}
+/**
+ * memparse_safe - convert a string to an unsigned long long, safer version of
+ * memparse()
+ *
+ * @s: The start of the string. Must be null-terminated.
+ * The base would be determined automatically, if it starts with
+ * "0x" the base would be 16, if it starts with "0" the base
+ * would be 8, otherwise the base would be 10.
+ * After a valid number string, there can be at most one
+ * case-insensive suffix character, specified by the @suffixes
+ * parameter.
+ *
+ * @suffixes: The suffixes which should be parsed. Use logical ORed
+ * memparse_suffix enum to indicate the supported suffixes.
+ * The suffixes are case-insensive, all 2 ^ 10 based.
+ * Supported ones are "KMGPTE".
+ * NOTE: If one suffix out of the supported one is hit, it would
+ * end the parse normally, with @retptr pointed to the unsupported
+ * suffix.
+ *
+ * @res: Where to write the result.
+ *
+ * @retptr: (output) Optional pointer to the next char after parse completes.
+ *
+ * Return 0 if any valid numberic string can be parsed, and @retptr updated.
+ * Return -INVALID if no valid number string can be found.
+ * Return -ERANGE if the number overflows.
+ * For minus return values, @retptr would not be updated.
+ */
+noinline int memparse_safe(const char *s, enum memparse_suffix suffixes,
+ unsigned long long *res, char **retptr)
+{
+ unsigned long long value;
+ unsigned int rv;
+ int shift = 0;
+ int base = 0;
+
+ s = _parse_integer_fixup_radix(s, &base);
+ rv = _parse_integer(s, base, &value);
+ if (rv & KSTRTOX_OVERFLOW)
+ return -ERANGE;
+ if (rv == 0)
+ return -EINVAL;
+
+ s += rv;
+ switch (*s) {
+ case 'K':
+ case 'k':
+ if (!(suffixes & MEMPARSE_SUFFIX_K))
+ break;
+ shift = 10;
+ break;
+ case 'M':
+ case 'm':
+ if (!(suffixes & MEMPARSE_SUFFIX_M))
+ break;
+ shift = 20;
+ break;
+ case 'G':
+ case 'g':
+ if (!(suffixes & MEMPARSE_SUFFIX_G))
+ break;
+ shift = 30;
+ break;
+ case 'T':
+ case 't':
+ if (!(suffixes & MEMPARSE_SUFFIX_T))
+ break;
+ shift = 40;
+ break;
+ case 'P':
+ case 'p':
+ if (!(suffixes & MEMPARSE_SUFFIX_P))
+ break;
+ shift = 50;
+ break;
+ case 'E':
+ case 'e':
+ if (!(suffixes & MEMPARSE_SUFFIX_E))
+ break;
+ shift = 60;
+ break;
+ }
+ if (shift) {
+ s++;
+ if (value >> (64 - shift))
+ return -ERANGE;
+ value <<= shift;
+ }
+ *res = value;
+ if (retptr)
+ *retptr = (char *)s;
+ return 0;
+}
+EXPORT_SYMBOL(memparse_safe);
+
/**
* kstrtoull - convert a string to an unsigned long long
* @s: The start of the string. The string must be null-terminated, and may also
--
2.43.0
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH 2/3] kstrtox: add unit tests for memparse_safe()
2023-12-23 9:58 [PATCH 0/3] kstrtox: introduce memparse_safe() Qu Wenruo
2023-12-23 9:58 ` [PATCH 1/3] kstrtox: introduce a safer version of memparse() Qu Wenruo
@ 2023-12-23 9:58 ` Qu Wenruo
2023-12-26 6:36 ` kernel test robot
2023-12-23 9:58 ` [PATCH 3/3] btrfs: migrate to the newer memparse_safe() helper Qu Wenruo
2023-12-27 16:12 ` [PATCH 0/3] kstrtox: introduce memparse_safe() Andy Shevchenko
3 siblings, 1 reply; 16+ messages in thread
From: Qu Wenruo @ 2023-12-23 9:58 UTC (permalink / raw
To: linux-btrfs, linux-kernel, akpm, christophe.jaillet,
andriy.shevchenko, David.Laight, ddiss
The new tests cases for memparse_safe() include:
- The existing test cases for kstrtoull()
Including all the 3 bases (8, 10, 16), and all the ok and failure
cases.
Although there are something we need to verify specific for
memparse_safe():
* @retptr and @value are not modified for failure cases
* return value are correct for failure cases
* @retptr is correct for the good cases
- New test cases
Not only testing the result value, but also the @retptr, including:
* good cases with extra tailing chars, but without valid prefix
The @retptr should point to the first char after a valid string.
3 cases for all the 3 bases.
* good cases with extra tailing chars, with valid prefix
5 cases for all the suffixes.
* bad cases without any number but stray suffix
Should be rejected with -EINVAL
Signed-off-by: Qu Wenruo <wqu@suse.com>
---
lib/test-kstrtox.c | 217 +++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 217 insertions(+)
diff --git a/lib/test-kstrtox.c b/lib/test-kstrtox.c
index f355f67169b6..094beab3dfac 100644
--- a/lib/test-kstrtox.c
+++ b/lib/test-kstrtox.c
@@ -268,6 +268,219 @@ static void __init test_kstrtoll_ok(void)
TEST_OK(kstrtoll, long long, "%lld", test_ll_ok);
}
+/*
+ * The special pattern to make sure the result is not modified for error cases.
+ */
+#define ULL_PATTERN (0xefefefef7a7a7a7aULL)
+
+/* Want to include "E" suffix for full coverage. */
+#define MEMPARSE_TEST_SUFFIX (MEMPARSE_SUFFIX_K | MEMPARSE_SUFFIX_M |\
+ MEMPARSE_SUFFIX_G | MEMPARSE_SUFFIX_T |\
+ MEMPARSE_SUFFIX_P | MEMPARSE_SUFFIX_E)
+
+static void __init test_memparse_safe_fail(void)
+{
+ struct memparse_test_fail {
+ const char *str;
+ /* Expected error number, either -EINVAL or -ERANGE. */
+ unsigned int expected_ret;
+ };
+ static const struct memparse_test_fail tests[] __initconst = {
+ /* No valid string can be found at all. */
+ {"", -EINVAL},
+ {"\n", -EINVAL},
+ {"\n0", -EINVAL},
+ {"+", -EINVAL},
+ {"-", -EINVAL},
+
+ /*
+ * No support for any leading "+-" chars, even followed by a valid
+ * number.
+ */
+ {"-0", -EINVAL},
+ {"+0", -EINVAL},
+ {"-1", -EINVAL},
+ {"+1", -EINVAL},
+
+ /* Stray suffix would also be rejected. */
+ {"K", -EINVAL},
+ {"P", -EINVAL},
+
+ /* Overflow in the string itself*/
+ {"18446744073709551616", -ERANGE},
+ {"02000000000000000000000", -ERANGE},
+ {"0x10000000000000000", -ERANGE},
+
+ /*
+ * Good string but would overflow with suffix.
+ *
+ * Note, for "E" suffix, one should not use with hex, or "0x1E"
+ * would be treated as 0x1e (30 in decimal), not 0x1 and "E" suffix.
+ * Another reason "E" suffix is cursed.
+ */
+ {"16E", -ERANGE},
+ {"020E", -ERANGE},
+ {"16384P", -ERANGE},
+ {"040000P", -ERANGE},
+ {"16777216T", -ERANGE},
+ {"0100000000T", -ERANGE},
+ {"17179869184G", -ERANGE},
+ {"0200000000000G", -ERANGE},
+ {"17592186044416M", -ERANGE},
+ {"0400000000000000M", -ERANGE},
+ {"18014398509481984K", -ERANGE},
+ {"01000000000000000000K", -ERANGE},
+ };
+ unsigned int i;
+
+ for_each_test(i, tests) {
+ const struct memparse_test_fail *t = &tests[i];
+ unsigned long long tmp = ULL_PATTERN;
+ char *retptr = (char *)ULL_PATTERN;
+ int ret;
+
+ ret = memparse_safe(t->str, MEMPARSE_TEST_SUFFIX, &tmp, &retptr);
+ if (ret != t->expected_ret) {
+ WARN(1, "str '%s', expected ret %d got %d\n", t->str,
+ t->expected_ret, ret);
+ continue;
+ }
+ if (tmp != ULL_PATTERN)
+ WARN(1, "str '%s' failed as expected, but result got modified",
+ t->str);
+ if (retptr != (char *)ULL_PATTERN)
+ WARN(1, "str '%s' failed as expected, but pointer got modified",
+ t->str);
+ }
+}
+
+static void __init test_memparse_safe_ok(void)
+{
+ struct memparse_test_ok {
+ const char *str;
+ unsigned long long expected_value;
+ /* How many bytes the @retptr pointer should be moved forward. */
+ unsigned int retptr_off;
+ };
+ static DEFINE_TEST_OK(struct memparse_test_ok, tests) = {
+ /*
+ * The same pattern of kstrtoull, just with extra @retptr
+ * verification.
+ */
+ {"0", 0ULL, 1},
+ {"1", 1ULL, 1},
+ {"127", 127ULL, 3},
+ {"128", 128ULL, 3},
+ {"129", 129ULL, 3},
+ {"255", 255ULL, 3},
+ {"256", 256ULL, 3},
+ {"257", 257ULL, 3},
+ {"32767", 32767ULL, 5},
+ {"32768", 32768ULL, 5},
+ {"32769", 32769ULL, 5},
+ {"65535", 65535ULL, 5},
+ {"65536", 65536ULL, 5},
+ {"65537", 65537ULL, 5},
+ {"2147483647", 2147483647ULL, 10},
+ {"2147483648", 2147483648ULL, 10},
+ {"2147483649", 2147483649ULL, 10},
+ {"4294967295", 4294967295ULL, 10},
+ {"4294967296", 4294967296ULL, 10},
+ {"4294967297", 4294967297ULL, 10},
+ {"9223372036854775807", 9223372036854775807ULL, 19},
+ {"9223372036854775808", 9223372036854775808ULL, 19},
+ {"9223372036854775809", 9223372036854775809ULL, 19},
+ {"18446744073709551614", 18446744073709551614ULL, 20},
+ {"18446744073709551615", 18446744073709551615ULL, 20},
+
+ {"00", 00ULL, 2},
+ {"01", 01ULL, 2},
+ {"0177", 0177ULL, 4},
+ {"0200", 0200ULL, 4},
+ {"0201", 0201ULL, 4},
+ {"0377", 0377ULL, 4},
+ {"0400", 0400ULL, 4},
+ {"0401", 0401ULL, 4},
+ {"077777", 077777ULL, 6},
+ {"0100000", 0100000ULL, 7},
+ {"0100001", 0100001ULL, 7},
+ {"0177777", 0177777ULL, 7},
+ {"0200000", 0200000ULL, 7},
+ {"0200001", 0200001ULL, 7},
+ {"017777777777", 017777777777ULL, 12},
+ {"020000000000", 020000000000ULL, 12},
+ {"020000000001", 020000000001ULL, 12},
+ {"037777777777", 037777777777ULL, 12},
+ {"040000000000", 040000000000ULL, 12},
+ {"040000000001", 040000000001ULL, 12},
+ {"0777777777777777777777", 0777777777777777777777ULL, 22},
+ {"01000000000000000000000", 01000000000000000000000ULL, 23},
+ {"01000000000000000000001", 01000000000000000000001ULL, 23},
+ {"01777777777777777777776", 01777777777777777777776ULL, 23},
+ {"01777777777777777777777", 01777777777777777777777ULL, 23},
+
+ {"0x0", 0x0ULL, 3},
+ {"0x1", 0x1ULL, 3},
+ {"0x7f", 0x7fULL, 4},
+ {"0x80", 0x80ULL, 4},
+ {"0x81", 0x81ULL, 4},
+ {"0xff", 0xffULL, 4},
+ {"0x100", 0x100ULL, 5},
+ {"0x101", 0x101ULL, 5},
+ {"0x7fff", 0x7fffULL, 6},
+ {"0x8000", 0x8000ULL, 6},
+ {"0x8001", 0x8001ULL, 6},
+ {"0xffff", 0xffffULL, 6},
+ {"0x10000", 0x10000ULL, 7},
+ {"0x10001", 0x10001ULL, 7},
+ {"0x7fffffff", 0x7fffffffULL, 10},
+ {"0x80000000", 0x80000000ULL, 10},
+ {"0x80000001", 0x80000001ULL, 10},
+ {"0xffffffff", 0xffffffffULL, 10},
+ {"0x100000000", 0x100000000ULL, 11},
+ {"0x100000001", 0x100000001ULL, 11},
+ {"0x7fffffffffffffff", 0x7fffffffffffffffULL, 18},
+ {"0x8000000000000000", 0x8000000000000000ULL, 18},
+ {"0x8000000000000001", 0x8000000000000001ULL, 18},
+ {"0xfffffffffffffffe", 0xfffffffffffffffeULL, 18},
+ {"0xffffffffffffffff", 0xffffffffffffffffULL, 18},
+
+ /* Now with extra non-suffix chars to test @retptr update. */
+ {"1q84", 1, 1},
+ {"02o45", 2, 2},
+ {"0xffvii", 0xff, 4},
+
+ /*
+ * Finally one suffix then tailing chars, to test the @retptr
+ * behavior.
+ */
+ {"68k ", 69632, 3},
+ {"8MS", 8388608, 2},
+ {"0xaeGis", 0x2b80000000, 5},
+ {"0xaTx", 0xa0000000000, 4},
+ {"3E8", 0x3000000000000000, 2},
+ };
+ unsigned int i;
+
+ for_each_test(i, tests) {
+ const struct memparse_test_ok *t = &tests[i];
+ unsigned long long tmp;
+ char *retptr;
+ int ret;
+
+ ret = memparse_safe(t->str, MEMPARSE_TEST_SUFFIX, &tmp, &retptr);
+ if (ret != 0) {
+ WARN(1, "str '%s', expected ret 0 got %d\n", t->str, ret);
+ continue;
+ }
+ if (tmp != t->expected_value)
+ WARN(1, "str '%s' incorrect result, expected %llu got %llu",
+ t->str, t->expected_value, tmp);
+ if (retptr != t->str + t->retptr_off)
+ WARN(1, "str '%s' incorrect endptr, expected %u got %zu",
+ t->str, t->retptr_off, retptr - t->str);
+ }
+}
static void __init test_kstrtoll_fail(void)
{
static DEFINE_TEST_FAIL(test_ll_fail) = {
@@ -710,6 +923,10 @@ static int __init test_kstrtox_init(void)
test_kstrtoll_ok();
test_kstrtoll_fail();
+ test_memparse_safe_ok();
+ test_memparse_safe_fail();
+
+
test_kstrtou64_ok();
test_kstrtou64_fail();
test_kstrtos64_ok();
--
2.43.0
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH 3/3] btrfs: migrate to the newer memparse_safe() helper
2023-12-23 9:58 [PATCH 0/3] kstrtox: introduce memparse_safe() Qu Wenruo
2023-12-23 9:58 ` [PATCH 1/3] kstrtox: introduce a safer version of memparse() Qu Wenruo
2023-12-23 9:58 ` [PATCH 2/3] kstrtox: add unit tests for memparse_safe() Qu Wenruo
@ 2023-12-23 9:58 ` Qu Wenruo
2023-12-26 4:53 ` kernel test robot
` (2 more replies)
2023-12-27 16:12 ` [PATCH 0/3] kstrtox: introduce memparse_safe() Andy Shevchenko
3 siblings, 3 replies; 16+ messages in thread
From: Qu Wenruo @ 2023-12-23 9:58 UTC (permalink / raw
To: linux-btrfs, linux-kernel, akpm, christophe.jaillet,
andriy.shevchenko, David.Laight, ddiss
The new helper has better error report and correct overflow detection,
furthermore the old @retptr behavior is also kept, thus there should be
no behavior change.
Signed-off-by: Qu Wenruo <wqu@suse.com>
---
fs/btrfs/ioctl.c | 8 ++++++--
fs/btrfs/super.c | 8 ++++++++
fs/btrfs/sysfs.c | 14 +++++++++++---
3 files changed, 25 insertions(+), 5 deletions(-)
diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
index 4e50b62db2a8..8bfd4b4ccf02 100644
--- a/fs/btrfs/ioctl.c
+++ b/fs/btrfs/ioctl.c
@@ -1175,8 +1175,12 @@ static noinline int btrfs_ioctl_resize(struct file *file,
mod = 1;
sizestr++;
}
- new_size = memparse(sizestr, &retptr);
- if (*retptr != '\0' || new_size == 0) {
+
+ ret = memparse_safe(sizestr, MEMPARSE_SUFFIXES_DEFAULT,
+ &new_size, &retptr);
+ if (ret < 0)
+ goto out_finish;
+ if (*retptr != '\0') {
ret = -EINVAL;
goto out_finish;
}
diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
index 3a677b808f0f..2bb6ea525e89 100644
--- a/fs/btrfs/super.c
+++ b/fs/btrfs/super.c
@@ -400,6 +400,14 @@ static int btrfs_parse_param(struct fs_context *fc, struct fs_parameter *param)
ctx->thread_pool_size = result.uint_32;
break;
case Opt_max_inline:
+ int ret;
+
+ ret = memparse_safe(param->string, MEMPARSE_SUFFIXES_DEFAULT,
+ &ctx->max_inline, NULL);
+ if (ret < 0) {
+ btrfs_err(NULL, "invalid string \"%s\"", param->string);
+ return ret;
+ }
ctx->max_inline = memparse(param->string, NULL);
break;
case Opt_acl:
diff --git a/fs/btrfs/sysfs.c b/fs/btrfs/sysfs.c
index 84c05246ffd8..6846572496a6 100644
--- a/fs/btrfs/sysfs.c
+++ b/fs/btrfs/sysfs.c
@@ -762,6 +762,7 @@ static ssize_t btrfs_chunk_size_store(struct kobject *kobj,
struct btrfs_fs_info *fs_info = to_fs_info(get_btrfs_kobj(kobj));
char *retptr;
u64 val;
+ int ret;
if (!capable(CAP_SYS_ADMIN))
return -EPERM;
@@ -776,7 +777,10 @@ static ssize_t btrfs_chunk_size_store(struct kobject *kobj,
if (space_info->flags & BTRFS_BLOCK_GROUP_SYSTEM)
return -EPERM;
- val = memparse(buf, &retptr);
+ ret = memparse_safe(buf, MEMPARSE_SUFFIXES_DEFAULT, &val, &retptr);
+ if (ret < 0)
+ return ret;
+
/* There could be trailing '\n', also catch any typos after the value */
retptr = skip_spaces(retptr);
if (*retptr != 0 || val == 0)
@@ -1779,10 +1783,14 @@ static ssize_t btrfs_devinfo_scrub_speed_max_store(struct kobject *kobj,
{
struct btrfs_device *device = container_of(kobj, struct btrfs_device,
devid_kobj);
- char *endptr;
unsigned long long limit;
+ char *endptr;
+ int ret;
+
+ ret = memparse_safe(buf, MEMPARSE_SUFFIXES_DEFAULT, &limit, &endptr);
+ if (ret < 0)
+ return ret;
- limit = memparse(buf, &endptr);
/* There could be trailing '\n', also catch any typos after the value. */
endptr = skip_spaces(endptr);
if (*endptr != 0)
--
2.43.0
^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH 3/3] btrfs: migrate to the newer memparse_safe() helper
@ 2023-12-26 1:24 kernel test robot
0 siblings, 0 replies; 16+ messages in thread
From: kernel test robot @ 2023-12-26 1:24 UTC (permalink / raw
To: oe-kbuild; +Cc: lkp
::::::
:::::: Manual check reason: "low confidence static check warning: fs/btrfs/super.c:403:17: sparse: sparse: typename in expression"
::::::
BCC: lkp@intel.com
CC: oe-kbuild-all@lists.linux.dev
In-Reply-To: <6dfa53ded887caa2269c1beeaedcff086342339a.1703324146.git.wqu@suse.com>
References: <6dfa53ded887caa2269c1beeaedcff086342339a.1703324146.git.wqu@suse.com>
TO: Qu Wenruo <wqu@suse.com>
TO: linux-btrfs@vger.kernel.org
TO: linux-kernel@vger.kernel.org
TO: akpm@linux-foundation.org
TO: christophe.jaillet@wanadoo.fr
TO: andriy.shevchenko@linux.intel.com
TO: David.Laight@ACULAB.COM
TO: ddiss@suse.de
Hi Qu,
kernel test robot noticed the following build warnings:
[auto build test WARNING on kdave/for-next]
[also build test WARNING on next-20231222]
[cannot apply to akpm-mm/mm-nonmm-unstable akpm-mm/mm-everything linus/master v6.7-rc7]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Qu-Wenruo/kstrtox-introduce-a-safer-version-of-memparse/20231225-151921
base: https://git.kernel.org/pub/scm/linux/kernel/git/kdave/linux.git for-next
patch link: https://lore.kernel.org/r/6dfa53ded887caa2269c1beeaedcff086342339a.1703324146.git.wqu%40suse.com
patch subject: [PATCH 3/3] btrfs: migrate to the newer memparse_safe() helper
:::::: branch date: 18 hours ago
:::::: commit date: 18 hours ago
config: s390-randconfig-r112-20231226 (https://download.01.org/0day-ci/archive/20231226/202312260953.igCSOI51-lkp@intel.com/config)
compiler: s390-linux-gcc (GCC) 13.2.0
reproduce: (https://download.01.org/0day-ci/archive/20231226/202312260953.igCSOI51-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/r/202312260953.igCSOI51-lkp@intel.com/
sparse warnings: (new ones prefixed by >>)
>> fs/btrfs/super.c:403:17: sparse: sparse: typename in expression
fs/btrfs/super.c:403:21: sparse: sparse: Expected ; at end of statement
fs/btrfs/super.c:403:21: sparse: sparse: got ret
fs/btrfs/super.c:403:17: sparse: sparse: undefined identifier 'int'
fs/btrfs/super.c:405:17: sparse: sparse: undefined identifier 'ret'
fs/btrfs/super.c:407:21: sparse: sparse: undefined identifier 'ret'
fs/btrfs/super.c:409:32: sparse: sparse: undefined identifier 'ret'
vim +403 fs/btrfs/super.c
15ddcdd34ebfe7 Josef Bacik 2023-11-22 261
17b3612022fe53 Josef Bacik 2023-11-22 262 static int btrfs_parse_param(struct fs_context *fc, struct fs_parameter *param)
17b3612022fe53 Josef Bacik 2023-11-22 263 {
17b3612022fe53 Josef Bacik 2023-11-22 264 struct btrfs_fs_context *ctx = fc->fs_private;
17b3612022fe53 Josef Bacik 2023-11-22 265 struct fs_parse_result result;
17b3612022fe53 Josef Bacik 2023-11-22 266 int opt;
17b3612022fe53 Josef Bacik 2023-11-22 267
17b3612022fe53 Josef Bacik 2023-11-22 268 opt = fs_parse(fc, btrfs_fs_parameters, param, &result);
17b3612022fe53 Josef Bacik 2023-11-22 269 if (opt < 0)
17b3612022fe53 Josef Bacik 2023-11-22 270 return opt;
17b3612022fe53 Josef Bacik 2023-11-22 271
17b3612022fe53 Josef Bacik 2023-11-22 272 switch (opt) {
17b3612022fe53 Josef Bacik 2023-11-22 273 case Opt_degraded:
17b3612022fe53 Josef Bacik 2023-11-22 274 btrfs_set_opt(ctx->mount_opt, DEGRADED);
17b3612022fe53 Josef Bacik 2023-11-22 275 break;
17b3612022fe53 Josef Bacik 2023-11-22 276 case Opt_subvol_empty:
17b3612022fe53 Josef Bacik 2023-11-22 277 /*
17b3612022fe53 Josef Bacik 2023-11-22 278 * This exists because we used to allow it on accident, so we're
17b3612022fe53 Josef Bacik 2023-11-22 279 * keeping it to maintain ABI. See 37becec95ac3 ("Btrfs: allow
17b3612022fe53 Josef Bacik 2023-11-22 280 * empty subvol= again").
17b3612022fe53 Josef Bacik 2023-11-22 281 */
17b3612022fe53 Josef Bacik 2023-11-22 282 break;
17b3612022fe53 Josef Bacik 2023-11-22 283 case Opt_subvol:
17b3612022fe53 Josef Bacik 2023-11-22 284 kfree(ctx->subvol_name);
17b3612022fe53 Josef Bacik 2023-11-22 285 ctx->subvol_name = kstrdup(param->string, GFP_KERNEL);
17b3612022fe53 Josef Bacik 2023-11-22 286 if (!ctx->subvol_name)
17b3612022fe53 Josef Bacik 2023-11-22 287 return -ENOMEM;
17b3612022fe53 Josef Bacik 2023-11-22 288 break;
17b3612022fe53 Josef Bacik 2023-11-22 289 case Opt_subvolid:
17b3612022fe53 Josef Bacik 2023-11-22 290 ctx->subvol_objectid = result.uint_64;
17b3612022fe53 Josef Bacik 2023-11-22 291
17b3612022fe53 Josef Bacik 2023-11-22 292 /* subvolid=0 means give me the original fs_tree. */
17b3612022fe53 Josef Bacik 2023-11-22 293 if (!ctx->subvol_objectid)
17b3612022fe53 Josef Bacik 2023-11-22 294 ctx->subvol_objectid = BTRFS_FS_TREE_OBJECTID;
17b3612022fe53 Josef Bacik 2023-11-22 295 break;
17b3612022fe53 Josef Bacik 2023-11-22 296 case Opt_device: {
17b3612022fe53 Josef Bacik 2023-11-22 297 struct btrfs_device *device;
17b3612022fe53 Josef Bacik 2023-11-22 298 blk_mode_t mode = sb_open_mode(fc->sb_flags);
17b3612022fe53 Josef Bacik 2023-11-22 299
17b3612022fe53 Josef Bacik 2023-11-22 300 mutex_lock(&uuid_mutex);
17b3612022fe53 Josef Bacik 2023-11-22 301 device = btrfs_scan_one_device(param->string, mode, false);
17b3612022fe53 Josef Bacik 2023-11-22 302 mutex_unlock(&uuid_mutex);
17b3612022fe53 Josef Bacik 2023-11-22 303 if (IS_ERR(device))
17b3612022fe53 Josef Bacik 2023-11-22 304 return PTR_ERR(device);
17b3612022fe53 Josef Bacik 2023-11-22 305 break;
17b3612022fe53 Josef Bacik 2023-11-22 306 }
17b3612022fe53 Josef Bacik 2023-11-22 307 case Opt_datasum:
17b3612022fe53 Josef Bacik 2023-11-22 308 if (result.negated) {
17b3612022fe53 Josef Bacik 2023-11-22 309 btrfs_set_opt(ctx->mount_opt, NODATASUM);
17b3612022fe53 Josef Bacik 2023-11-22 310 } else {
17b3612022fe53 Josef Bacik 2023-11-22 311 btrfs_clear_opt(ctx->mount_opt, NODATACOW);
17b3612022fe53 Josef Bacik 2023-11-22 312 btrfs_clear_opt(ctx->mount_opt, NODATASUM);
17b3612022fe53 Josef Bacik 2023-11-22 313 }
17b3612022fe53 Josef Bacik 2023-11-22 314 break;
17b3612022fe53 Josef Bacik 2023-11-22 315 case Opt_datacow:
17b3612022fe53 Josef Bacik 2023-11-22 316 if (result.negated) {
17b3612022fe53 Josef Bacik 2023-11-22 317 btrfs_clear_opt(ctx->mount_opt, COMPRESS);
17b3612022fe53 Josef Bacik 2023-11-22 318 btrfs_clear_opt(ctx->mount_opt, FORCE_COMPRESS);
17b3612022fe53 Josef Bacik 2023-11-22 319 btrfs_set_opt(ctx->mount_opt, NODATACOW);
17b3612022fe53 Josef Bacik 2023-11-22 320 btrfs_set_opt(ctx->mount_opt, NODATASUM);
17b3612022fe53 Josef Bacik 2023-11-22 321 } else {
17b3612022fe53 Josef Bacik 2023-11-22 322 btrfs_clear_opt(ctx->mount_opt, NODATACOW);
17b3612022fe53 Josef Bacik 2023-11-22 323 }
17b3612022fe53 Josef Bacik 2023-11-22 324 break;
17b3612022fe53 Josef Bacik 2023-11-22 325 case Opt_compress_force:
17b3612022fe53 Josef Bacik 2023-11-22 326 case Opt_compress_force_type:
17b3612022fe53 Josef Bacik 2023-11-22 327 btrfs_set_opt(ctx->mount_opt, FORCE_COMPRESS);
17b3612022fe53 Josef Bacik 2023-11-22 328 fallthrough;
17b3612022fe53 Josef Bacik 2023-11-22 329 case Opt_compress:
17b3612022fe53 Josef Bacik 2023-11-22 330 case Opt_compress_type:
17b3612022fe53 Josef Bacik 2023-11-22 331 if (opt == Opt_compress || opt == Opt_compress_force) {
17b3612022fe53 Josef Bacik 2023-11-22 332 ctx->compress_type = BTRFS_COMPRESS_ZLIB;
17b3612022fe53 Josef Bacik 2023-11-22 333 ctx->compress_level = BTRFS_ZLIB_DEFAULT_LEVEL;
17b3612022fe53 Josef Bacik 2023-11-22 334 btrfs_set_opt(ctx->mount_opt, COMPRESS);
17b3612022fe53 Josef Bacik 2023-11-22 335 btrfs_clear_opt(ctx->mount_opt, NODATACOW);
17b3612022fe53 Josef Bacik 2023-11-22 336 btrfs_clear_opt(ctx->mount_opt, NODATASUM);
17b3612022fe53 Josef Bacik 2023-11-22 337 } else if (strncmp(param->string, "zlib", 4) == 0) {
17b3612022fe53 Josef Bacik 2023-11-22 338 ctx->compress_type = BTRFS_COMPRESS_ZLIB;
17b3612022fe53 Josef Bacik 2023-11-22 339 ctx->compress_level =
17b3612022fe53 Josef Bacik 2023-11-22 340 btrfs_compress_str2level(BTRFS_COMPRESS_ZLIB,
17b3612022fe53 Josef Bacik 2023-11-22 341 param->string + 4);
17b3612022fe53 Josef Bacik 2023-11-22 342 btrfs_set_opt(ctx->mount_opt, COMPRESS);
17b3612022fe53 Josef Bacik 2023-11-22 343 btrfs_clear_opt(ctx->mount_opt, NODATACOW);
17b3612022fe53 Josef Bacik 2023-11-22 344 btrfs_clear_opt(ctx->mount_opt, NODATASUM);
17b3612022fe53 Josef Bacik 2023-11-22 345 } else if (strncmp(param->string, "lzo", 3) == 0) {
17b3612022fe53 Josef Bacik 2023-11-22 346 ctx->compress_type = BTRFS_COMPRESS_LZO;
17b3612022fe53 Josef Bacik 2023-11-22 347 ctx->compress_level = 0;
17b3612022fe53 Josef Bacik 2023-11-22 348 btrfs_set_opt(ctx->mount_opt, COMPRESS);
17b3612022fe53 Josef Bacik 2023-11-22 349 btrfs_clear_opt(ctx->mount_opt, NODATACOW);
17b3612022fe53 Josef Bacik 2023-11-22 350 btrfs_clear_opt(ctx->mount_opt, NODATASUM);
17b3612022fe53 Josef Bacik 2023-11-22 351 } else if (strncmp(param->string, "zstd", 4) == 0) {
17b3612022fe53 Josef Bacik 2023-11-22 352 ctx->compress_type = BTRFS_COMPRESS_ZSTD;
17b3612022fe53 Josef Bacik 2023-11-22 353 ctx->compress_level =
17b3612022fe53 Josef Bacik 2023-11-22 354 btrfs_compress_str2level(BTRFS_COMPRESS_ZSTD,
17b3612022fe53 Josef Bacik 2023-11-22 355 param->string + 4);
17b3612022fe53 Josef Bacik 2023-11-22 356 btrfs_set_opt(ctx->mount_opt, COMPRESS);
17b3612022fe53 Josef Bacik 2023-11-22 357 btrfs_clear_opt(ctx->mount_opt, NODATACOW);
17b3612022fe53 Josef Bacik 2023-11-22 358 btrfs_clear_opt(ctx->mount_opt, NODATASUM);
17b3612022fe53 Josef Bacik 2023-11-22 359 } else if (strncmp(param->string, "no", 2) == 0) {
17b3612022fe53 Josef Bacik 2023-11-22 360 ctx->compress_level = 0;
17b3612022fe53 Josef Bacik 2023-11-22 361 ctx->compress_type = 0;
17b3612022fe53 Josef Bacik 2023-11-22 362 btrfs_clear_opt(ctx->mount_opt, COMPRESS);
17b3612022fe53 Josef Bacik 2023-11-22 363 btrfs_clear_opt(ctx->mount_opt, FORCE_COMPRESS);
17b3612022fe53 Josef Bacik 2023-11-22 364 } else {
17b3612022fe53 Josef Bacik 2023-11-22 365 btrfs_err(NULL, "unrecognized compression value %s",
17b3612022fe53 Josef Bacik 2023-11-22 366 param->string);
17b3612022fe53 Josef Bacik 2023-11-22 367 return -EINVAL;
17b3612022fe53 Josef Bacik 2023-11-22 368 }
17b3612022fe53 Josef Bacik 2023-11-22 369 break;
17b3612022fe53 Josef Bacik 2023-11-22 370 case Opt_ssd:
17b3612022fe53 Josef Bacik 2023-11-22 371 if (result.negated) {
17b3612022fe53 Josef Bacik 2023-11-22 372 btrfs_set_opt(ctx->mount_opt, NOSSD);
17b3612022fe53 Josef Bacik 2023-11-22 373 btrfs_clear_opt(ctx->mount_opt, SSD);
17b3612022fe53 Josef Bacik 2023-11-22 374 btrfs_clear_opt(ctx->mount_opt, SSD_SPREAD);
17b3612022fe53 Josef Bacik 2023-11-22 375 } else {
17b3612022fe53 Josef Bacik 2023-11-22 376 btrfs_set_opt(ctx->mount_opt, SSD);
17b3612022fe53 Josef Bacik 2023-11-22 377 btrfs_clear_opt(ctx->mount_opt, NOSSD);
17b3612022fe53 Josef Bacik 2023-11-22 378 }
17b3612022fe53 Josef Bacik 2023-11-22 379 break;
17b3612022fe53 Josef Bacik 2023-11-22 380 case Opt_ssd_spread:
17b3612022fe53 Josef Bacik 2023-11-22 381 if (result.negated) {
17b3612022fe53 Josef Bacik 2023-11-22 382 btrfs_clear_opt(ctx->mount_opt, SSD_SPREAD);
17b3612022fe53 Josef Bacik 2023-11-22 383 } else {
17b3612022fe53 Josef Bacik 2023-11-22 384 btrfs_set_opt(ctx->mount_opt, SSD);
17b3612022fe53 Josef Bacik 2023-11-22 385 btrfs_set_opt(ctx->mount_opt, SSD_SPREAD);
17b3612022fe53 Josef Bacik 2023-11-22 386 btrfs_clear_opt(ctx->mount_opt, NOSSD);
17b3612022fe53 Josef Bacik 2023-11-22 387 }
17b3612022fe53 Josef Bacik 2023-11-22 388 break;
17b3612022fe53 Josef Bacik 2023-11-22 389 case Opt_barrier:
17b3612022fe53 Josef Bacik 2023-11-22 390 if (result.negated)
17b3612022fe53 Josef Bacik 2023-11-22 391 btrfs_set_opt(ctx->mount_opt, NOBARRIER);
17b3612022fe53 Josef Bacik 2023-11-22 392 else
17b3612022fe53 Josef Bacik 2023-11-22 393 btrfs_clear_opt(ctx->mount_opt, NOBARRIER);
17b3612022fe53 Josef Bacik 2023-11-22 394 break;
17b3612022fe53 Josef Bacik 2023-11-22 395 case Opt_thread_pool:
17b3612022fe53 Josef Bacik 2023-11-22 396 if (result.uint_32 == 0) {
17b3612022fe53 Josef Bacik 2023-11-22 397 btrfs_err(NULL, "invalid value 0 for thread_pool");
17b3612022fe53 Josef Bacik 2023-11-22 398 return -EINVAL;
17b3612022fe53 Josef Bacik 2023-11-22 399 }
17b3612022fe53 Josef Bacik 2023-11-22 400 ctx->thread_pool_size = result.uint_32;
17b3612022fe53 Josef Bacik 2023-11-22 401 break;
17b3612022fe53 Josef Bacik 2023-11-22 402 case Opt_max_inline:
eb3fb8b856cc42 Qu Wenruo 2023-12-23 @403 int ret;
eb3fb8b856cc42 Qu Wenruo 2023-12-23 404
eb3fb8b856cc42 Qu Wenruo 2023-12-23 405 ret = memparse_safe(param->string, MEMPARSE_SUFFIXES_DEFAULT,
eb3fb8b856cc42 Qu Wenruo 2023-12-23 406 &ctx->max_inline, NULL);
eb3fb8b856cc42 Qu Wenruo 2023-12-23 407 if (ret < 0) {
eb3fb8b856cc42 Qu Wenruo 2023-12-23 408 btrfs_err(NULL, "invalid string \"%s\"", param->string);
eb3fb8b856cc42 Qu Wenruo 2023-12-23 409 return ret;
eb3fb8b856cc42 Qu Wenruo 2023-12-23 410 }
17b3612022fe53 Josef Bacik 2023-11-22 411 ctx->max_inline = memparse(param->string, NULL);
17b3612022fe53 Josef Bacik 2023-11-22 412 break;
17b3612022fe53 Josef Bacik 2023-11-22 413 case Opt_acl:
17b3612022fe53 Josef Bacik 2023-11-22 414 if (result.negated) {
17b3612022fe53 Josef Bacik 2023-11-22 415 fc->sb_flags &= ~SB_POSIXACL;
17b3612022fe53 Josef Bacik 2023-11-22 416 } else {
17b3612022fe53 Josef Bacik 2023-11-22 417 #ifdef CONFIG_BTRFS_FS_POSIX_ACL
17b3612022fe53 Josef Bacik 2023-11-22 418 fc->sb_flags |= SB_POSIXACL;
17b3612022fe53 Josef Bacik 2023-11-22 419 #else
17b3612022fe53 Josef Bacik 2023-11-22 420 btrfs_err(NULL, "support for ACL not compiled in");
17b3612022fe53 Josef Bacik 2023-11-22 421 return -EINVAL;
17b3612022fe53 Josef Bacik 2023-11-22 422 #endif
17b3612022fe53 Josef Bacik 2023-11-22 423 }
17b3612022fe53 Josef Bacik 2023-11-22 424 /*
17b3612022fe53 Josef Bacik 2023-11-22 425 * VFS limits the ability to toggle ACL on and off via remount,
17b3612022fe53 Josef Bacik 2023-11-22 426 * despite every file system allowing this. This seems to be
17b3612022fe53 Josef Bacik 2023-11-22 427 * an oversight since we all do, but it'll fail if we're
17b3612022fe53 Josef Bacik 2023-11-22 428 * remounting. So don't set the mask here, we'll check it in
17b3612022fe53 Josef Bacik 2023-11-22 429 * btrfs_reconfigure and do the toggling ourselves.
17b3612022fe53 Josef Bacik 2023-11-22 430 */
17b3612022fe53 Josef Bacik 2023-11-22 431 if (fc->purpose != FS_CONTEXT_FOR_RECONFIGURE)
17b3612022fe53 Josef Bacik 2023-11-22 432 fc->sb_flags_mask |= SB_POSIXACL;
17b3612022fe53 Josef Bacik 2023-11-22 433 break;
17b3612022fe53 Josef Bacik 2023-11-22 434 case Opt_treelog:
17b3612022fe53 Josef Bacik 2023-11-22 435 if (result.negated)
17b3612022fe53 Josef Bacik 2023-11-22 436 btrfs_set_opt(ctx->mount_opt, NOTREELOG);
17b3612022fe53 Josef Bacik 2023-11-22 437 else
17b3612022fe53 Josef Bacik 2023-11-22 438 btrfs_clear_opt(ctx->mount_opt, NOTREELOG);
17b3612022fe53 Josef Bacik 2023-11-22 439 break;
17b3612022fe53 Josef Bacik 2023-11-22 440 case Opt_nologreplay:
17b3612022fe53 Josef Bacik 2023-11-22 441 btrfs_warn(NULL,
17b3612022fe53 Josef Bacik 2023-11-22 442 "'nologreplay' is deprecated, use 'rescue=nologreplay' instead");
17b3612022fe53 Josef Bacik 2023-11-22 443 btrfs_set_opt(ctx->mount_opt, NOLOGREPLAY);
17b3612022fe53 Josef Bacik 2023-11-22 444 break;
17b3612022fe53 Josef Bacik 2023-11-22 445 case Opt_flushoncommit:
17b3612022fe53 Josef Bacik 2023-11-22 446 if (result.negated)
17b3612022fe53 Josef Bacik 2023-11-22 447 btrfs_clear_opt(ctx->mount_opt, FLUSHONCOMMIT);
17b3612022fe53 Josef Bacik 2023-11-22 448 else
17b3612022fe53 Josef Bacik 2023-11-22 449 btrfs_set_opt(ctx->mount_opt, FLUSHONCOMMIT);
17b3612022fe53 Josef Bacik 2023-11-22 450 break;
17b3612022fe53 Josef Bacik 2023-11-22 451 case Opt_ratio:
17b3612022fe53 Josef Bacik 2023-11-22 452 ctx->metadata_ratio = result.uint_32;
17b3612022fe53 Josef Bacik 2023-11-22 453 break;
17b3612022fe53 Josef Bacik 2023-11-22 454 case Opt_discard:
17b3612022fe53 Josef Bacik 2023-11-22 455 if (result.negated) {
17b3612022fe53 Josef Bacik 2023-11-22 456 btrfs_clear_opt(ctx->mount_opt, DISCARD_SYNC);
17b3612022fe53 Josef Bacik 2023-11-22 457 btrfs_clear_opt(ctx->mount_opt, DISCARD_ASYNC);
17b3612022fe53 Josef Bacik 2023-11-22 458 btrfs_set_opt(ctx->mount_opt, NODISCARD);
17b3612022fe53 Josef Bacik 2023-11-22 459 } else {
17b3612022fe53 Josef Bacik 2023-11-22 460 btrfs_set_opt(ctx->mount_opt, DISCARD_SYNC);
17b3612022fe53 Josef Bacik 2023-11-22 461 btrfs_clear_opt(ctx->mount_opt, DISCARD_ASYNC);
17b3612022fe53 Josef Bacik 2023-11-22 462 }
17b3612022fe53 Josef Bacik 2023-11-22 463 break;
17b3612022fe53 Josef Bacik 2023-11-22 464 case Opt_discard_mode:
17b3612022fe53 Josef Bacik 2023-11-22 465 switch (result.uint_32) {
17b3612022fe53 Josef Bacik 2023-11-22 466 case Opt_discard_sync:
17b3612022fe53 Josef Bacik 2023-11-22 467 btrfs_clear_opt(ctx->mount_opt, DISCARD_ASYNC);
17b3612022fe53 Josef Bacik 2023-11-22 468 btrfs_set_opt(ctx->mount_opt, DISCARD_SYNC);
17b3612022fe53 Josef Bacik 2023-11-22 469 break;
17b3612022fe53 Josef Bacik 2023-11-22 470 case Opt_discard_async:
17b3612022fe53 Josef Bacik 2023-11-22 471 btrfs_clear_opt(ctx->mount_opt, DISCARD_SYNC);
17b3612022fe53 Josef Bacik 2023-11-22 472 btrfs_set_opt(ctx->mount_opt, DISCARD_ASYNC);
17b3612022fe53 Josef Bacik 2023-11-22 473 break;
17b3612022fe53 Josef Bacik 2023-11-22 474 default:
17b3612022fe53 Josef Bacik 2023-11-22 475 btrfs_err(NULL, "unrecognized discard mode value %s",
17b3612022fe53 Josef Bacik 2023-11-22 476 param->key);
17b3612022fe53 Josef Bacik 2023-11-22 477 return -EINVAL;
17b3612022fe53 Josef Bacik 2023-11-22 478 }
17b3612022fe53 Josef Bacik 2023-11-22 479 btrfs_clear_opt(ctx->mount_opt, NODISCARD);
17b3612022fe53 Josef Bacik 2023-11-22 480 break;
17b3612022fe53 Josef Bacik 2023-11-22 481 case Opt_space_cache:
17b3612022fe53 Josef Bacik 2023-11-22 482 if (result.negated) {
17b3612022fe53 Josef Bacik 2023-11-22 483 btrfs_set_opt(ctx->mount_opt, NOSPACECACHE);
17b3612022fe53 Josef Bacik 2023-11-22 484 btrfs_clear_opt(ctx->mount_opt, SPACE_CACHE);
17b3612022fe53 Josef Bacik 2023-11-22 485 btrfs_clear_opt(ctx->mount_opt, FREE_SPACE_TREE);
17b3612022fe53 Josef Bacik 2023-11-22 486 } else {
17b3612022fe53 Josef Bacik 2023-11-22 487 btrfs_clear_opt(ctx->mount_opt, FREE_SPACE_TREE);
17b3612022fe53 Josef Bacik 2023-11-22 488 btrfs_set_opt(ctx->mount_opt, SPACE_CACHE);
17b3612022fe53 Josef Bacik 2023-11-22 489 }
17b3612022fe53 Josef Bacik 2023-11-22 490 break;
17b3612022fe53 Josef Bacik 2023-11-22 491 case Opt_space_cache_version:
17b3612022fe53 Josef Bacik 2023-11-22 492 switch (result.uint_32) {
17b3612022fe53 Josef Bacik 2023-11-22 493 case Opt_space_cache_v1:
17b3612022fe53 Josef Bacik 2023-11-22 494 btrfs_set_opt(ctx->mount_opt, SPACE_CACHE);
17b3612022fe53 Josef Bacik 2023-11-22 495 btrfs_clear_opt(ctx->mount_opt, FREE_SPACE_TREE);
17b3612022fe53 Josef Bacik 2023-11-22 496 break;
17b3612022fe53 Josef Bacik 2023-11-22 497 case Opt_space_cache_v2:
17b3612022fe53 Josef Bacik 2023-11-22 498 btrfs_clear_opt(ctx->mount_opt, SPACE_CACHE);
17b3612022fe53 Josef Bacik 2023-11-22 499 btrfs_set_opt(ctx->mount_opt, FREE_SPACE_TREE);
17b3612022fe53 Josef Bacik 2023-11-22 500 break;
17b3612022fe53 Josef Bacik 2023-11-22 501 default:
17b3612022fe53 Josef Bacik 2023-11-22 502 btrfs_err(NULL, "unrecognized space_cache value %s",
17b3612022fe53 Josef Bacik 2023-11-22 503 param->key);
17b3612022fe53 Josef Bacik 2023-11-22 504 return -EINVAL;
17b3612022fe53 Josef Bacik 2023-11-22 505 }
17b3612022fe53 Josef Bacik 2023-11-22 506 break;
17b3612022fe53 Josef Bacik 2023-11-22 507 case Opt_rescan_uuid_tree:
17b3612022fe53 Josef Bacik 2023-11-22 508 btrfs_set_opt(ctx->mount_opt, RESCAN_UUID_TREE);
17b3612022fe53 Josef Bacik 2023-11-22 509 break;
17b3612022fe53 Josef Bacik 2023-11-22 510 case Opt_clear_cache:
17b3612022fe53 Josef Bacik 2023-11-22 511 btrfs_set_opt(ctx->mount_opt, CLEAR_CACHE);
17b3612022fe53 Josef Bacik 2023-11-22 512 break;
17b3612022fe53 Josef Bacik 2023-11-22 513 case Opt_user_subvol_rm_allowed:
17b3612022fe53 Josef Bacik 2023-11-22 514 btrfs_set_opt(ctx->mount_opt, USER_SUBVOL_RM_ALLOWED);
17b3612022fe53 Josef Bacik 2023-11-22 515 break;
17b3612022fe53 Josef Bacik 2023-11-22 516 case Opt_enospc_debug:
17b3612022fe53 Josef Bacik 2023-11-22 517 if (result.negated)
17b3612022fe53 Josef Bacik 2023-11-22 518 btrfs_clear_opt(ctx->mount_opt, ENOSPC_DEBUG);
17b3612022fe53 Josef Bacik 2023-11-22 519 else
17b3612022fe53 Josef Bacik 2023-11-22 520 btrfs_set_opt(ctx->mount_opt, ENOSPC_DEBUG);
17b3612022fe53 Josef Bacik 2023-11-22 521 break;
17b3612022fe53 Josef Bacik 2023-11-22 522 case Opt_defrag:
17b3612022fe53 Josef Bacik 2023-11-22 523 if (result.negated)
17b3612022fe53 Josef Bacik 2023-11-22 524 btrfs_clear_opt(ctx->mount_opt, AUTO_DEFRAG);
17b3612022fe53 Josef Bacik 2023-11-22 525 else
17b3612022fe53 Josef Bacik 2023-11-22 526 btrfs_set_opt(ctx->mount_opt, AUTO_DEFRAG);
17b3612022fe53 Josef Bacik 2023-11-22 527 break;
17b3612022fe53 Josef Bacik 2023-11-22 528 case Opt_usebackuproot:
17b3612022fe53 Josef Bacik 2023-11-22 529 btrfs_warn(NULL,
17b3612022fe53 Josef Bacik 2023-11-22 530 "'usebackuproot' is deprecated, use 'rescue=usebackuproot' instead");
17b3612022fe53 Josef Bacik 2023-11-22 531 btrfs_set_opt(ctx->mount_opt, USEBACKUPROOT);
9fb3b1a7fed796 Josef Bacik 2023-11-22 532
9fb3b1a7fed796 Josef Bacik 2023-11-22 533 /* If we're loading the backup roots we can't trust the space cache. */
9fb3b1a7fed796 Josef Bacik 2023-11-22 534 btrfs_set_opt(ctx->mount_opt, CLEAR_CACHE);
17b3612022fe53 Josef Bacik 2023-11-22 535 break;
17b3612022fe53 Josef Bacik 2023-11-22 536 case Opt_skip_balance:
17b3612022fe53 Josef Bacik 2023-11-22 537 btrfs_set_opt(ctx->mount_opt, SKIP_BALANCE);
17b3612022fe53 Josef Bacik 2023-11-22 538 break;
17b3612022fe53 Josef Bacik 2023-11-22 539 case Opt_fatal_errors:
17b3612022fe53 Josef Bacik 2023-11-22 540 switch (result.uint_32) {
17b3612022fe53 Josef Bacik 2023-11-22 541 case Opt_fatal_errors_panic:
17b3612022fe53 Josef Bacik 2023-11-22 542 btrfs_set_opt(ctx->mount_opt, PANIC_ON_FATAL_ERROR);
17b3612022fe53 Josef Bacik 2023-11-22 543 break;
17b3612022fe53 Josef Bacik 2023-11-22 544 case Opt_fatal_errors_bug:
17b3612022fe53 Josef Bacik 2023-11-22 545 btrfs_clear_opt(ctx->mount_opt, PANIC_ON_FATAL_ERROR);
17b3612022fe53 Josef Bacik 2023-11-22 546 break;
17b3612022fe53 Josef Bacik 2023-11-22 547 default:
17b3612022fe53 Josef Bacik 2023-11-22 548 btrfs_err(NULL, "unrecognized fatal_errors value %s",
17b3612022fe53 Josef Bacik 2023-11-22 549 param->key);
17b3612022fe53 Josef Bacik 2023-11-22 550 return -EINVAL;
17b3612022fe53 Josef Bacik 2023-11-22 551 }
17b3612022fe53 Josef Bacik 2023-11-22 552 break;
17b3612022fe53 Josef Bacik 2023-11-22 553 case Opt_commit_interval:
17b3612022fe53 Josef Bacik 2023-11-22 554 ctx->commit_interval = result.uint_32;
17b3612022fe53 Josef Bacik 2023-11-22 555 if (ctx->commit_interval == 0)
17b3612022fe53 Josef Bacik 2023-11-22 556 ctx->commit_interval = BTRFS_DEFAULT_COMMIT_INTERVAL;
17b3612022fe53 Josef Bacik 2023-11-22 557 break;
17b3612022fe53 Josef Bacik 2023-11-22 558 case Opt_rescue:
17b3612022fe53 Josef Bacik 2023-11-22 559 switch (result.uint_32) {
17b3612022fe53 Josef Bacik 2023-11-22 560 case Opt_rescue_usebackuproot:
17b3612022fe53 Josef Bacik 2023-11-22 561 btrfs_set_opt(ctx->mount_opt, USEBACKUPROOT);
17b3612022fe53 Josef Bacik 2023-11-22 562 break;
17b3612022fe53 Josef Bacik 2023-11-22 563 case Opt_rescue_nologreplay:
17b3612022fe53 Josef Bacik 2023-11-22 564 btrfs_set_opt(ctx->mount_opt, NOLOGREPLAY);
17b3612022fe53 Josef Bacik 2023-11-22 565 break;
17b3612022fe53 Josef Bacik 2023-11-22 566 case Opt_rescue_ignorebadroots:
17b3612022fe53 Josef Bacik 2023-11-22 567 btrfs_set_opt(ctx->mount_opt, IGNOREBADROOTS);
17b3612022fe53 Josef Bacik 2023-11-22 568 break;
17b3612022fe53 Josef Bacik 2023-11-22 569 case Opt_rescue_ignoredatacsums:
17b3612022fe53 Josef Bacik 2023-11-22 570 btrfs_set_opt(ctx->mount_opt, IGNOREDATACSUMS);
17b3612022fe53 Josef Bacik 2023-11-22 571 break;
17b3612022fe53 Josef Bacik 2023-11-22 572 case Opt_rescue_parameter_all:
17b3612022fe53 Josef Bacik 2023-11-22 573 btrfs_set_opt(ctx->mount_opt, IGNOREDATACSUMS);
17b3612022fe53 Josef Bacik 2023-11-22 574 btrfs_set_opt(ctx->mount_opt, IGNOREBADROOTS);
17b3612022fe53 Josef Bacik 2023-11-22 575 btrfs_set_opt(ctx->mount_opt, NOLOGREPLAY);
17b3612022fe53 Josef Bacik 2023-11-22 576 break;
17b3612022fe53 Josef Bacik 2023-11-22 577 default:
17b3612022fe53 Josef Bacik 2023-11-22 578 btrfs_info(NULL, "unrecognized rescue option '%s'",
17b3612022fe53 Josef Bacik 2023-11-22 579 param->key);
17b3612022fe53 Josef Bacik 2023-11-22 580 return -EINVAL;
17b3612022fe53 Josef Bacik 2023-11-22 581 }
17b3612022fe53 Josef Bacik 2023-11-22 582 break;
17b3612022fe53 Josef Bacik 2023-11-22 583 #ifdef CONFIG_BTRFS_DEBUG
17b3612022fe53 Josef Bacik 2023-11-22 584 case Opt_fragment:
17b3612022fe53 Josef Bacik 2023-11-22 585 switch (result.uint_32) {
17b3612022fe53 Josef Bacik 2023-11-22 586 case Opt_fragment_parameter_all:
17b3612022fe53 Josef Bacik 2023-11-22 587 btrfs_set_opt(ctx->mount_opt, FRAGMENT_DATA);
17b3612022fe53 Josef Bacik 2023-11-22 588 btrfs_set_opt(ctx->mount_opt, FRAGMENT_METADATA);
17b3612022fe53 Josef Bacik 2023-11-22 589 break;
17b3612022fe53 Josef Bacik 2023-11-22 590 case Opt_fragment_parameter_metadata:
17b3612022fe53 Josef Bacik 2023-11-22 591 btrfs_set_opt(ctx->mount_opt, FRAGMENT_METADATA);
17b3612022fe53 Josef Bacik 2023-11-22 592 break;
17b3612022fe53 Josef Bacik 2023-11-22 593 case Opt_fragment_parameter_data:
17b3612022fe53 Josef Bacik 2023-11-22 594 btrfs_set_opt(ctx->mount_opt, FRAGMENT_DATA);
17b3612022fe53 Josef Bacik 2023-11-22 595 break;
17b3612022fe53 Josef Bacik 2023-11-22 596 default:
17b3612022fe53 Josef Bacik 2023-11-22 597 btrfs_info(NULL, "unrecognized fragment option '%s'",
17b3612022fe53 Josef Bacik 2023-11-22 598 param->key);
17b3612022fe53 Josef Bacik 2023-11-22 599 return -EINVAL;
17b3612022fe53 Josef Bacik 2023-11-22 600 }
17b3612022fe53 Josef Bacik 2023-11-22 601 break;
17b3612022fe53 Josef Bacik 2023-11-22 602 #endif
17b3612022fe53 Josef Bacik 2023-11-22 603 #ifdef CONFIG_BTRFS_FS_REF_VERIFY
17b3612022fe53 Josef Bacik 2023-11-22 604 case Opt_ref_verify:
17b3612022fe53 Josef Bacik 2023-11-22 605 btrfs_set_opt(ctx->mount_opt, REF_VERIFY);
17b3612022fe53 Josef Bacik 2023-11-22 606 break;
17b3612022fe53 Josef Bacik 2023-11-22 607 #endif
17b3612022fe53 Josef Bacik 2023-11-22 608 default:
17b3612022fe53 Josef Bacik 2023-11-22 609 btrfs_err(NULL, "unrecognized mount option '%s'", param->key);
17b3612022fe53 Josef Bacik 2023-11-22 610 return -EINVAL;
17b3612022fe53 Josef Bacik 2023-11-22 611 }
17b3612022fe53 Josef Bacik 2023-11-22 612
17b3612022fe53 Josef Bacik 2023-11-22 613 return 0;
17b3612022fe53 Josef Bacik 2023-11-22 614 }
17b3612022fe53 Josef Bacik 2023-11-22 615
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 3/3] btrfs: migrate to the newer memparse_safe() helper
2023-12-23 9:58 ` [PATCH 3/3] btrfs: migrate to the newer memparse_safe() helper Qu Wenruo
@ 2023-12-26 4:53 ` kernel test robot
2023-12-26 6:06 ` kernel test robot
2023-12-27 6:27 ` David Disseldorp
2 siblings, 0 replies; 16+ messages in thread
From: kernel test robot @ 2023-12-26 4:53 UTC (permalink / raw
To: Qu Wenruo, linux-btrfs, linux-kernel, akpm, christophe.jaillet,
andriy.shevchenko, David.Laight, ddiss
Cc: llvm, oe-kbuild-all
Hi Qu,
kernel test robot noticed the following build warnings:
[auto build test WARNING on kdave/for-next]
[also build test WARNING on next-20231222]
[cannot apply to akpm-mm/mm-nonmm-unstable akpm-mm/mm-everything linus/master v6.7-rc7]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Qu-Wenruo/kstrtox-introduce-a-safer-version-of-memparse/20231225-151921
base: https://git.kernel.org/pub/scm/linux/kernel/git/kdave/linux.git for-next
patch link: https://lore.kernel.org/r/6dfa53ded887caa2269c1beeaedcff086342339a.1703324146.git.wqu%40suse.com
patch subject: [PATCH 3/3] btrfs: migrate to the newer memparse_safe() helper
config: powerpc-allmodconfig (https://download.01.org/0day-ci/archive/20231226/202312261215.eFf9J1xV-lkp@intel.com/config)
compiler: clang version 18.0.0git (https://github.com/llvm/llvm-project d3ef86708241a3bee902615c190dead1638c4e09)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231226/202312261215.eFf9J1xV-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202312261215.eFf9J1xV-lkp@intel.com/
All warnings (new ones prefixed by >>):
>> fs/btrfs/super.c:403:3: warning: label followed by a declaration is a C23 extension [-Wc23-extensions]
403 | int ret;
| ^
1 warning generated.
vim +403 fs/btrfs/super.c
261
262 static int btrfs_parse_param(struct fs_context *fc, struct fs_parameter *param)
263 {
264 struct btrfs_fs_context *ctx = fc->fs_private;
265 struct fs_parse_result result;
266 int opt;
267
268 opt = fs_parse(fc, btrfs_fs_parameters, param, &result);
269 if (opt < 0)
270 return opt;
271
272 switch (opt) {
273 case Opt_degraded:
274 btrfs_set_opt(ctx->mount_opt, DEGRADED);
275 break;
276 case Opt_subvol_empty:
277 /*
278 * This exists because we used to allow it on accident, so we're
279 * keeping it to maintain ABI. See 37becec95ac3 ("Btrfs: allow
280 * empty subvol= again").
281 */
282 break;
283 case Opt_subvol:
284 kfree(ctx->subvol_name);
285 ctx->subvol_name = kstrdup(param->string, GFP_KERNEL);
286 if (!ctx->subvol_name)
287 return -ENOMEM;
288 break;
289 case Opt_subvolid:
290 ctx->subvol_objectid = result.uint_64;
291
292 /* subvolid=0 means give me the original fs_tree. */
293 if (!ctx->subvol_objectid)
294 ctx->subvol_objectid = BTRFS_FS_TREE_OBJECTID;
295 break;
296 case Opt_device: {
297 struct btrfs_device *device;
298 blk_mode_t mode = sb_open_mode(fc->sb_flags);
299
300 mutex_lock(&uuid_mutex);
301 device = btrfs_scan_one_device(param->string, mode, false);
302 mutex_unlock(&uuid_mutex);
303 if (IS_ERR(device))
304 return PTR_ERR(device);
305 break;
306 }
307 case Opt_datasum:
308 if (result.negated) {
309 btrfs_set_opt(ctx->mount_opt, NODATASUM);
310 } else {
311 btrfs_clear_opt(ctx->mount_opt, NODATACOW);
312 btrfs_clear_opt(ctx->mount_opt, NODATASUM);
313 }
314 break;
315 case Opt_datacow:
316 if (result.negated) {
317 btrfs_clear_opt(ctx->mount_opt, COMPRESS);
318 btrfs_clear_opt(ctx->mount_opt, FORCE_COMPRESS);
319 btrfs_set_opt(ctx->mount_opt, NODATACOW);
320 btrfs_set_opt(ctx->mount_opt, NODATASUM);
321 } else {
322 btrfs_clear_opt(ctx->mount_opt, NODATACOW);
323 }
324 break;
325 case Opt_compress_force:
326 case Opt_compress_force_type:
327 btrfs_set_opt(ctx->mount_opt, FORCE_COMPRESS);
328 fallthrough;
329 case Opt_compress:
330 case Opt_compress_type:
331 if (opt == Opt_compress || opt == Opt_compress_force) {
332 ctx->compress_type = BTRFS_COMPRESS_ZLIB;
333 ctx->compress_level = BTRFS_ZLIB_DEFAULT_LEVEL;
334 btrfs_set_opt(ctx->mount_opt, COMPRESS);
335 btrfs_clear_opt(ctx->mount_opt, NODATACOW);
336 btrfs_clear_opt(ctx->mount_opt, NODATASUM);
337 } else if (strncmp(param->string, "zlib", 4) == 0) {
338 ctx->compress_type = BTRFS_COMPRESS_ZLIB;
339 ctx->compress_level =
340 btrfs_compress_str2level(BTRFS_COMPRESS_ZLIB,
341 param->string + 4);
342 btrfs_set_opt(ctx->mount_opt, COMPRESS);
343 btrfs_clear_opt(ctx->mount_opt, NODATACOW);
344 btrfs_clear_opt(ctx->mount_opt, NODATASUM);
345 } else if (strncmp(param->string, "lzo", 3) == 0) {
346 ctx->compress_type = BTRFS_COMPRESS_LZO;
347 ctx->compress_level = 0;
348 btrfs_set_opt(ctx->mount_opt, COMPRESS);
349 btrfs_clear_opt(ctx->mount_opt, NODATACOW);
350 btrfs_clear_opt(ctx->mount_opt, NODATASUM);
351 } else if (strncmp(param->string, "zstd", 4) == 0) {
352 ctx->compress_type = BTRFS_COMPRESS_ZSTD;
353 ctx->compress_level =
354 btrfs_compress_str2level(BTRFS_COMPRESS_ZSTD,
355 param->string + 4);
356 btrfs_set_opt(ctx->mount_opt, COMPRESS);
357 btrfs_clear_opt(ctx->mount_opt, NODATACOW);
358 btrfs_clear_opt(ctx->mount_opt, NODATASUM);
359 } else if (strncmp(param->string, "no", 2) == 0) {
360 ctx->compress_level = 0;
361 ctx->compress_type = 0;
362 btrfs_clear_opt(ctx->mount_opt, COMPRESS);
363 btrfs_clear_opt(ctx->mount_opt, FORCE_COMPRESS);
364 } else {
365 btrfs_err(NULL, "unrecognized compression value %s",
366 param->string);
367 return -EINVAL;
368 }
369 break;
370 case Opt_ssd:
371 if (result.negated) {
372 btrfs_set_opt(ctx->mount_opt, NOSSD);
373 btrfs_clear_opt(ctx->mount_opt, SSD);
374 btrfs_clear_opt(ctx->mount_opt, SSD_SPREAD);
375 } else {
376 btrfs_set_opt(ctx->mount_opt, SSD);
377 btrfs_clear_opt(ctx->mount_opt, NOSSD);
378 }
379 break;
380 case Opt_ssd_spread:
381 if (result.negated) {
382 btrfs_clear_opt(ctx->mount_opt, SSD_SPREAD);
383 } else {
384 btrfs_set_opt(ctx->mount_opt, SSD);
385 btrfs_set_opt(ctx->mount_opt, SSD_SPREAD);
386 btrfs_clear_opt(ctx->mount_opt, NOSSD);
387 }
388 break;
389 case Opt_barrier:
390 if (result.negated)
391 btrfs_set_opt(ctx->mount_opt, NOBARRIER);
392 else
393 btrfs_clear_opt(ctx->mount_opt, NOBARRIER);
394 break;
395 case Opt_thread_pool:
396 if (result.uint_32 == 0) {
397 btrfs_err(NULL, "invalid value 0 for thread_pool");
398 return -EINVAL;
399 }
400 ctx->thread_pool_size = result.uint_32;
401 break;
402 case Opt_max_inline:
> 403 int ret;
404
405 ret = memparse_safe(param->string, MEMPARSE_SUFFIXES_DEFAULT,
406 &ctx->max_inline, NULL);
407 if (ret < 0) {
408 btrfs_err(NULL, "invalid string \"%s\"", param->string);
409 return ret;
410 }
411 ctx->max_inline = memparse(param->string, NULL);
412 break;
413 case Opt_acl:
414 if (result.negated) {
415 fc->sb_flags &= ~SB_POSIXACL;
416 } else {
417 #ifdef CONFIG_BTRFS_FS_POSIX_ACL
418 fc->sb_flags |= SB_POSIXACL;
419 #else
420 btrfs_err(NULL, "support for ACL not compiled in");
421 return -EINVAL;
422 #endif
423 }
424 /*
425 * VFS limits the ability to toggle ACL on and off via remount,
426 * despite every file system allowing this. This seems to be
427 * an oversight since we all do, but it'll fail if we're
428 * remounting. So don't set the mask here, we'll check it in
429 * btrfs_reconfigure and do the toggling ourselves.
430 */
431 if (fc->purpose != FS_CONTEXT_FOR_RECONFIGURE)
432 fc->sb_flags_mask |= SB_POSIXACL;
433 break;
434 case Opt_treelog:
435 if (result.negated)
436 btrfs_set_opt(ctx->mount_opt, NOTREELOG);
437 else
438 btrfs_clear_opt(ctx->mount_opt, NOTREELOG);
439 break;
440 case Opt_nologreplay:
441 btrfs_warn(NULL,
442 "'nologreplay' is deprecated, use 'rescue=nologreplay' instead");
443 btrfs_set_opt(ctx->mount_opt, NOLOGREPLAY);
444 break;
445 case Opt_flushoncommit:
446 if (result.negated)
447 btrfs_clear_opt(ctx->mount_opt, FLUSHONCOMMIT);
448 else
449 btrfs_set_opt(ctx->mount_opt, FLUSHONCOMMIT);
450 break;
451 case Opt_ratio:
452 ctx->metadata_ratio = result.uint_32;
453 break;
454 case Opt_discard:
455 if (result.negated) {
456 btrfs_clear_opt(ctx->mount_opt, DISCARD_SYNC);
457 btrfs_clear_opt(ctx->mount_opt, DISCARD_ASYNC);
458 btrfs_set_opt(ctx->mount_opt, NODISCARD);
459 } else {
460 btrfs_set_opt(ctx->mount_opt, DISCARD_SYNC);
461 btrfs_clear_opt(ctx->mount_opt, DISCARD_ASYNC);
462 }
463 break;
464 case Opt_discard_mode:
465 switch (result.uint_32) {
466 case Opt_discard_sync:
467 btrfs_clear_opt(ctx->mount_opt, DISCARD_ASYNC);
468 btrfs_set_opt(ctx->mount_opt, DISCARD_SYNC);
469 break;
470 case Opt_discard_async:
471 btrfs_clear_opt(ctx->mount_opt, DISCARD_SYNC);
472 btrfs_set_opt(ctx->mount_opt, DISCARD_ASYNC);
473 break;
474 default:
475 btrfs_err(NULL, "unrecognized discard mode value %s",
476 param->key);
477 return -EINVAL;
478 }
479 btrfs_clear_opt(ctx->mount_opt, NODISCARD);
480 break;
481 case Opt_space_cache:
482 if (result.negated) {
483 btrfs_set_opt(ctx->mount_opt, NOSPACECACHE);
484 btrfs_clear_opt(ctx->mount_opt, SPACE_CACHE);
485 btrfs_clear_opt(ctx->mount_opt, FREE_SPACE_TREE);
486 } else {
487 btrfs_clear_opt(ctx->mount_opt, FREE_SPACE_TREE);
488 btrfs_set_opt(ctx->mount_opt, SPACE_CACHE);
489 }
490 break;
491 case Opt_space_cache_version:
492 switch (result.uint_32) {
493 case Opt_space_cache_v1:
494 btrfs_set_opt(ctx->mount_opt, SPACE_CACHE);
495 btrfs_clear_opt(ctx->mount_opt, FREE_SPACE_TREE);
496 break;
497 case Opt_space_cache_v2:
498 btrfs_clear_opt(ctx->mount_opt, SPACE_CACHE);
499 btrfs_set_opt(ctx->mount_opt, FREE_SPACE_TREE);
500 break;
501 default:
502 btrfs_err(NULL, "unrecognized space_cache value %s",
503 param->key);
504 return -EINVAL;
505 }
506 break;
507 case Opt_rescan_uuid_tree:
508 btrfs_set_opt(ctx->mount_opt, RESCAN_UUID_TREE);
509 break;
510 case Opt_clear_cache:
511 btrfs_set_opt(ctx->mount_opt, CLEAR_CACHE);
512 break;
513 case Opt_user_subvol_rm_allowed:
514 btrfs_set_opt(ctx->mount_opt, USER_SUBVOL_RM_ALLOWED);
515 break;
516 case Opt_enospc_debug:
517 if (result.negated)
518 btrfs_clear_opt(ctx->mount_opt, ENOSPC_DEBUG);
519 else
520 btrfs_set_opt(ctx->mount_opt, ENOSPC_DEBUG);
521 break;
522 case Opt_defrag:
523 if (result.negated)
524 btrfs_clear_opt(ctx->mount_opt, AUTO_DEFRAG);
525 else
526 btrfs_set_opt(ctx->mount_opt, AUTO_DEFRAG);
527 break;
528 case Opt_usebackuproot:
529 btrfs_warn(NULL,
530 "'usebackuproot' is deprecated, use 'rescue=usebackuproot' instead");
531 btrfs_set_opt(ctx->mount_opt, USEBACKUPROOT);
532
533 /* If we're loading the backup roots we can't trust the space cache. */
534 btrfs_set_opt(ctx->mount_opt, CLEAR_CACHE);
535 break;
536 case Opt_skip_balance:
537 btrfs_set_opt(ctx->mount_opt, SKIP_BALANCE);
538 break;
539 case Opt_fatal_errors:
540 switch (result.uint_32) {
541 case Opt_fatal_errors_panic:
542 btrfs_set_opt(ctx->mount_opt, PANIC_ON_FATAL_ERROR);
543 break;
544 case Opt_fatal_errors_bug:
545 btrfs_clear_opt(ctx->mount_opt, PANIC_ON_FATAL_ERROR);
546 break;
547 default:
548 btrfs_err(NULL, "unrecognized fatal_errors value %s",
549 param->key);
550 return -EINVAL;
551 }
552 break;
553 case Opt_commit_interval:
554 ctx->commit_interval = result.uint_32;
555 if (ctx->commit_interval == 0)
556 ctx->commit_interval = BTRFS_DEFAULT_COMMIT_INTERVAL;
557 break;
558 case Opt_rescue:
559 switch (result.uint_32) {
560 case Opt_rescue_usebackuproot:
561 btrfs_set_opt(ctx->mount_opt, USEBACKUPROOT);
562 break;
563 case Opt_rescue_nologreplay:
564 btrfs_set_opt(ctx->mount_opt, NOLOGREPLAY);
565 break;
566 case Opt_rescue_ignorebadroots:
567 btrfs_set_opt(ctx->mount_opt, IGNOREBADROOTS);
568 break;
569 case Opt_rescue_ignoredatacsums:
570 btrfs_set_opt(ctx->mount_opt, IGNOREDATACSUMS);
571 break;
572 case Opt_rescue_parameter_all:
573 btrfs_set_opt(ctx->mount_opt, IGNOREDATACSUMS);
574 btrfs_set_opt(ctx->mount_opt, IGNOREBADROOTS);
575 btrfs_set_opt(ctx->mount_opt, NOLOGREPLAY);
576 break;
577 default:
578 btrfs_info(NULL, "unrecognized rescue option '%s'",
579 param->key);
580 return -EINVAL;
581 }
582 break;
583 #ifdef CONFIG_BTRFS_DEBUG
584 case Opt_fragment:
585 switch (result.uint_32) {
586 case Opt_fragment_parameter_all:
587 btrfs_set_opt(ctx->mount_opt, FRAGMENT_DATA);
588 btrfs_set_opt(ctx->mount_opt, FRAGMENT_METADATA);
589 break;
590 case Opt_fragment_parameter_metadata:
591 btrfs_set_opt(ctx->mount_opt, FRAGMENT_METADATA);
592 break;
593 case Opt_fragment_parameter_data:
594 btrfs_set_opt(ctx->mount_opt, FRAGMENT_DATA);
595 break;
596 default:
597 btrfs_info(NULL, "unrecognized fragment option '%s'",
598 param->key);
599 return -EINVAL;
600 }
601 break;
602 #endif
603 #ifdef CONFIG_BTRFS_FS_REF_VERIFY
604 case Opt_ref_verify:
605 btrfs_set_opt(ctx->mount_opt, REF_VERIFY);
606 break;
607 #endif
608 default:
609 btrfs_err(NULL, "unrecognized mount option '%s'", param->key);
610 return -EINVAL;
611 }
612
613 return 0;
614 }
615
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 3/3] btrfs: migrate to the newer memparse_safe() helper
2023-12-23 9:58 ` [PATCH 3/3] btrfs: migrate to the newer memparse_safe() helper Qu Wenruo
2023-12-26 4:53 ` kernel test robot
@ 2023-12-26 6:06 ` kernel test robot
2023-12-27 6:27 ` David Disseldorp
2 siblings, 0 replies; 16+ messages in thread
From: kernel test robot @ 2023-12-26 6:06 UTC (permalink / raw
To: Qu Wenruo, linux-btrfs, linux-kernel, akpm, christophe.jaillet,
andriy.shevchenko, David.Laight, ddiss
Cc: llvm, oe-kbuild-all
Hi Qu,
kernel test robot noticed the following build errors:
[auto build test ERROR on kdave/for-next]
[also build test ERROR on next-20231222]
[cannot apply to akpm-mm/mm-nonmm-unstable akpm-mm/mm-everything linus/master v6.7-rc7]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Qu-Wenruo/kstrtox-introduce-a-safer-version-of-memparse/20231225-151921
base: https://git.kernel.org/pub/scm/linux/kernel/git/kdave/linux.git for-next
patch link: https://lore.kernel.org/r/6dfa53ded887caa2269c1beeaedcff086342339a.1703324146.git.wqu%40suse.com
patch subject: [PATCH 3/3] btrfs: migrate to the newer memparse_safe() helper
config: x86_64-rhel-8.3-rust (https://download.01.org/0day-ci/archive/20231226/202312261319.pYgA0ivL-lkp@intel.com/config)
compiler: clang version 16.0.4 (https://github.com/llvm/llvm-project.git ae42196bc493ffe877a7e3dff8be32035dea4d07)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231226/202312261319.pYgA0ivL-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202312261319.pYgA0ivL-lkp@intel.com/
All errors (new ones prefixed by >>):
>> fs/btrfs/super.c:403:3: error: expected expression
int ret;
^
>> fs/btrfs/super.c:405:3: error: use of undeclared identifier 'ret'
ret = memparse_safe(param->string, MEMPARSE_SUFFIXES_DEFAULT,
^
fs/btrfs/super.c:407:7: error: use of undeclared identifier 'ret'
if (ret < 0) {
^
fs/btrfs/super.c:409:11: error: use of undeclared identifier 'ret'
return ret;
^
4 errors generated.
vim +403 fs/btrfs/super.c
261
262 static int btrfs_parse_param(struct fs_context *fc, struct fs_parameter *param)
263 {
264 struct btrfs_fs_context *ctx = fc->fs_private;
265 struct fs_parse_result result;
266 int opt;
267
268 opt = fs_parse(fc, btrfs_fs_parameters, param, &result);
269 if (opt < 0)
270 return opt;
271
272 switch (opt) {
273 case Opt_degraded:
274 btrfs_set_opt(ctx->mount_opt, DEGRADED);
275 break;
276 case Opt_subvol_empty:
277 /*
278 * This exists because we used to allow it on accident, so we're
279 * keeping it to maintain ABI. See 37becec95ac3 ("Btrfs: allow
280 * empty subvol= again").
281 */
282 break;
283 case Opt_subvol:
284 kfree(ctx->subvol_name);
285 ctx->subvol_name = kstrdup(param->string, GFP_KERNEL);
286 if (!ctx->subvol_name)
287 return -ENOMEM;
288 break;
289 case Opt_subvolid:
290 ctx->subvol_objectid = result.uint_64;
291
292 /* subvolid=0 means give me the original fs_tree. */
293 if (!ctx->subvol_objectid)
294 ctx->subvol_objectid = BTRFS_FS_TREE_OBJECTID;
295 break;
296 case Opt_device: {
297 struct btrfs_device *device;
298 blk_mode_t mode = sb_open_mode(fc->sb_flags);
299
300 mutex_lock(&uuid_mutex);
301 device = btrfs_scan_one_device(param->string, mode, false);
302 mutex_unlock(&uuid_mutex);
303 if (IS_ERR(device))
304 return PTR_ERR(device);
305 break;
306 }
307 case Opt_datasum:
308 if (result.negated) {
309 btrfs_set_opt(ctx->mount_opt, NODATASUM);
310 } else {
311 btrfs_clear_opt(ctx->mount_opt, NODATACOW);
312 btrfs_clear_opt(ctx->mount_opt, NODATASUM);
313 }
314 break;
315 case Opt_datacow:
316 if (result.negated) {
317 btrfs_clear_opt(ctx->mount_opt, COMPRESS);
318 btrfs_clear_opt(ctx->mount_opt, FORCE_COMPRESS);
319 btrfs_set_opt(ctx->mount_opt, NODATACOW);
320 btrfs_set_opt(ctx->mount_opt, NODATASUM);
321 } else {
322 btrfs_clear_opt(ctx->mount_opt, NODATACOW);
323 }
324 break;
325 case Opt_compress_force:
326 case Opt_compress_force_type:
327 btrfs_set_opt(ctx->mount_opt, FORCE_COMPRESS);
328 fallthrough;
329 case Opt_compress:
330 case Opt_compress_type:
331 if (opt == Opt_compress || opt == Opt_compress_force) {
332 ctx->compress_type = BTRFS_COMPRESS_ZLIB;
333 ctx->compress_level = BTRFS_ZLIB_DEFAULT_LEVEL;
334 btrfs_set_opt(ctx->mount_opt, COMPRESS);
335 btrfs_clear_opt(ctx->mount_opt, NODATACOW);
336 btrfs_clear_opt(ctx->mount_opt, NODATASUM);
337 } else if (strncmp(param->string, "zlib", 4) == 0) {
338 ctx->compress_type = BTRFS_COMPRESS_ZLIB;
339 ctx->compress_level =
340 btrfs_compress_str2level(BTRFS_COMPRESS_ZLIB,
341 param->string + 4);
342 btrfs_set_opt(ctx->mount_opt, COMPRESS);
343 btrfs_clear_opt(ctx->mount_opt, NODATACOW);
344 btrfs_clear_opt(ctx->mount_opt, NODATASUM);
345 } else if (strncmp(param->string, "lzo", 3) == 0) {
346 ctx->compress_type = BTRFS_COMPRESS_LZO;
347 ctx->compress_level = 0;
348 btrfs_set_opt(ctx->mount_opt, COMPRESS);
349 btrfs_clear_opt(ctx->mount_opt, NODATACOW);
350 btrfs_clear_opt(ctx->mount_opt, NODATASUM);
351 } else if (strncmp(param->string, "zstd", 4) == 0) {
352 ctx->compress_type = BTRFS_COMPRESS_ZSTD;
353 ctx->compress_level =
354 btrfs_compress_str2level(BTRFS_COMPRESS_ZSTD,
355 param->string + 4);
356 btrfs_set_opt(ctx->mount_opt, COMPRESS);
357 btrfs_clear_opt(ctx->mount_opt, NODATACOW);
358 btrfs_clear_opt(ctx->mount_opt, NODATASUM);
359 } else if (strncmp(param->string, "no", 2) == 0) {
360 ctx->compress_level = 0;
361 ctx->compress_type = 0;
362 btrfs_clear_opt(ctx->mount_opt, COMPRESS);
363 btrfs_clear_opt(ctx->mount_opt, FORCE_COMPRESS);
364 } else {
365 btrfs_err(NULL, "unrecognized compression value %s",
366 param->string);
367 return -EINVAL;
368 }
369 break;
370 case Opt_ssd:
371 if (result.negated) {
372 btrfs_set_opt(ctx->mount_opt, NOSSD);
373 btrfs_clear_opt(ctx->mount_opt, SSD);
374 btrfs_clear_opt(ctx->mount_opt, SSD_SPREAD);
375 } else {
376 btrfs_set_opt(ctx->mount_opt, SSD);
377 btrfs_clear_opt(ctx->mount_opt, NOSSD);
378 }
379 break;
380 case Opt_ssd_spread:
381 if (result.negated) {
382 btrfs_clear_opt(ctx->mount_opt, SSD_SPREAD);
383 } else {
384 btrfs_set_opt(ctx->mount_opt, SSD);
385 btrfs_set_opt(ctx->mount_opt, SSD_SPREAD);
386 btrfs_clear_opt(ctx->mount_opt, NOSSD);
387 }
388 break;
389 case Opt_barrier:
390 if (result.negated)
391 btrfs_set_opt(ctx->mount_opt, NOBARRIER);
392 else
393 btrfs_clear_opt(ctx->mount_opt, NOBARRIER);
394 break;
395 case Opt_thread_pool:
396 if (result.uint_32 == 0) {
397 btrfs_err(NULL, "invalid value 0 for thread_pool");
398 return -EINVAL;
399 }
400 ctx->thread_pool_size = result.uint_32;
401 break;
402 case Opt_max_inline:
> 403 int ret;
404
> 405 ret = memparse_safe(param->string, MEMPARSE_SUFFIXES_DEFAULT,
406 &ctx->max_inline, NULL);
407 if (ret < 0) {
408 btrfs_err(NULL, "invalid string \"%s\"", param->string);
409 return ret;
410 }
411 ctx->max_inline = memparse(param->string, NULL);
412 break;
413 case Opt_acl:
414 if (result.negated) {
415 fc->sb_flags &= ~SB_POSIXACL;
416 } else {
417 #ifdef CONFIG_BTRFS_FS_POSIX_ACL
418 fc->sb_flags |= SB_POSIXACL;
419 #else
420 btrfs_err(NULL, "support for ACL not compiled in");
421 return -EINVAL;
422 #endif
423 }
424 /*
425 * VFS limits the ability to toggle ACL on and off via remount,
426 * despite every file system allowing this. This seems to be
427 * an oversight since we all do, but it'll fail if we're
428 * remounting. So don't set the mask here, we'll check it in
429 * btrfs_reconfigure and do the toggling ourselves.
430 */
431 if (fc->purpose != FS_CONTEXT_FOR_RECONFIGURE)
432 fc->sb_flags_mask |= SB_POSIXACL;
433 break;
434 case Opt_treelog:
435 if (result.negated)
436 btrfs_set_opt(ctx->mount_opt, NOTREELOG);
437 else
438 btrfs_clear_opt(ctx->mount_opt, NOTREELOG);
439 break;
440 case Opt_nologreplay:
441 btrfs_warn(NULL,
442 "'nologreplay' is deprecated, use 'rescue=nologreplay' instead");
443 btrfs_set_opt(ctx->mount_opt, NOLOGREPLAY);
444 break;
445 case Opt_flushoncommit:
446 if (result.negated)
447 btrfs_clear_opt(ctx->mount_opt, FLUSHONCOMMIT);
448 else
449 btrfs_set_opt(ctx->mount_opt, FLUSHONCOMMIT);
450 break;
451 case Opt_ratio:
452 ctx->metadata_ratio = result.uint_32;
453 break;
454 case Opt_discard:
455 if (result.negated) {
456 btrfs_clear_opt(ctx->mount_opt, DISCARD_SYNC);
457 btrfs_clear_opt(ctx->mount_opt, DISCARD_ASYNC);
458 btrfs_set_opt(ctx->mount_opt, NODISCARD);
459 } else {
460 btrfs_set_opt(ctx->mount_opt, DISCARD_SYNC);
461 btrfs_clear_opt(ctx->mount_opt, DISCARD_ASYNC);
462 }
463 break;
464 case Opt_discard_mode:
465 switch (result.uint_32) {
466 case Opt_discard_sync:
467 btrfs_clear_opt(ctx->mount_opt, DISCARD_ASYNC);
468 btrfs_set_opt(ctx->mount_opt, DISCARD_SYNC);
469 break;
470 case Opt_discard_async:
471 btrfs_clear_opt(ctx->mount_opt, DISCARD_SYNC);
472 btrfs_set_opt(ctx->mount_opt, DISCARD_ASYNC);
473 break;
474 default:
475 btrfs_err(NULL, "unrecognized discard mode value %s",
476 param->key);
477 return -EINVAL;
478 }
479 btrfs_clear_opt(ctx->mount_opt, NODISCARD);
480 break;
481 case Opt_space_cache:
482 if (result.negated) {
483 btrfs_set_opt(ctx->mount_opt, NOSPACECACHE);
484 btrfs_clear_opt(ctx->mount_opt, SPACE_CACHE);
485 btrfs_clear_opt(ctx->mount_opt, FREE_SPACE_TREE);
486 } else {
487 btrfs_clear_opt(ctx->mount_opt, FREE_SPACE_TREE);
488 btrfs_set_opt(ctx->mount_opt, SPACE_CACHE);
489 }
490 break;
491 case Opt_space_cache_version:
492 switch (result.uint_32) {
493 case Opt_space_cache_v1:
494 btrfs_set_opt(ctx->mount_opt, SPACE_CACHE);
495 btrfs_clear_opt(ctx->mount_opt, FREE_SPACE_TREE);
496 break;
497 case Opt_space_cache_v2:
498 btrfs_clear_opt(ctx->mount_opt, SPACE_CACHE);
499 btrfs_set_opt(ctx->mount_opt, FREE_SPACE_TREE);
500 break;
501 default:
502 btrfs_err(NULL, "unrecognized space_cache value %s",
503 param->key);
504 return -EINVAL;
505 }
506 break;
507 case Opt_rescan_uuid_tree:
508 btrfs_set_opt(ctx->mount_opt, RESCAN_UUID_TREE);
509 break;
510 case Opt_clear_cache:
511 btrfs_set_opt(ctx->mount_opt, CLEAR_CACHE);
512 break;
513 case Opt_user_subvol_rm_allowed:
514 btrfs_set_opt(ctx->mount_opt, USER_SUBVOL_RM_ALLOWED);
515 break;
516 case Opt_enospc_debug:
517 if (result.negated)
518 btrfs_clear_opt(ctx->mount_opt, ENOSPC_DEBUG);
519 else
520 btrfs_set_opt(ctx->mount_opt, ENOSPC_DEBUG);
521 break;
522 case Opt_defrag:
523 if (result.negated)
524 btrfs_clear_opt(ctx->mount_opt, AUTO_DEFRAG);
525 else
526 btrfs_set_opt(ctx->mount_opt, AUTO_DEFRAG);
527 break;
528 case Opt_usebackuproot:
529 btrfs_warn(NULL,
530 "'usebackuproot' is deprecated, use 'rescue=usebackuproot' instead");
531 btrfs_set_opt(ctx->mount_opt, USEBACKUPROOT);
532
533 /* If we're loading the backup roots we can't trust the space cache. */
534 btrfs_set_opt(ctx->mount_opt, CLEAR_CACHE);
535 break;
536 case Opt_skip_balance:
537 btrfs_set_opt(ctx->mount_opt, SKIP_BALANCE);
538 break;
539 case Opt_fatal_errors:
540 switch (result.uint_32) {
541 case Opt_fatal_errors_panic:
542 btrfs_set_opt(ctx->mount_opt, PANIC_ON_FATAL_ERROR);
543 break;
544 case Opt_fatal_errors_bug:
545 btrfs_clear_opt(ctx->mount_opt, PANIC_ON_FATAL_ERROR);
546 break;
547 default:
548 btrfs_err(NULL, "unrecognized fatal_errors value %s",
549 param->key);
550 return -EINVAL;
551 }
552 break;
553 case Opt_commit_interval:
554 ctx->commit_interval = result.uint_32;
555 if (ctx->commit_interval == 0)
556 ctx->commit_interval = BTRFS_DEFAULT_COMMIT_INTERVAL;
557 break;
558 case Opt_rescue:
559 switch (result.uint_32) {
560 case Opt_rescue_usebackuproot:
561 btrfs_set_opt(ctx->mount_opt, USEBACKUPROOT);
562 break;
563 case Opt_rescue_nologreplay:
564 btrfs_set_opt(ctx->mount_opt, NOLOGREPLAY);
565 break;
566 case Opt_rescue_ignorebadroots:
567 btrfs_set_opt(ctx->mount_opt, IGNOREBADROOTS);
568 break;
569 case Opt_rescue_ignoredatacsums:
570 btrfs_set_opt(ctx->mount_opt, IGNOREDATACSUMS);
571 break;
572 case Opt_rescue_parameter_all:
573 btrfs_set_opt(ctx->mount_opt, IGNOREDATACSUMS);
574 btrfs_set_opt(ctx->mount_opt, IGNOREBADROOTS);
575 btrfs_set_opt(ctx->mount_opt, NOLOGREPLAY);
576 break;
577 default:
578 btrfs_info(NULL, "unrecognized rescue option '%s'",
579 param->key);
580 return -EINVAL;
581 }
582 break;
583 #ifdef CONFIG_BTRFS_DEBUG
584 case Opt_fragment:
585 switch (result.uint_32) {
586 case Opt_fragment_parameter_all:
587 btrfs_set_opt(ctx->mount_opt, FRAGMENT_DATA);
588 btrfs_set_opt(ctx->mount_opt, FRAGMENT_METADATA);
589 break;
590 case Opt_fragment_parameter_metadata:
591 btrfs_set_opt(ctx->mount_opt, FRAGMENT_METADATA);
592 break;
593 case Opt_fragment_parameter_data:
594 btrfs_set_opt(ctx->mount_opt, FRAGMENT_DATA);
595 break;
596 default:
597 btrfs_info(NULL, "unrecognized fragment option '%s'",
598 param->key);
599 return -EINVAL;
600 }
601 break;
602 #endif
603 #ifdef CONFIG_BTRFS_FS_REF_VERIFY
604 case Opt_ref_verify:
605 btrfs_set_opt(ctx->mount_opt, REF_VERIFY);
606 break;
607 #endif
608 default:
609 btrfs_err(NULL, "unrecognized mount option '%s'", param->key);
610 return -EINVAL;
611 }
612
613 return 0;
614 }
615
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 2/3] kstrtox: add unit tests for memparse_safe()
2023-12-23 9:58 ` [PATCH 2/3] kstrtox: add unit tests for memparse_safe() Qu Wenruo
@ 2023-12-26 6:36 ` kernel test robot
2023-12-26 7:37 ` Qu Wenruo
0 siblings, 1 reply; 16+ messages in thread
From: kernel test robot @ 2023-12-26 6:36 UTC (permalink / raw
To: Qu Wenruo, linux-btrfs, linux-kernel, akpm, christophe.jaillet,
andriy.shevchenko, David.Laight, ddiss
Cc: oe-kbuild-all
Hi Qu,
kernel test robot noticed the following build warnings:
[auto build test WARNING on kdave/for-next]
[also build test WARNING on akpm-mm/mm-everything linus/master v6.7-rc7 next-20231222]
[cannot apply to akpm-mm/mm-nonmm-unstable]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Qu-Wenruo/kstrtox-introduce-a-safer-version-of-memparse/20231225-151921
base: https://git.kernel.org/pub/scm/linux/kernel/git/kdave/linux.git for-next
patch link: https://lore.kernel.org/r/56ea15d8b430f4fe3f8e55509ad0bc72b1d9356f.1703324146.git.wqu%40suse.com
patch subject: [PATCH 2/3] kstrtox: add unit tests for memparse_safe()
config: m68k-randconfig-r133-20231226 (https://download.01.org/0day-ci/archive/20231226/202312261423.zqIlU2hn-lkp@intel.com/config)
compiler: m68k-linux-gcc (GCC) 13.2.0
reproduce: (https://download.01.org/0day-ci/archive/20231226/202312261423.zqIlU2hn-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202312261423.zqIlU2hn-lkp@intel.com/
sparse warnings: (new ones prefixed by >>)
>> lib/test-kstrtox.c:339:40: sparse: sparse: cast truncates bits from constant value (efefefef7a7a7a7a becomes 7a7a7a7a)
lib/test-kstrtox.c:351:39: sparse: sparse: cast truncates bits from constant value (efefefef7a7a7a7a becomes 7a7a7a7a)
vim +339 lib/test-kstrtox.c
275
276 /* Want to include "E" suffix for full coverage. */
277 #define MEMPARSE_TEST_SUFFIX (MEMPARSE_SUFFIX_K | MEMPARSE_SUFFIX_M |\
278 MEMPARSE_SUFFIX_G | MEMPARSE_SUFFIX_T |\
279 MEMPARSE_SUFFIX_P | MEMPARSE_SUFFIX_E)
280
281 static void __init test_memparse_safe_fail(void)
282 {
283 struct memparse_test_fail {
284 const char *str;
285 /* Expected error number, either -EINVAL or -ERANGE. */
286 unsigned int expected_ret;
287 };
288 static const struct memparse_test_fail tests[] __initconst = {
289 /* No valid string can be found at all. */
290 {"", -EINVAL},
291 {"\n", -EINVAL},
292 {"\n0", -EINVAL},
293 {"+", -EINVAL},
294 {"-", -EINVAL},
295
296 /*
297 * No support for any leading "+-" chars, even followed by a valid
298 * number.
299 */
300 {"-0", -EINVAL},
301 {"+0", -EINVAL},
302 {"-1", -EINVAL},
303 {"+1", -EINVAL},
304
305 /* Stray suffix would also be rejected. */
306 {"K", -EINVAL},
307 {"P", -EINVAL},
308
309 /* Overflow in the string itself*/
310 {"18446744073709551616", -ERANGE},
311 {"02000000000000000000000", -ERANGE},
312 {"0x10000000000000000", -ERANGE},
313
314 /*
315 * Good string but would overflow with suffix.
316 *
317 * Note, for "E" suffix, one should not use with hex, or "0x1E"
318 * would be treated as 0x1e (30 in decimal), not 0x1 and "E" suffix.
319 * Another reason "E" suffix is cursed.
320 */
321 {"16E", -ERANGE},
322 {"020E", -ERANGE},
323 {"16384P", -ERANGE},
324 {"040000P", -ERANGE},
325 {"16777216T", -ERANGE},
326 {"0100000000T", -ERANGE},
327 {"17179869184G", -ERANGE},
328 {"0200000000000G", -ERANGE},
329 {"17592186044416M", -ERANGE},
330 {"0400000000000000M", -ERANGE},
331 {"18014398509481984K", -ERANGE},
332 {"01000000000000000000K", -ERANGE},
333 };
334 unsigned int i;
335
336 for_each_test(i, tests) {
337 const struct memparse_test_fail *t = &tests[i];
338 unsigned long long tmp = ULL_PATTERN;
> 339 char *retptr = (char *)ULL_PATTERN;
340 int ret;
341
342 ret = memparse_safe(t->str, MEMPARSE_TEST_SUFFIX, &tmp, &retptr);
343 if (ret != t->expected_ret) {
344 WARN(1, "str '%s', expected ret %d got %d\n", t->str,
345 t->expected_ret, ret);
346 continue;
347 }
348 if (tmp != ULL_PATTERN)
349 WARN(1, "str '%s' failed as expected, but result got modified",
350 t->str);
351 if (retptr != (char *)ULL_PATTERN)
352 WARN(1, "str '%s' failed as expected, but pointer got modified",
353 t->str);
354 }
355 }
356
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 2/3] kstrtox: add unit tests for memparse_safe()
2023-12-26 6:36 ` kernel test robot
@ 2023-12-26 7:37 ` Qu Wenruo
2024-01-02 1:33 ` Yujie Liu
0 siblings, 1 reply; 16+ messages in thread
From: Qu Wenruo @ 2023-12-26 7:37 UTC (permalink / raw
To: kernel test robot, Qu Wenruo, linux-btrfs, linux-kernel, akpm,
christophe.jaillet, andriy.shevchenko, David.Laight, ddiss
Cc: oe-kbuild-all
On 2023/12/26 17:06, kernel test robot wrote:
> Hi Qu,
>
> kernel test robot noticed the following build warnings:
>
> [auto build test WARNING on kdave/for-next]
> [also build test WARNING on akpm-mm/mm-everything linus/master v6.7-rc7 next-20231222]
> [cannot apply to akpm-mm/mm-nonmm-unstable]
> [If your patch is applied to the wrong git tree, kindly drop us a note.
> And when submitting patch, we suggest to use '--base' as documented in
> https://git-scm.com/docs/git-format-patch#_base_tree_information]
>
> url: https://github.com/intel-lab-lkp/linux/commits/Qu-Wenruo/kstrtox-introduce-a-safer-version-of-memparse/20231225-151921
> base: https://git.kernel.org/pub/scm/linux/kernel/git/kdave/linux.git for-next
> patch link: https://lore.kernel.org/r/56ea15d8b430f4fe3f8e55509ad0bc72b1d9356f.1703324146.git.wqu%40suse.com
> patch subject: [PATCH 2/3] kstrtox: add unit tests for memparse_safe()
> config: m68k-randconfig-r133-20231226 (https://download.01.org/0day-ci/archive/20231226/202312261423.zqIlU2hn-lkp@intel.com/config)
> compiler: m68k-linux-gcc (GCC) 13.2.0
> reproduce: (https://download.01.org/0day-ci/archive/20231226/202312261423.zqIlU2hn-lkp@intel.com/reproduce)
>
> If you fix the issue in a separate patch/commit (i.e. not just a new version of
> the same patch/commit), kindly add following tags
> | Reported-by: kernel test robot <lkp@intel.com>
> | Closes: https://lore.kernel.org/oe-kbuild-all/202312261423.zqIlU2hn-lkp@intel.com/
>
> sparse warnings: (new ones prefixed by >>)
>>> lib/test-kstrtox.c:339:40: sparse: sparse: cast truncates bits from constant value (efefefef7a7a7a7a becomes 7a7a7a7a)
> lib/test-kstrtox.c:351:39: sparse: sparse: cast truncates bits from constant value (efefefef7a7a7a7a becomes 7a7a7a7a)
Any way to suppress the warning? As long as the constant value (u64) is
checked against the same truncated value (u32), the result should be fine.
I really want to make sure the pointer is not incorrectly updated in the
failure case.
Thanks,
Qu
>
> vim +339 lib/test-kstrtox.c
>
> 275
> 276 /* Want to include "E" suffix for full coverage. */
> 277 #define MEMPARSE_TEST_SUFFIX (MEMPARSE_SUFFIX_K | MEMPARSE_SUFFIX_M |\
> 278 MEMPARSE_SUFFIX_G | MEMPARSE_SUFFIX_T |\
> 279 MEMPARSE_SUFFIX_P | MEMPARSE_SUFFIX_E)
> 280
> 281 static void __init test_memparse_safe_fail(void)
> 282 {
> 283 struct memparse_test_fail {
> 284 const char *str;
> 285 /* Expected error number, either -EINVAL or -ERANGE. */
> 286 unsigned int expected_ret;
> 287 };
> 288 static const struct memparse_test_fail tests[] __initconst = {
> 289 /* No valid string can be found at all. */
> 290 {"", -EINVAL},
> 291 {"\n", -EINVAL},
> 292 {"\n0", -EINVAL},
> 293 {"+", -EINVAL},
> 294 {"-", -EINVAL},
> 295
> 296 /*
> 297 * No support for any leading "+-" chars, even followed by a valid
> 298 * number.
> 299 */
> 300 {"-0", -EINVAL},
> 301 {"+0", -EINVAL},
> 302 {"-1", -EINVAL},
> 303 {"+1", -EINVAL},
> 304
> 305 /* Stray suffix would also be rejected. */
> 306 {"K", -EINVAL},
> 307 {"P", -EINVAL},
> 308
> 309 /* Overflow in the string itself*/
> 310 {"18446744073709551616", -ERANGE},
> 311 {"02000000000000000000000", -ERANGE},
> 312 {"0x10000000000000000", -ERANGE},
> 313
> 314 /*
> 315 * Good string but would overflow with suffix.
> 316 *
> 317 * Note, for "E" suffix, one should not use with hex, or "0x1E"
> 318 * would be treated as 0x1e (30 in decimal), not 0x1 and "E" suffix.
> 319 * Another reason "E" suffix is cursed.
> 320 */
> 321 {"16E", -ERANGE},
> 322 {"020E", -ERANGE},
> 323 {"16384P", -ERANGE},
> 324 {"040000P", -ERANGE},
> 325 {"16777216T", -ERANGE},
> 326 {"0100000000T", -ERANGE},
> 327 {"17179869184G", -ERANGE},
> 328 {"0200000000000G", -ERANGE},
> 329 {"17592186044416M", -ERANGE},
> 330 {"0400000000000000M", -ERANGE},
> 331 {"18014398509481984K", -ERANGE},
> 332 {"01000000000000000000K", -ERANGE},
> 333 };
> 334 unsigned int i;
> 335
> 336 for_each_test(i, tests) {
> 337 const struct memparse_test_fail *t = &tests[i];
> 338 unsigned long long tmp = ULL_PATTERN;
> > 339 char *retptr = (char *)ULL_PATTERN;
> 340 int ret;
> 341
> 342 ret = memparse_safe(t->str, MEMPARSE_TEST_SUFFIX, &tmp, &retptr);
> 343 if (ret != t->expected_ret) {
> 344 WARN(1, "str '%s', expected ret %d got %d\n", t->str,
> 345 t->expected_ret, ret);
> 346 continue;
> 347 }
> 348 if (tmp != ULL_PATTERN)
> 349 WARN(1, "str '%s' failed as expected, but result got modified",
> 350 t->str);
> 351 if (retptr != (char *)ULL_PATTERN)
> 352 WARN(1, "str '%s' failed as expected, but pointer got modified",
> 353 t->str);
> 354 }
> 355 }
> 356
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 3/3] btrfs: migrate to the newer memparse_safe() helper
2023-12-23 9:58 ` [PATCH 3/3] btrfs: migrate to the newer memparse_safe() helper Qu Wenruo
2023-12-26 4:53 ` kernel test robot
2023-12-26 6:06 ` kernel test robot
@ 2023-12-27 6:27 ` David Disseldorp
2023-12-27 8:26 ` Qu Wenruo
2 siblings, 1 reply; 16+ messages in thread
From: David Disseldorp @ 2023-12-27 6:27 UTC (permalink / raw
To: Qu Wenruo
Cc: linux-btrfs, linux-kernel, akpm, christophe.jaillet,
andriy.shevchenko, David.Laight
On Sat, 23 Dec 2023 20:28:07 +1030, Qu Wenruo wrote:
> The new helper has better error report and correct overflow detection,
> furthermore the old @retptr behavior is also kept, thus there should be
> no behavior change.
>
> Signed-off-by: Qu Wenruo <wqu@suse.com>
> ---
> fs/btrfs/ioctl.c | 8 ++++++--
> fs/btrfs/super.c | 8 ++++++++
> fs/btrfs/sysfs.c | 14 +++++++++++---
> 3 files changed, 25 insertions(+), 5 deletions(-)
>
> diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
> index 4e50b62db2a8..8bfd4b4ccf02 100644
> --- a/fs/btrfs/ioctl.c
> +++ b/fs/btrfs/ioctl.c
> @@ -1175,8 +1175,12 @@ static noinline int btrfs_ioctl_resize(struct file *file,
> mod = 1;
> sizestr++;
> }
> - new_size = memparse(sizestr, &retptr);
> - if (*retptr != '\0' || new_size == 0) {
> +
> + ret = memparse_safe(sizestr, MEMPARSE_SUFFIXES_DEFAULT,
> + &new_size, &retptr);
> + if (ret < 0)
> + goto out_finish;
> + if (*retptr != '\0') {
Was dropping the -EINVAL return for new_size=0 intentional?
> ret = -EINVAL;
> goto out_finish;
> }
> diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
> index 3a677b808f0f..2bb6ea525e89 100644
> --- a/fs/btrfs/super.c
> +++ b/fs/btrfs/super.c
> @@ -400,6 +400,14 @@ static int btrfs_parse_param(struct fs_context *fc, struct fs_parameter *param)
> ctx->thread_pool_size = result.uint_32;
> break;
> case Opt_max_inline:
> + int ret;
> +
> + ret = memparse_safe(param->string, MEMPARSE_SUFFIXES_DEFAULT,
> + &ctx->max_inline, NULL);
> + if (ret < 0) {
> + btrfs_err(NULL, "invalid string \"%s\"", param->string);
> + return ret;
> + }
> ctx->max_inline = memparse(param->string, NULL);
Looks like you overlooked removal of the old memparse() call above.
Cheers, David
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 3/3] btrfs: migrate to the newer memparse_safe() helper
2023-12-27 6:27 ` David Disseldorp
@ 2023-12-27 8:26 ` Qu Wenruo
0 siblings, 0 replies; 16+ messages in thread
From: Qu Wenruo @ 2023-12-27 8:26 UTC (permalink / raw
To: David Disseldorp, Qu Wenruo
Cc: linux-btrfs, linux-kernel, akpm, christophe.jaillet,
andriy.shevchenko, David.Laight
On 2023/12/27 16:57, David Disseldorp wrote:
> On Sat, 23 Dec 2023 20:28:07 +1030, Qu Wenruo wrote:
>
>> The new helper has better error report and correct overflow detection,
>> furthermore the old @retptr behavior is also kept, thus there should be
>> no behavior change.
>>
>> Signed-off-by: Qu Wenruo <wqu@suse.com>
>> ---
>> fs/btrfs/ioctl.c | 8 ++++++--
>> fs/btrfs/super.c | 8 ++++++++
>> fs/btrfs/sysfs.c | 14 +++++++++++---
>> 3 files changed, 25 insertions(+), 5 deletions(-)
>>
>> diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
>> index 4e50b62db2a8..8bfd4b4ccf02 100644
>> --- a/fs/btrfs/ioctl.c
>> +++ b/fs/btrfs/ioctl.c
>> @@ -1175,8 +1175,12 @@ static noinline int btrfs_ioctl_resize(struct file *file,
>> mod = 1;
>> sizestr++;
>> }
>> - new_size = memparse(sizestr, &retptr);
>> - if (*retptr != '\0' || new_size == 0) {
>> +
>> + ret = memparse_safe(sizestr, MEMPARSE_SUFFIXES_DEFAULT,
>> + &new_size, &retptr);
>> + if (ret < 0)
>> + goto out_finish;
>> + if (*retptr != '\0') {
>
> Was dropping the -EINVAL return for new_size=0 intentional?
Oh, that's unintentional. Although we would reject the invalid string, a
dedicated "0" can still be parsed.
In that case we should still return -EINVAL.
I just got it confused with the old behavior for invalid string (where 0
is returned and @retptr is not advanced).
>
>> ret = -EINVAL;
>> goto out_finish;
>> }
>> diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
>> index 3a677b808f0f..2bb6ea525e89 100644
>> --- a/fs/btrfs/super.c
>> +++ b/fs/btrfs/super.c
>> @@ -400,6 +400,14 @@ static int btrfs_parse_param(struct fs_context *fc, struct fs_parameter *param)
>> ctx->thread_pool_size = result.uint_32;
>> break;
>> case Opt_max_inline:
>> + int ret;
>> +
>> + ret = memparse_safe(param->string, MEMPARSE_SUFFIXES_DEFAULT,
>> + &ctx->max_inline, NULL);
>> + if (ret < 0) {
>> + btrfs_err(NULL, "invalid string \"%s\"", param->string);
>> + return ret;
>> + }
>> ctx->max_inline = memparse(param->string, NULL);
>
> Looks like you overlooked removal of the old memparse() call above.
My bad, I forgot to remove the old line.
Furthermore, the declaration of "ret" inside case block is not allowed,
I'll fix it anyway.
Thanks,
Qu
>
> Cheers, David
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 1/3] kstrtox: introduce a safer version of memparse()
2023-12-23 9:58 ` [PATCH 1/3] kstrtox: introduce a safer version of memparse() Qu Wenruo
@ 2023-12-27 13:26 ` David Disseldorp
2023-12-27 20:29 ` Qu Wenruo
0 siblings, 1 reply; 16+ messages in thread
From: David Disseldorp @ 2023-12-27 13:26 UTC (permalink / raw
To: Qu Wenruo
Cc: linux-btrfs, linux-kernel, akpm, christophe.jaillet,
andriy.shevchenko, David.Laight
On Sat, 23 Dec 2023 20:28:05 +1030, Qu Wenruo wrote:
> + s = _parse_integer_fixup_radix(s, &base);
> + rv = _parse_integer(s, base, &value);
> + if (rv & KSTRTOX_OVERFLOW)
> + return -ERANGE;
> + if (rv == 0)
> + return -EINVAL;
I was playing around with your unit tests and noticed that "0xG" didn't
reach the expected rv == 0 -> -EINVAL above. It seems that
_parse_integer_fixup_radix() should handle 0x<non hex> differently, or
at least step past any autodetected '0' octal prefix.
Cheers, David
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 0/3] kstrtox: introduce memparse_safe()
2023-12-23 9:58 [PATCH 0/3] kstrtox: introduce memparse_safe() Qu Wenruo
` (2 preceding siblings ...)
2023-12-23 9:58 ` [PATCH 3/3] btrfs: migrate to the newer memparse_safe() helper Qu Wenruo
@ 2023-12-27 16:12 ` Andy Shevchenko
3 siblings, 0 replies; 16+ messages in thread
From: Andy Shevchenko @ 2023-12-27 16:12 UTC (permalink / raw
To: Qu Wenruo
Cc: linux-btrfs, linux-kernel, akpm, christophe.jaillet, David.Laight,
ddiss
On Sat, Dec 23, 2023 at 08:28:04PM +1030, Qu Wenruo wrote:
> Function memparse() lacks error handling:
>
> - If no valid number string at all
> In that case @retptr would just be updated and return value would be
> zero.
>
> - No overflown detection
> This applies to both the number string part, and the suffixes part.
> And since we have no way to indicate errors, we can get weird results
> like:
>
> "25E" -> 10376293541461622784 (9E)
>
> This is due to the fact that for "E" suffix, there is only 4 bits
> left, and 25 with 60 bits left shift would lead to overflow.
> (And decision to support for that "E" suffix is already cursed)
>
> So here we introduce a safer version of it: memparse_safe(), and mark
> the original one deprecated.
> Unfortunately I didn't find a good way to mark it deprecated, as with
> recent -Werror changes, '__deprecated' marco does not seem to warn
> anymore.
>
> The new helper has the following advantages:
>
> - Better overflow and invalid string detection
> The overflow detection is for both the numberic part, and with the
> suffix. Thus above "25E" would be rejected correctly.
> The invalid string part means if there is no valid number starts at
> the buffer, we return -EINVAL.
>
> - Allow caller to select the suffixes, and saner default ones
> The new default one would be "KMGTP", without the cursed and overflow
> prone "E".
> Some older code like setup_elfcorehdr() would benefit from it, if the
> code really wants to only allow "KMG" suffixes.
>
> - Keep the old @retptr behavior
> So the existing callers can easily migrate to the new one, without the
> need to do extra strsep() work.
>
> - Finally test cases
> The test case would cover more things other than the existing kstrtox
> tests:
> * The @retptr behavior
> Either for bad cases, which @retptr should not be touched,
> or for good cases, the @retptr is properly advanced,
>
> * Return value verification
> Make sure we distinguish -EINVAL and -ERANGE correctly.
>
> With the new helper, migrate btrfs to the interface, and since the
> @retptr behavior is the same, we won't cause any behavior change.
Thank you for the prompt update, I will be off till end of January,
and in any case this is material for v6.9+, so I will look at this
afterwards.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 1/3] kstrtox: introduce a safer version of memparse()
2023-12-27 13:26 ` David Disseldorp
@ 2023-12-27 20:29 ` Qu Wenruo
0 siblings, 0 replies; 16+ messages in thread
From: Qu Wenruo @ 2023-12-27 20:29 UTC (permalink / raw
To: David Disseldorp, Qu Wenruo
Cc: linux-btrfs, linux-kernel, akpm, christophe.jaillet,
andriy.shevchenko, David.Laight
On 2023/12/27 23:56, David Disseldorp wrote:
> On Sat, 23 Dec 2023 20:28:05 +1030, Qu Wenruo wrote:
>
>> + s = _parse_integer_fixup_radix(s, &base);
>> + rv = _parse_integer(s, base, &value);
>> + if (rv & KSTRTOX_OVERFLOW)
>> + return -ERANGE;
>> + if (rv == 0)
>> + return -EINVAL;
>
> I was playing around with your unit tests and noticed that "0xG" didn't
> reach the expected rv == 0 -> -EINVAL above. It seems that
> _parse_integer_fixup_radix() should handle 0x<non hex> differently, or
> at least step past any autodetected '0' octal prefix.
Yes, that's also the problem I hit, but I'm not 100% sure if changing
_parse_integer_fixup_radix() is safe for other call sites thus I didn't
put such test case here.
My initial failure cases includes things like "0x" which would still
return 0 is already a warning sign.
Thanks,
Qu
>
> Cheers, David
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 2/3] kstrtox: add unit tests for memparse_safe()
2023-12-26 7:37 ` Qu Wenruo
@ 2024-01-02 1:33 ` Yujie Liu
2024-01-02 2:03 ` Qu Wenruo
0 siblings, 1 reply; 16+ messages in thread
From: Yujie Liu @ 2024-01-02 1:33 UTC (permalink / raw
To: Qu Wenruo
Cc: kernel test robot, Qu Wenruo, linux-btrfs, linux-kernel, akpm,
christophe.jaillet, andriy.shevchenko, David.Laight, ddiss,
oe-kbuild-all
On Tue, Dec 26, 2023 at 06:07:40PM +1030, Qu Wenruo wrote:
> On 2023/12/26 17:06, kernel test robot wrote:
> > Hi Qu,
> >
> > kernel test robot noticed the following build warnings:
> >
> > [auto build test WARNING on kdave/for-next]
> > [also build test WARNING on akpm-mm/mm-everything linus/master v6.7-rc7 next-20231222]
> > [cannot apply to akpm-mm/mm-nonmm-unstable]
> > [If your patch is applied to the wrong git tree, kindly drop us a note.
> > And when submitting patch, we suggest to use '--base' as documented in
> > https://git-scm.com/docs/git-format-patch#_base_tree_information]
> >
> > url: https://github.com/intel-lab-lkp/linux/commits/Qu-Wenruo/kstrtox-introduce-a-safer-version-of-memparse/20231225-151921
> > base: https://git.kernel.org/pub/scm/linux/kernel/git/kdave/linux.git for-next
> > patch link: https://lore.kernel.org/r/56ea15d8b430f4fe3f8e55509ad0bc72b1d9356f.1703324146.git.wqu%40suse.com
> > patch subject: [PATCH 2/3] kstrtox: add unit tests for memparse_safe()
> > config: m68k-randconfig-r133-20231226 (https://download.01.org/0day-ci/archive/20231226/202312261423.zqIlU2hn-lkp@intel.com/config)
> > compiler: m68k-linux-gcc (GCC) 13.2.0
> > reproduce: (https://download.01.org/0day-ci/archive/20231226/202312261423.zqIlU2hn-lkp@intel.com/reproduce)
> >
> > If you fix the issue in a separate patch/commit (i.e. not just a new version of
> > the same patch/commit), kindly add following tags
> > | Reported-by: kernel test robot <lkp@intel.com>
> > | Closes: https://lore.kernel.org/oe-kbuild-all/202312261423.zqIlU2hn-lkp@intel.com/
> >
> > sparse warnings: (new ones prefixed by >>)
> > > > lib/test-kstrtox.c:339:40: sparse: sparse: cast truncates bits from constant value (efefefef7a7a7a7a becomes 7a7a7a7a)
> > lib/test-kstrtox.c:351:39: sparse: sparse: cast truncates bits from constant value (efefefef7a7a7a7a becomes 7a7a7a7a)
>
> Any way to suppress the warning? As long as the constant value (u64) is
> checked against the same truncated value (u32), the result should be fine.
Hi Qu, we've suppressed this warning in the bot for the specific file
lib/test-kstrtox.c, while keep it enabled for the rest. If there are
similar warnings in other files that could be false positives, we will
also suppress them later.
Thanks,
Yujie
>
> I really want to make sure the pointer is not incorrectly updated in the
> failure case.
>
> Thanks,
> Qu
> >
> > vim +339 lib/test-kstrtox.c
> >
> > 275
> > 276 /* Want to include "E" suffix for full coverage. */
> > 277 #define MEMPARSE_TEST_SUFFIX (MEMPARSE_SUFFIX_K | MEMPARSE_SUFFIX_M |\
> > 278 MEMPARSE_SUFFIX_G | MEMPARSE_SUFFIX_T |\
> > 279 MEMPARSE_SUFFIX_P | MEMPARSE_SUFFIX_E)
> > 280
> > 281 static void __init test_memparse_safe_fail(void)
> > 282 {
> > 283 struct memparse_test_fail {
> > 284 const char *str;
> > 285 /* Expected error number, either -EINVAL or -ERANGE. */
> > 286 unsigned int expected_ret;
> > 287 };
> > 288 static const struct memparse_test_fail tests[] __initconst = {
> > 289 /* No valid string can be found at all. */
> > 290 {"", -EINVAL},
> > 291 {"\n", -EINVAL},
> > 292 {"\n0", -EINVAL},
> > 293 {"+", -EINVAL},
> > 294 {"-", -EINVAL},
> > 295
> > 296 /*
> > 297 * No support for any leading "+-" chars, even followed by a valid
> > 298 * number.
> > 299 */
> > 300 {"-0", -EINVAL},
> > 301 {"+0", -EINVAL},
> > 302 {"-1", -EINVAL},
> > 303 {"+1", -EINVAL},
> > 304
> > 305 /* Stray suffix would also be rejected. */
> > 306 {"K", -EINVAL},
> > 307 {"P", -EINVAL},
> > 308
> > 309 /* Overflow in the string itself*/
> > 310 {"18446744073709551616", -ERANGE},
> > 311 {"02000000000000000000000", -ERANGE},
> > 312 {"0x10000000000000000", -ERANGE},
> > 313
> > 314 /*
> > 315 * Good string but would overflow with suffix.
> > 316 *
> > 317 * Note, for "E" suffix, one should not use with hex, or "0x1E"
> > 318 * would be treated as 0x1e (30 in decimal), not 0x1 and "E" suffix.
> > 319 * Another reason "E" suffix is cursed.
> > 320 */
> > 321 {"16E", -ERANGE},
> > 322 {"020E", -ERANGE},
> > 323 {"16384P", -ERANGE},
> > 324 {"040000P", -ERANGE},
> > 325 {"16777216T", -ERANGE},
> > 326 {"0100000000T", -ERANGE},
> > 327 {"17179869184G", -ERANGE},
> > 328 {"0200000000000G", -ERANGE},
> > 329 {"17592186044416M", -ERANGE},
> > 330 {"0400000000000000M", -ERANGE},
> > 331 {"18014398509481984K", -ERANGE},
> > 332 {"01000000000000000000K", -ERANGE},
> > 333 };
> > 334 unsigned int i;
> > 335
> > 336 for_each_test(i, tests) {
> > 337 const struct memparse_test_fail *t = &tests[i];
> > 338 unsigned long long tmp = ULL_PATTERN;
> > > 339 char *retptr = (char *)ULL_PATTERN;
> > 340 int ret;
> > 341
> > 342 ret = memparse_safe(t->str, MEMPARSE_TEST_SUFFIX, &tmp, &retptr);
> > 343 if (ret != t->expected_ret) {
> > 344 WARN(1, "str '%s', expected ret %d got %d\n", t->str,
> > 345 t->expected_ret, ret);
> > 346 continue;
> > 347 }
> > 348 if (tmp != ULL_PATTERN)
> > 349 WARN(1, "str '%s' failed as expected, but result got modified",
> > 350 t->str);
> > 351 if (retptr != (char *)ULL_PATTERN)
> > 352 WARN(1, "str '%s' failed as expected, but pointer got modified",
> > 353 t->str);
> > 354 }
> > 355 }
> > 356
> >
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 2/3] kstrtox: add unit tests for memparse_safe()
2024-01-02 1:33 ` Yujie Liu
@ 2024-01-02 2:03 ` Qu Wenruo
0 siblings, 0 replies; 16+ messages in thread
From: Qu Wenruo @ 2024-01-02 2:03 UTC (permalink / raw
To: Yujie Liu
Cc: kernel test robot, Qu Wenruo, linux-btrfs, linux-kernel, akpm,
christophe.jaillet, andriy.shevchenko, David.Laight, ddiss,
oe-kbuild-all
On 2024/1/2 12:03, Yujie Liu wrote:
> On Tue, Dec 26, 2023 at 06:07:40PM +1030, Qu Wenruo wrote:
>> On 2023/12/26 17:06, kernel test robot wrote:
>>> Hi Qu,
>>>
>>> kernel test robot noticed the following build warnings:
>>>
>>> [auto build test WARNING on kdave/for-next]
>>> [also build test WARNING on akpm-mm/mm-everything linus/master v6.7-rc7 next-20231222]
>>> [cannot apply to akpm-mm/mm-nonmm-unstable]
>>> [If your patch is applied to the wrong git tree, kindly drop us a note.
>>> And when submitting patch, we suggest to use '--base' as documented in
>>> https://git-scm.com/docs/git-format-patch#_base_tree_information]
>>>
>>> url: https://github.com/intel-lab-lkp/linux/commits/Qu-Wenruo/kstrtox-introduce-a-safer-version-of-memparse/20231225-151921
>>> base: https://git.kernel.org/pub/scm/linux/kernel/git/kdave/linux.git for-next
>>> patch link: https://lore.kernel.org/r/56ea15d8b430f4fe3f8e55509ad0bc72b1d9356f.1703324146.git.wqu%40suse.com
>>> patch subject: [PATCH 2/3] kstrtox: add unit tests for memparse_safe()
>>> config: m68k-randconfig-r133-20231226 (https://download.01.org/0day-ci/archive/20231226/202312261423.zqIlU2hn-lkp@intel.com/config)
>>> compiler: m68k-linux-gcc (GCC) 13.2.0
>>> reproduce: (https://download.01.org/0day-ci/archive/20231226/202312261423.zqIlU2hn-lkp@intel.com/reproduce)
>>>
>>> If you fix the issue in a separate patch/commit (i.e. not just a new version of
>>> the same patch/commit), kindly add following tags
>>> | Reported-by: kernel test robot <lkp@intel.com>
>>> | Closes: https://lore.kernel.org/oe-kbuild-all/202312261423.zqIlU2hn-lkp@intel.com/
>>>
>>> sparse warnings: (new ones prefixed by >>)
>>>>> lib/test-kstrtox.c:339:40: sparse: sparse: cast truncates bits from constant value (efefefef7a7a7a7a becomes 7a7a7a7a)
>>> lib/test-kstrtox.c:351:39: sparse: sparse: cast truncates bits from constant value (efefefef7a7a7a7a becomes 7a7a7a7a)
>>
>> Any way to suppress the warning? As long as the constant value (u64) is
>> checked against the same truncated value (u32), the result should be fine.
>
> Hi Qu, we've suppressed this warning in the bot for the specific file
> lib/test-kstrtox.c, while keep it enabled for the rest. If there are
> similar warnings in other files that could be false positives, we will
> also suppress them later.
I'll update the pattern depending on the ULL size to avoid the warning.
Thanks,
Qu
>
> Thanks,
> Yujie
>
>>
>> I really want to make sure the pointer is not incorrectly updated in the
>> failure case.
>>
>> Thanks,
>> Qu
>>>
>>> vim +339 lib/test-kstrtox.c
>>>
>>> 275
>>> 276 /* Want to include "E" suffix for full coverage. */
>>> 277 #define MEMPARSE_TEST_SUFFIX (MEMPARSE_SUFFIX_K | MEMPARSE_SUFFIX_M |\
>>> 278 MEMPARSE_SUFFIX_G | MEMPARSE_SUFFIX_T |\
>>> 279 MEMPARSE_SUFFIX_P | MEMPARSE_SUFFIX_E)
>>> 280
>>> 281 static void __init test_memparse_safe_fail(void)
>>> 282 {
>>> 283 struct memparse_test_fail {
>>> 284 const char *str;
>>> 285 /* Expected error number, either -EINVAL or -ERANGE. */
>>> 286 unsigned int expected_ret;
>>> 287 };
>>> 288 static const struct memparse_test_fail tests[] __initconst = {
>>> 289 /* No valid string can be found at all. */
>>> 290 {"", -EINVAL},
>>> 291 {"\n", -EINVAL},
>>> 292 {"\n0", -EINVAL},
>>> 293 {"+", -EINVAL},
>>> 294 {"-", -EINVAL},
>>> 295
>>> 296 /*
>>> 297 * No support for any leading "+-" chars, even followed by a valid
>>> 298 * number.
>>> 299 */
>>> 300 {"-0", -EINVAL},
>>> 301 {"+0", -EINVAL},
>>> 302 {"-1", -EINVAL},
>>> 303 {"+1", -EINVAL},
>>> 304
>>> 305 /* Stray suffix would also be rejected. */
>>> 306 {"K", -EINVAL},
>>> 307 {"P", -EINVAL},
>>> 308
>>> 309 /* Overflow in the string itself*/
>>> 310 {"18446744073709551616", -ERANGE},
>>> 311 {"02000000000000000000000", -ERANGE},
>>> 312 {"0x10000000000000000", -ERANGE},
>>> 313
>>> 314 /*
>>> 315 * Good string but would overflow with suffix.
>>> 316 *
>>> 317 * Note, for "E" suffix, one should not use with hex, or "0x1E"
>>> 318 * would be treated as 0x1e (30 in decimal), not 0x1 and "E" suffix.
>>> 319 * Another reason "E" suffix is cursed.
>>> 320 */
>>> 321 {"16E", -ERANGE},
>>> 322 {"020E", -ERANGE},
>>> 323 {"16384P", -ERANGE},
>>> 324 {"040000P", -ERANGE},
>>> 325 {"16777216T", -ERANGE},
>>> 326 {"0100000000T", -ERANGE},
>>> 327 {"17179869184G", -ERANGE},
>>> 328 {"0200000000000G", -ERANGE},
>>> 329 {"17592186044416M", -ERANGE},
>>> 330 {"0400000000000000M", -ERANGE},
>>> 331 {"18014398509481984K", -ERANGE},
>>> 332 {"01000000000000000000K", -ERANGE},
>>> 333 };
>>> 334 unsigned int i;
>>> 335
>>> 336 for_each_test(i, tests) {
>>> 337 const struct memparse_test_fail *t = &tests[i];
>>> 338 unsigned long long tmp = ULL_PATTERN;
>>> > 339 char *retptr = (char *)ULL_PATTERN;
>>> 340 int ret;
>>> 341
>>> 342 ret = memparse_safe(t->str, MEMPARSE_TEST_SUFFIX, &tmp, &retptr);
>>> 343 if (ret != t->expected_ret) {
>>> 344 WARN(1, "str '%s', expected ret %d got %d\n", t->str,
>>> 345 t->expected_ret, ret);
>>> 346 continue;
>>> 347 }
>>> 348 if (tmp != ULL_PATTERN)
>>> 349 WARN(1, "str '%s' failed as expected, but result got modified",
>>> 350 t->str);
>>> 351 if (retptr != (char *)ULL_PATTERN)
>>> 352 WARN(1, "str '%s' failed as expected, but pointer got modified",
>>> 353 t->str);
>>> 354 }
>>> 355 }
>>> 356
>>>
>>
>
^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2024-01-02 2:03 UTC | newest]
Thread overview: 16+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-12-23 9:58 [PATCH 0/3] kstrtox: introduce memparse_safe() Qu Wenruo
2023-12-23 9:58 ` [PATCH 1/3] kstrtox: introduce a safer version of memparse() Qu Wenruo
2023-12-27 13:26 ` David Disseldorp
2023-12-27 20:29 ` Qu Wenruo
2023-12-23 9:58 ` [PATCH 2/3] kstrtox: add unit tests for memparse_safe() Qu Wenruo
2023-12-26 6:36 ` kernel test robot
2023-12-26 7:37 ` Qu Wenruo
2024-01-02 1:33 ` Yujie Liu
2024-01-02 2:03 ` Qu Wenruo
2023-12-23 9:58 ` [PATCH 3/3] btrfs: migrate to the newer memparse_safe() helper Qu Wenruo
2023-12-26 4:53 ` kernel test robot
2023-12-26 6:06 ` kernel test robot
2023-12-27 6:27 ` David Disseldorp
2023-12-27 8:26 ` Qu Wenruo
2023-12-27 16:12 ` [PATCH 0/3] kstrtox: introduce memparse_safe() Andy Shevchenko
-- strict thread matches above, loose matches on Subject: below --
2023-12-26 1:24 [PATCH 3/3] btrfs: migrate to the newer memparse_safe() helper kernel test robot
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.