Re: mm/kfence/kfence_test.c:287 test_alloc() warn: use 'gfp' here instead of GFP_KERNEL?

[Helge Deller]

On 7/27/23 06:51, Dan Carpenter wrote:
> Hi Helge,
>
> FYI, the error/warning was bisected to this commit, please ignore it if it's irrelevant.
>
> tree:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
> head:   18b44bc5a67275641fb26f2c54ba7eef80ac5950
> commit: adf8e96a7ea670d45b5de7594acc67e8f4787ae6 parisc: Enable LOCKDEP support
> config: parisc-randconfig-m041-20230726 (https://download.01.org/0day-ci/archive/20230727/202307270305.L19EfaJD-lkp@xxxxxxxxx/config)
> compiler: hppa-linux-gcc (GCC) 12.3.0
> reproduce: (https://download.01.org/0day-ci/archive/20230727/202307270305.L19EfaJD-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>
> | Reported-by: Dan Carpenter <dan.carpenter@xxxxxxxxxx>
> | Closes: https://lore.kernel.org/r/202307270305.L19EfaJD-lkp@xxxxxxxxx/
>
> 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'
>
> vim +/gfp +287 mm/kfence/kfence_test.c
>
> bc8fbc5f305aec Marco Elver     2021-02-25  243  static void *test_alloc(struct kunit *test, size_t size, gfp_t gfp, enum allocation_policy policy)
> bc8fbc5f305aec Marco Elver     2021-02-25  244  {
> bc8fbc5f305aec Marco Elver     2021-02-25  245  	void *alloc;
> bc8fbc5f305aec Marco Elver     2021-02-25  246  	unsigned long timeout, resched_after;
> bc8fbc5f305aec Marco Elver     2021-02-25  247  	const char *policy_name;
> bc8fbc5f305aec Marco Elver     2021-02-25  248
> bc8fbc5f305aec Marco Elver     2021-02-25  249  	switch (policy) {
> bc8fbc5f305aec Marco Elver     2021-02-25  250  	case ALLOCATE_ANY:
> bc8fbc5f305aec Marco Elver     2021-02-25  251  		policy_name = "any";
> bc8fbc5f305aec Marco Elver     2021-02-25  252  		break;
> bc8fbc5f305aec Marco Elver     2021-02-25  253  	case ALLOCATE_LEFT:
> bc8fbc5f305aec Marco Elver     2021-02-25  254  		policy_name = "left";
> bc8fbc5f305aec Marco Elver     2021-02-25  255  		break;
> bc8fbc5f305aec Marco Elver     2021-02-25  256  	case ALLOCATE_RIGHT:
> bc8fbc5f305aec Marco Elver     2021-02-25  257  		policy_name = "right";
> bc8fbc5f305aec Marco Elver     2021-02-25  258  		break;
> bc8fbc5f305aec Marco Elver     2021-02-25  259  	case ALLOCATE_NONE:
> bc8fbc5f305aec Marco Elver     2021-02-25  260  		policy_name = "none";
> bc8fbc5f305aec Marco Elver     2021-02-25  261  		break;
> bc8fbc5f305aec Marco Elver     2021-02-25  262  	}
> bc8fbc5f305aec Marco Elver     2021-02-25  263
> bc8fbc5f305aec Marco Elver     2021-02-25  264  	kunit_info(test, "%s: size=%zu, gfp=%x, policy=%s, cache=%i\n", __func__, size, gfp,
> bc8fbc5f305aec Marco Elver     2021-02-25  265  		   policy_name, !!test_cache);
> bc8fbc5f305aec Marco Elver     2021-02-25  266
> bc8fbc5f305aec Marco Elver     2021-02-25  267  	/*
> bc8fbc5f305aec Marco Elver     2021-02-25  268  	 * 100x the sample interval should be more than enough to ensure we get
> bc8fbc5f305aec Marco Elver     2021-02-25  269  	 * a KFENCE allocation eventually.
> bc8fbc5f305aec Marco Elver     2021-02-25  270  	 */
> 8913c610014823 Peng Liu        2022-02-11  271  	timeout = jiffies + msecs_to_jiffies(100 * kfence_sample_interval);
> bc8fbc5f305aec Marco Elver     2021-02-25  272  	/*
> bc8fbc5f305aec Marco Elver     2021-02-25  273  	 * Especially for non-preemption kernels, ensure the allocation-gate
> bc8fbc5f305aec Marco Elver     2021-02-25  274  	 * timer can catch up: after @resched_after, every failed allocation
> bc8fbc5f305aec Marco Elver     2021-02-25  275  	 * attempt yields, to ensure the allocation-gate timer is scheduled.
> bc8fbc5f305aec Marco Elver     2021-02-25  276  	 */
> 8913c610014823 Peng Liu        2022-02-11  277  	resched_after = jiffies + msecs_to_jiffies(kfence_sample_interval);
> 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.

Since this is a test program, I assume that "GFP_KERNEL" is used intentionally
instead of "gfp" here to check if the "kmalloc(size, gfp)" above gets the right kmalloc_caches[].
If so, is there a way to silence the smatch warning ("mm/kfence/kfence_test.c:287 test_alloc() warn: use 'gfp' here instead of GFP_KERNEL?") ?
But I'm not sure either, so adding Hyeonggon to the CC too...

Helge