Re: [PATCH 2/3] kstrtox: add unit tests for memparse_safe()

[Qu Wenruo]

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@xxxxxxxxx/config)
> compiler: m68k-linux-gcc (GCC) 13.2.0
> reproduce: (https://download.01.org/0day-ci/archive/20231226/202312261423.zqIlU2hn-lkp@xxxxxxxxx/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@xxxxxxxxx>
> | Closes: https://lore.kernel.org/oe-kbuild-all/202312261423.zqIlU2hn-lkp@xxxxxxxxx/
>
> 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