* [bug report] scftorture: Use a lock-less list to free memory.
@ 2024-11-12 10:23 Dan Carpenter
2024-11-12 16:20 ` [PATCH] scftorture: Handle NULL argument passed to scf_add_to_free_list() Sebastian Andrzej Siewior
0 siblings, 1 reply; 6+ messages in thread
From: Dan Carpenter @ 2024-11-12 10:23 UTC (permalink / raw)
To: Sebastian Andrzej Siewior; +Cc: Paul E. McKenney, kernel-janitors
Hello Sebastian Andrzej Siewior,
Commit 4788c861ad7e ("scftorture: Use a lock-less list to free
memory.") from Nov 8, 2024 (linux-next), leads to the following
Smatch static checker warning:
kernel/scftorture.c:393 scftorture_invoke_one()
error: we previously assumed 'scfcp' could be null (see line 382)
kernel/scftorture.c
342 static void scftorture_invoke_one(struct scf_statistics *scfp, struct torture_random_state *trsp)
343 {
344 bool allocfail = false;
345 uintptr_t cpu;
346 int ret = 0;
347 struct scf_check *scfcp = NULL;
348 struct scf_selector *scfsp = scf_sel_rand(trsp);
349
350 if (scfsp->scfs_prim == SCF_PRIM_SINGLE || scfsp->scfs_wait) {
351 scfcp = kmalloc(sizeof(*scfcp), GFP_ATOMIC);
352 if (!scfcp) {
353 WARN_ON_ONCE(!IS_ENABLED(CONFIG_KASAN));
354 atomic_inc(&n_alloc_errs);
355 allocfail = true;
356 } else {
357 scfcp->scfc_cpu = -1;
358 scfcp->scfc_wait = scfsp->scfs_wait;
359 scfcp->scfc_out = false;
360 scfcp->scfc_rpc = false;
361 }
362 }
363 if (use_cpus_read_lock)
364 cpus_read_lock();
365 else
366 preempt_disable();
367 switch (scfsp->scfs_prim) {
368 case SCF_PRIM_RESCHED:
369 if (IS_BUILTIN(CONFIG_SCF_TORTURE_TEST)) {
370 cpu = torture_random(trsp) % nr_cpu_ids;
371 scfp->n_resched++;
372 resched_cpu(cpu);
373 this_cpu_inc(scf_invoked_count);
374 }
375 break;
376 case SCF_PRIM_SINGLE:
377 cpu = torture_random(trsp) % nr_cpu_ids;
378 if (scfsp->scfs_wait)
379 scfp->n_single_wait++;
380 else
381 scfp->n_single++;
382 if (scfcp) {
This code assumes that scfcp can be NULL.
383 scfcp->scfc_cpu = cpu;
384 barrier(); // Prevent race-reduction compiler optimizations.
385 scfcp->scfc_in = true;
386 }
387 ret = smp_call_function_single(cpu, scf_handler_1, (void *)scfcp, scfsp->scfs_wait);
388 if (ret) {
389 if (scfsp->scfs_wait)
390 scfp->n_single_wait_ofl++;
391 else
392 scfp->n_single_ofl++;
--> 393 scf_add_to_free_list(scfcp);
Originally this was a kfree(scfcp) which can accept a NULL, but
scf_add_to_free_list() can't handle a NULL parameter.
394 scfcp = NULL;
395 }
396 break;
397 case SCF_PRIM_SINGLE_RPC:
regards,
dan carpenter
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH] scftorture: Handle NULL argument passed to scf_add_to_free_list().
2024-11-12 10:23 [bug report] scftorture: Use a lock-less list to free memory Dan Carpenter
@ 2024-11-12 16:20 ` Sebastian Andrzej Siewior
2024-11-12 16:30 ` Dan Carpenter
0 siblings, 1 reply; 6+ messages in thread
From: Sebastian Andrzej Siewior @ 2024-11-12 16:20 UTC (permalink / raw)
To: Dan Carpenter; +Cc: Paul E. McKenney, kernel-janitors
Dan reported that after the rework the newly introduced
scf_add_to_free_list() may get a NULL pointer passed. This replaced
kfree() which was fine with a NULL pointer but scf_add_to_free_list()
isn't.
Let scf_add_to_free_list() handle NULL pointer.
Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
Closes: https://lore.kernel.org/all/2375aa2c-3248-4ffa-b9b0-f0a24c50f237@stanley.mountain
Fixes: 4788c861ad7e9 ("scftorture: Use a lock-less list to free memory.")
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
Thank you Dan. I had to look twice, that `scfsp' above looked almost
identical.
kernel/scftorture.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/kernel/scftorture.c b/kernel/scftorture.c
index eeafd3fc16820..d86d2d9c46248 100644
--- a/kernel/scftorture.c
+++ b/kernel/scftorture.c
@@ -155,6 +155,8 @@ static void scf_add_to_free_list(struct scf_check *scfcp)
struct llist_head *pool;
unsigned int cpu;
+ if (!scfcp)
+ return;
cpu = raw_smp_processor_id() % nthreads;
pool = &per_cpu(scf_free_pool, cpu);
llist_add(&scfcp->scf_node, pool);
--
2.45.2
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] scftorture: Handle NULL argument passed to scf_add_to_free_list().
2024-11-12 16:20 ` [PATCH] scftorture: Handle NULL argument passed to scf_add_to_free_list() Sebastian Andrzej Siewior
@ 2024-11-12 16:30 ` Dan Carpenter
2024-11-12 19:48 ` Paul E. McKenney
0 siblings, 1 reply; 6+ messages in thread
From: Dan Carpenter @ 2024-11-12 16:30 UTC (permalink / raw)
To: Sebastian Andrzej Siewior; +Cc: Paul E. McKenney, kernel-janitors
On Tue, Nov 12, 2024 at 05:20:23PM +0100, Sebastian Andrzej Siewior wrote:
> Dan reported that after the rework the newly introduced
> scf_add_to_free_list() may get a NULL pointer passed. This replaced
> kfree() which was fine with a NULL pointer but scf_add_to_free_list()
> isn't.
>
> Let scf_add_to_free_list() handle NULL pointer.
>
> Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
> Closes: https://lore.kernel.org/all/2375aa2c-3248-4ffa-b9b0-f0a24c50f237@stanley.mountain
> Fixes: 4788c861ad7e9 ("scftorture: Use a lock-less list to free memory.")
> Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> ---
>
> Thank you Dan. I had to look twice, that `scfsp' above looked almost
> identical.
Yeap... Me too. #LowHammingDistance
regards,
dan carpenter
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] scftorture: Handle NULL argument passed to scf_add_to_free_list().
2024-11-12 16:30 ` Dan Carpenter
@ 2024-11-12 19:48 ` Paul E. McKenney
2024-11-13 7:30 ` Dan Carpenter
0 siblings, 1 reply; 6+ messages in thread
From: Paul E. McKenney @ 2024-11-12 19:48 UTC (permalink / raw)
To: Dan Carpenter; +Cc: Sebastian Andrzej Siewior, kernel-janitors
On Tue, Nov 12, 2024 at 07:30:03PM +0300, Dan Carpenter wrote:
> On Tue, Nov 12, 2024 at 05:20:23PM +0100, Sebastian Andrzej Siewior wrote:
> > Dan reported that after the rework the newly introduced
> > scf_add_to_free_list() may get a NULL pointer passed. This replaced
> > kfree() which was fine with a NULL pointer but scf_add_to_free_list()
> > isn't.
> >
> > Let scf_add_to_free_list() handle NULL pointer.
> >
> > Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
> > Closes: https://lore.kernel.org/all/2375aa2c-3248-4ffa-b9b0-f0a24c50f237@stanley.mountain
> > Fixes: 4788c861ad7e9 ("scftorture: Use a lock-less list to free memory.")
> > Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> > ---
> >
> > Thank you Dan. I had to look twice, that `scfsp' above looked almost
> > identical.
Queued and thank you both!
> Yeap... Me too. #LowHammingDistance
That could likely be improved, to be sure. I am one of these strange
people who wants the pointer to give a hint about its type, something
about pointers to rcu_data, rcu_node, and rcu_state pointers all
being messed with at the same time.
Maybe s/scfcp/scfchkp/?
Thanx, Paul
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] scftorture: Handle NULL argument passed to scf_add_to_free_list().
2024-11-12 19:48 ` Paul E. McKenney
@ 2024-11-13 7:30 ` Dan Carpenter
2024-11-13 14:57 ` Paul E. McKenney
0 siblings, 1 reply; 6+ messages in thread
From: Dan Carpenter @ 2024-11-13 7:30 UTC (permalink / raw)
To: Paul E. McKenney; +Cc: Sebastian Andrzej Siewior, kernel-janitors
On Tue, Nov 12, 2024 at 11:48:41AM -0800, Paul E. McKenney wrote:
> On Tue, Nov 12, 2024 at 07:30:03PM +0300, Dan Carpenter wrote:
> > On Tue, Nov 12, 2024 at 05:20:23PM +0100, Sebastian Andrzej Siewior wrote:
> > > Dan reported that after the rework the newly introduced
> > > scf_add_to_free_list() may get a NULL pointer passed. This replaced
> > > kfree() which was fine with a NULL pointer but scf_add_to_free_list()
> > > isn't.
> > >
> > > Let scf_add_to_free_list() handle NULL pointer.
> > >
> > > Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
> > > Closes: https://lore.kernel.org/all/2375aa2c-3248-4ffa-b9b0-f0a24c50f237@stanley.mountain
> > > Fixes: 4788c861ad7e9 ("scftorture: Use a lock-less list to free memory.")
> > > Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> > > ---
> > >
> > > Thank you Dan. I had to look twice, that `scfsp' above looked almost
> > > identical.
>
> Queued and thank you both!
>
> > Yeap... Me too. #LowHammingDistance
>
> That could likely be improved, to be sure. I am one of these strange
> people who wants the pointer to give a hint about its type, something
> about pointers to rcu_data, rcu_node, and rcu_state pointers all
> being messed with at the same time.
>
> Maybe s/scfcp/scfchkp/?
>
What about scf_chkp, maybe?
regards,
dan carpenter
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] scftorture: Handle NULL argument passed to scf_add_to_free_list().
2024-11-13 7:30 ` Dan Carpenter
@ 2024-11-13 14:57 ` Paul E. McKenney
0 siblings, 0 replies; 6+ messages in thread
From: Paul E. McKenney @ 2024-11-13 14:57 UTC (permalink / raw)
To: Dan Carpenter; +Cc: Sebastian Andrzej Siewior, kernel-janitors
On Wed, Nov 13, 2024 at 10:30:04AM +0300, Dan Carpenter wrote:
> On Tue, Nov 12, 2024 at 11:48:41AM -0800, Paul E. McKenney wrote:
> > On Tue, Nov 12, 2024 at 07:30:03PM +0300, Dan Carpenter wrote:
> > > On Tue, Nov 12, 2024 at 05:20:23PM +0100, Sebastian Andrzej Siewior wrote:
> > > > Dan reported that after the rework the newly introduced
> > > > scf_add_to_free_list() may get a NULL pointer passed. This replaced
> > > > kfree() which was fine with a NULL pointer but scf_add_to_free_list()
> > > > isn't.
> > > >
> > > > Let scf_add_to_free_list() handle NULL pointer.
> > > >
> > > > Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
> > > > Closes: https://lore.kernel.org/all/2375aa2c-3248-4ffa-b9b0-f0a24c50f237@stanley.mountain
> > > > Fixes: 4788c861ad7e9 ("scftorture: Use a lock-less list to free memory.")
> > > > Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> > > > ---
> > > >
> > > > Thank you Dan. I had to look twice, that `scfsp' above looked almost
> > > > identical.
> >
> > Queued and thank you both!
> >
> > > Yeap... Me too. #LowHammingDistance
> >
> > That could likely be improved, to be sure. I am one of these strange
> > people who wants the pointer to give a hint about its type, something
> > about pointers to rcu_data, rcu_node, and rcu_state pointers all
> > being messed with at the same time.
> >
> > Maybe s/scfcp/scfchkp/?
>
> What about scf_chkp, maybe?
I would rather avoid the underscore, myself, but...
Thanx, Paul
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2024-11-13 14:57 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-11-12 10:23 [bug report] scftorture: Use a lock-less list to free memory Dan Carpenter
2024-11-12 16:20 ` [PATCH] scftorture: Handle NULL argument passed to scf_add_to_free_list() Sebastian Andrzej Siewior
2024-11-12 16:30 ` Dan Carpenter
2024-11-12 19:48 ` Paul E. McKenney
2024-11-13 7:30 ` Dan Carpenter
2024-11-13 14:57 ` Paul E. McKenney
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.