| 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
| ||
|
Message-ID: <f763512b-d588-4756-9120-2e283e8f5bb9@gmx.com>
Date: Tue, 2 Jan 2024 12:33:04 +1030
From: Qu Wenruo <quwenruo.btrfs@....com>
To: Yujie Liu <yujie.liu@...el.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 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@...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.
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
>>>
>>
>
Powered by blists - more mailing lists