Messages in this thread | | | From | Marco Elver <> | Date | Thu, 27 Jul 2023 10:06:25 +0200 | Subject | Re: mm/kfence/kfence_test.c:287 test_alloc() warn: use 'gfp' here instead of GFP_KERNEL? |
| |
On Thu, 27 Jul 2023 at 06:51, Dan Carpenter <dan.carpenter@linaro.org> wrote: [...] > smatch warnings: > mm/kfence/kfence_test.c:287 test_alloc() warn: use 'gfp' here instead of GFP_KERNEL? > > (Just included these for the LOLs) > mm/kfence/kfence_test.c:395 test_double_free() error: double free of 'expect.addr' > mm/kfence/kfence_test.c:671 test_memcache_typesafe_by_rcu() error: dereferencing freed memory 'expect.addr'
Nice! ;-)
> vim +/gfp +287 mm/kfence/kfence_test.c > [...] > bc8fbc5f305aec Marco Elver 2021-02-25 278 do { > bc8fbc5f305aec Marco Elver 2021-02-25 279 if (test_cache) > bc8fbc5f305aec Marco Elver 2021-02-25 280 alloc = kmem_cache_alloc(test_cache, gfp); > bc8fbc5f305aec Marco Elver 2021-02-25 281 else > bc8fbc5f305aec Marco Elver 2021-02-25 282 alloc = kmalloc(size, gfp); > ^^^ > > bc8fbc5f305aec Marco Elver 2021-02-25 283 > bc8fbc5f305aec Marco Elver 2021-02-25 284 if (is_kfence_address(alloc)) { > 8dae0cfed57357 Vlastimil Babka 2021-11-03 285 struct slab *slab = virt_to_slab(alloc); > 588c7fa022d7b2 Hyeonggon Yoo 2021-06-28 286 struct kmem_cache *s = test_cache ?: > 588c7fa022d7b2 Hyeonggon Yoo 2021-06-28 @287 kmalloc_caches[kmalloc_type(GFP_KERNEL)][__kmalloc_index(size, false)]; > ^^^^^^^^^^ > I feel like using gfp might be correct but I'm not sure? This code > is from prior to this commit. Let's add Marco to the CC.
It's not a bug today: If we were testing something other than GFP_KERNEL, then yes, we should use gfp here. But the only reason "gfp" is a function arg at all, is that 1 test case also uses __GFP_ZERO, but that's irrelevant in getting the kmalloc type (at least today).
The cache is used to test the below "helpers return the right values even for KFENCE objects".
I think when I wrote the test I just chose GFP_KERNEL, because none of the test cases use something else, and didn't give it a second thought. Not sure it's worth changing, because functionally it doesn't matter.
I'm open to changing it, if it makes this warning go away. Preferences?
> bc8fbc5f305aec Marco Elver 2021-02-25 288 > bc8fbc5f305aec Marco Elver 2021-02-25 289 /* > bc8fbc5f305aec Marco Elver 2021-02-25 290 * Verify that various helpers return the right values > bc8fbc5f305aec Marco Elver 2021-02-25 291 * even for KFENCE objects; these are required so that > bc8fbc5f305aec Marco Elver 2021-02-25 292 * memcg accounting works correctly. > bc8fbc5f305aec Marco Elver 2021-02-25 293 */ > 8dae0cfed57357 Vlastimil Babka 2021-11-03 294 KUNIT_EXPECT_EQ(test, obj_to_index(s, slab, alloc), 0U); > 8dae0cfed57357 Vlastimil Babka 2021-11-03 295 KUNIT_EXPECT_EQ(test, objs_per_slab(s, slab), 1);
| |