lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <ZZNn9HKlxbPNLt1H@yujie-X299>
Date: Tue, 2 Jan 2024 09:33:40 +0800
From: Yujie Liu <yujie.liu@...el.com>
To: Qu Wenruo <quwenruo.btrfs@....com>
CC: kernel test robot <lkp@...el.com>, Qu Wenruo <wqu@...e.com>,
	<linux-btrfs@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
	<akpm@...ux-foundation.org>, <christophe.jaillet@...adoo.fr>,
	<andriy.shevchenko@...ux.intel.com>, <David.Laight@...lab.com>,
	<ddiss@...e.de>, <oe-kbuild-all@...ts.linux.dev>
Subject: Re: [PATCH 2/3] kstrtox: add unit tests for memparse_safe()

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@...el.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
> > 
> 

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ