Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 750d857c authored by Robert Richter's avatar Robert Richter
Browse files

oprofile: fix crash when accessing freed task structs 



This patch fixes a crash during shutdown reported below. The crash is
caused by accessing already freed task structs. The fix changes the
order for registering and unregistering notifier callbacks.

All notifiers must be initialized before buffers start working. To
stop buffer synchronization we cancel all workqueues, unregister the
notifier callback and then flush all buffers. After all of this we
finally can free all tasks listed.

This should avoid accessing freed tasks.

On 22.07.10 01:14:40, Benjamin Herrenschmidt wrote:

> So the initial observation is a spinlock bad magic followed by a crash
> in the spinlock debug code:
>
> [ 1541.586531] BUG: spinlock bad magic on CPU#5, events/5/136
> [ 1541.597564] Unable to handle kernel paging request for data at address 0x6b6b6b6b6b6b6d03
>
> Backtrace looks like:
>
>       spin_bug+0x74/0xd4
>       ._raw_spin_lock+0x48/0x184
>       ._spin_lock+0x10/0x24
>       .get_task_mm+0x28/0x8c
>       .sync_buffer+0x1b4/0x598
>       .wq_sync_buffer+0xa0/0xdc
>       .worker_thread+0x1d8/0x2a8
>       .kthread+0xa8/0xb4
>       .kernel_thread+0x54/0x70
>
> So we are accessing a freed task struct in the work queue when
> processing the samples.

Reported-by: default avatarBenjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: stable@kernel.org
Signed-off-by: default avatarRobert Richter <robert.richter@amd.com>
parent da5cabf8

drivers/oprofile/buffer_sync.c

+14 −13
Original line number Original line Diff line number Diff line
@@ -141,16 +141,6 @@ static struct notifier_block module_load_nb = { 
	.notifier_call = module_load_notify,

	.notifier_call = module_load_notify,

};

};







static void end_sync(void)

{

	end_cpu_work();

	/* make sure we don't leak task structs */

	process_task_mortuary();

	process_task_mortuary();

}





int sync_start(void)

int sync_start(void)

{

{

	int err;

	int err;
@@ -158,7 +148,7 @@ int sync_start(void) 
	if (!zalloc_cpumask_var(&marked_cpus, GFP_KERNEL))

	if (!zalloc_cpumask_var(&marked_cpus, GFP_KERNEL))

		return -ENOMEM;

		return -ENOMEM;





	start_cpu_work();

	mutex_lock(&buffer_mutex);





	err = task_handoff_register(&task_free_nb);

	err = task_handoff_register(&task_free_nb);

	if (err)

	if (err)
@@ -173,7 +163,10 @@ int sync_start(void) 
	if (err)

	if (err)

		goto out4;

		goto out4;





	start_cpu_work();



out:

out:

	mutex_unlock(&buffer_mutex);

	return err;

	return err;

out4:

out4:

	profile_event_unregister(PROFILE_MUNMAP, &munmap_nb);

	profile_event_unregister(PROFILE_MUNMAP, &munmap_nb);
@@ -182,7 +175,6 @@ int sync_start(void) 
out2:

out2:

	task_handoff_unregister(&task_free_nb);

	task_handoff_unregister(&task_free_nb);

out1:

out1:

	end_sync();

	free_cpumask_var(marked_cpus);

	free_cpumask_var(marked_cpus);

	goto out;

	goto out;

}

}
@@ -190,11 +182,20 @@ int sync_start(void) 




void sync_stop(void)

void sync_stop(void)

{

{

	/* flush buffers */

	mutex_lock(&buffer_mutex);

	end_cpu_work();

	unregister_module_notifier(&module_load_nb);

	unregister_module_notifier(&module_load_nb);

	profile_event_unregister(PROFILE_MUNMAP, &munmap_nb);

	profile_event_unregister(PROFILE_MUNMAP, &munmap_nb);

	profile_event_unregister(PROFILE_TASK_EXIT, &task_exit_nb);

	profile_event_unregister(PROFILE_TASK_EXIT, &task_exit_nb);

	task_handoff_unregister(&task_free_nb);

	task_handoff_unregister(&task_free_nb);

	end_sync();

	mutex_unlock(&buffer_mutex);

	flush_scheduled_work();



	/* make sure we don't leak task structs */

	process_task_mortuary();

	process_task_mortuary();



	free_cpumask_var(marked_cpus);

	free_cpumask_var(marked_cpus);

}

}

drivers/oprofile/cpu_buffer.c

+0 −2
Original line number Original line Diff line number Diff line
@@ -120,8 +120,6 @@ void end_cpu_work(void) 




		cancel_delayed_work(&b->work);

		cancel_delayed_work(&b->work);

	}

	}



	flush_scheduled_work();

}

}





/*

/*