On Wed, Apr 29, 2020 at 04:24:02PM +0300, Dan Carpenter wrote:
> Hello Paul E. McKenney,
>
> The patch e02882cd57e3: "rcutorture: Add races with task-exit
> processing" from Apr 24, 2020, leads to the following static checker
> warning:
>
> kernel/rcu/rcutorture.c:2429 rcu_torture_read_exit()
> warn: 'rep' was already freed.
Ah, good catch, will fix!
> kernel/rcu/rcutorture.c
> 2369 static int rcu_torture_read_exit(void *unused)
> 2370 {
> 2371 int count = 0;
> 2372 bool errexit = false;
> 2373 int i;
> 2374 struct task_struct **rep;
> 2375 struct torture_random_state *trsp;
> 2376
> 2377 // Allocate and initialize.
> 2378 set_user_nice(current, MAX_NICE);
> 2379 rep = kcalloc(read_exit, sizeof(*rep), GFP_KERNEL);
> 2380 trsp = kcalloc(read_exit, sizeof(*trsp), GFP_KERNEL);
> 2381 if (rep && trsp) {
> 2382 for (i = 0; i < read_exit; i++)
> 2383 torture_random_init(&trsp[i]);
> 2384 VERBOSE_TOROUT_STRING("rcu_torture_read_exit: Start of test");
> 2385 } else {
> 2386 kfree(rep);
> ^^^
> 2387 kfree(trsp);
> ^^^^
> Freed.
And setting both rep and trsp to NULL here takes care of this, correct?
As in kfree(NULL) is a no-op. Or is your tool more strict?
Thanx, Paul
> 2388 errexit = true;
> 2389 VERBOSE_TOROUT_ERRSTRING("out of memory");
> 2390 }
> 2391
> 2392 // Each pass through this loop does one read-exit episode.
> 2393 while (!errexit && ! READ_ONCE(read_exit_child_stop)) {
> 2394 if (++count > read_exit_burst) {
> 2395 VERBOSE_TOROUT_STRING("rcu_torture_read_exit: End of episode");
> 2396 schedule_timeout_uninterruptible(HZ * read_exit_delay);
> 2397 VERBOSE_TOROUT_STRING("rcu_torture_read_exit: Start of episode");
> 2398 count = 0;
> 2399 }
> 2400 // Spawn children.
> 2401 for (i = 0; i < read_exit && i <= num_online_cpus(); i++) {
> 2402 // We don't want per-child console messages.
> 2403 rep[i] = kthread_run(rcu_torture_read_exit_child,
> 2404 &trsp[i], "%s",
> 2405 "rcu_torture_read_exit_child");
> 2406 if (IS_ERR(rep[i])) {
> 2407 VERBOSE_TOROUT_ERRSTRING("out of memory");
> 2408 errexit = true;
> 2409 rep[i] = NULL;
> 2410 break;
> 2411 }
> 2412 cond_resched();
> 2413 }
> 2414 n_read_exits += i;
> 2415 // Reap children.
> 2416 for (i--; i >= 0; i--) {
> 2417 kthread_stop(rep[i]);
> 2418 rep[i] = NULL;
> 2419 cond_resched();
> 2420 }
> 2421 rcu_barrier(); // Wait for task_struct freeing, avoid OOM.
> 2422 stutter_wait("rcu_torture_read_exit");
> 2423 }
> 2424
> 2425 // Clean up and exit.
> 2426 smp_store_release(&read_exit_child_stopped, true); // After reaping.
> 2427 smp_mb(); // Store before wakeup.
> 2428 wake_up(&read_exit_wq);
> 2429 kfree(rep);
> ^^^
> 2430 kfree(trsp);
> ^^^^
> Double freed.
>
> 2431 while (!torture_must_stop())
> 2432 schedule_timeout_uninterruptible(1);
> 2433 torture_kthread_stopping("rcu_torture_read_exit");
> 2434 return 0;
> 2435 }
>
> regards,
> dan carpenter
