Skip to contentSkip to conversation

sched: delayed cleanup of user_struct

BUG stable, 2.6.32.27 Oleg Nesterov 4 participants 10 messages Jan 10, 2011 2 messages not in the archive

lore
Share & tools
lore

A message that is not in the archive (sent off-list or not yet received): [email protected]

A message that is not in the archive (sent off-list or not yet received): [email protected]

#2 lore raw
Quoted 16 lines · Oleg Nesterov
Quoted 7 lines · Stefan Priebe - Profihost AG

i've seen your patch and i've seen that we've a lot of crashes in the process cleanup since upgrading from 2.6.32.19 to 2.6.32.27 and i would like to know if you can tell me if your patch will solve them.

Log: (ATTENTION Log is in reverse order)
http://pastebin.com/WiyEKScs

No, that patch has nothing to do with this crash.

Looks like, this is CONFIG_USER_SCHED bug. Probably something like double-free but I know nothing about this code and USER_SCHED is deprecated anyway.

Quoted 6 lines

I'd suggest you to disable this option.

Perhaps it makes sense to report this bug to lkml, though. Probably 3959214f971417f4162926ac52ad4cd042958caa is the offending commit.

Yes, at first glance "sched: delayed cleanup of user_struct" looks buggy...

uid_hash_find:

	hlist_for_each_entry(user, h, hashent, uidhash_node) {
		if (user->uid == uid) {
			/* possibly resurrect an "almost deleted" object */
			if (atomic_inc_return(&user->__count) == 1)
				cancel_delayed_work(&user->work);
			return user;

cancel_delayed_work() can only cancel the timer. If the timer has already expired, it can't cancel the pending work, and cleanup_user_struct() can run after uid_hash_find() returns.

This _looks_ OK, cleanup_user_struct() should notice ->__count == 0 and do nothing. But it is not.

Suppose that the new "owner" of this user_struct (the caller of uid_hash_find) in turn does free_uid() before up->work->func() completes. In this case INIT_DELAYED_WORK() can corrupt the pending work, or 2 instances of work->func() can race with each other on different CPUs. In particular, this can lead to double free.

Kay?

Oleg.

#4 lore raw
Quoted 4 lines · Stefan Priebe - Profihost AG

Hi,

is it a workaround to disable CONFIG_GROUP_SCHED?

                                ^^^^^^^^^^^^^^^^^^

CONFIG_USER_SCHED, not CONFIG_GROUP_SCHED.

I think that disabling it should obviously help. But it is possible that there is something else. Given that you can reproduce the crash, it would be nice if you can test this and report ;)

Or. You can revert

	b00bc0b237055b4c45816325ee14f0bd83e6f590
	uids: Prevent tear down race

	3959214f971417f4162926ac52ad4cd042958caa
	sched: delayed cleanup of user_struct

patches for testing.

Oleg.

#5 lore raw
Quoted 4 lines
Quoted 2 lines

is it a workaround to disable CONFIG_GROUP_SCHED?

                                 ^^^^^^^^^^^^^^^^^^

CONFIG_USER_SCHED, not CONFIG_GROUP_SCHED.

correct but CONFIG_USER_SCHED depends on CONFIG_GROUP_SCHED and as we don't use CONFIG_GROUP_SCHED at all my idea was to completely remove this feature.

Quoted 4 lines

I think that disabling it should obviously help. But it is possible that there is something else. Given that you can reproduce the crash, it would be nice if you can test this and report ;)

Main problem - right i can't reproduce it. But the system was crashing two times more in the past.

You can find the logs here:
http://pastebin.com/hNd15E8K

Quoted 6 lines

Or. You can revert

	b00bc0b237055b4c45816325ee14f0bd83e6f590
	uids: Prevent tear down race

	3959214f971417f4162926ac52ad4cd042958caa
	sched: delayed cleanup of user_struct

Not sure if it is a good idea to introduce another race again.

Stefan

#6 lore raw
Quoted 6 lines · Stefan Priebe - Profihost AG

Main problem - right i can't reproduce it. But the system was crashing two times more in the past.

You can find the logs here:
http://pastebin.com/hNd15E8K

Good. Sadly, I didn't see this trace before...

kernel BUG at kernel/workqueue.c:287!

this matches "INIT_DELAYED_WORK() can corrupt the pending work" I mentioned.

Quoted 7 lines
Quoted 6 lines

Or. You can revert

	b00bc0b237055b4c45816325ee14f0bd83e6f590
	uids: Prevent tear down race

	3959214f971417f4162926ac52ad4cd042958caa
	sched: delayed cleanup of user_struct

Not sure if it is a good idea to introduce another race again.

Which race? b00bc0b237055b4c45816325ee14f0bd83e6f590 fixes the race, yes. But this race was introduced by 3959214f971417f4162926ac52ad4cd042958caa. And afaics the latter commit is optimization...

Anyway, this is up to you, of course.

Oleg.

#7 lore raw
Quoted 9 lines
Quoted 2 lines

You can find the logs here:
http://pastebin.com/hNd15E8K

Good. Sadly, I didn't see this trace before...

kernel BUG at kernel/workqueue.c:287!

this matches "INIT_DELAYED_WORK() can corrupt the pending work" I mentioned.

Sorry to ask this but this could also be "workarounded" by disabled GROUP_SCHED? Or is this another thing?

Quoted 6 lines
Quoted 6 lines
Quoted 6 lines

Or. You can revert

	b00bc0b237055b4c45816325ee14f0bd83e6f590
	uids: Prevent tear down race

	3959214f971417f4162926ac52ad4cd042958caa
	sched: delayed cleanup of user_struct
Quoted 7 lines
Quoted 1 line

Not sure if it is a good idea to introduce another race again.

Which race? b00bc0b237055b4c45816325ee14f0bd83e6f590 fixes the race, yes. But this race was introduced by 3959214f971417f4162926ac52ad4cd042958caa. And afaics the latter commit is optimization...

Anyway, this is up to you, of course.

ah OK sorry didn't get this. Will wait if greg or anybody else can tell us something about it.

Thanks for your findings and help!

#8 lore raw
Quoted 4 lines · Stefan Priebe - Profihost AG
Quoted 4 lines
Quoted 3 lines

You can find the logs here:
http://pastebin.com/hNd15E8K

Quoted 8 lines
Quoted 6 lines

Good. Sadly, I didn't see this trace before...

kernel BUG at kernel/workqueue.c:287!

this matches "INIT_DELAYED_WORK() can corrupt the pending work" I mentioned.

Sorry to ask this but this could also be "workarounded" by disabled GROUP_SCHED? Or is this another thing?

Yes, this looks like the same thing.

But let me repeat, today I looked at this code for the first time ;)

Quoted 2 lines

Will wait if greg or anybody else can tell
us something about it.

Indeed.

Oleg.

#9 lore raw
Quoted 14 lines · Oleg Nesterov
Quoted 4 lines · Stefan Priebe - Profihost AG
Quoted 4 lines
Quoted 3 lines

You can find the logs here:
http://pastebin.com/hNd15E8K

Quoted 8 lines
Quoted 6 lines

Good. Sadly, I didn't see this trace before...

kernel BUG at kernel/workqueue.c:287!

this matches "INIT_DELAYED_WORK() can corrupt the pending work" I mentioned.

Sorry to ask this but this could also be "workarounded" by disabled GROUP_SCHED? Or is this another thing?

Quoted 8 lines

Yes, this looks like the same thing.

But let me repeat, today I looked at this code for the first time ;)

Quoted 2 lines

Will wait if greg or anybody else can tell
us something about it.

Indeed.

I'd be inclined to just shoot USER_SCHED, since it's dead upstream. I'm putting a small backport series together, and could add that to the mix.

-Mike

#10 lore raw
In reply to Stefan Priebe - Profihost AG
Quoted 20 lines · Stefan Priebe - Profihost AG
Quoted 9 lines
Quoted 2 lines

You can find the logs here:
http://pastebin.com/hNd15E8K

Good. Sadly, I didn't see this trace before...

kernel BUG at kernel/workqueue.c:287!

this matches "INIT_DELAYED_WORK() can corrupt the pending work" I mentioned.

Sorry to ask this but this could also be "workarounded" by disabled GROUP_SCHED? Or is this another thing?

Quoted 8 lines
Quoted 7 lines
Quoted 6 lines

Or. You can revert

	b00bc0b237055b4c45816325ee14f0bd83e6f590
	uids: Prevent tear down race

	3959214f971417f4162926ac52ad4cd042958caa
	sched: delayed cleanup of user_struct

Not sure if it is a good idea to introduce another race again.

Quoted 7 lines
Quoted 5 lines

Which race? b00bc0b237055b4c45816325ee14f0bd83e6f590 fixes the race, yes. But this race was introduced by 3959214f971417f4162926ac52ad4cd042958caa. And afaics the latter commit is optimization...

Anyway, this is up to you, of course.

ah OK sorry didn't get this. Will wait if greg or anybody else can tell us something about it.

Tell us about what? I'm totally confused here, what is the problem?

thanks,

greg k-h

#11 lore raw
Quoted 2 lines · Greg KH

Tell us about what? I'm totally confused here, what is the problem?

Please look at http://marc.info/?l=linux-kernel&m=129466345327931

I think that free_user's delayed logic is buggy, and the traces from Stefan seems to confirm the theory.

As Mike suggests, we can just remove the deprecated USER_SCHED code and forget about this problem.

Oleg.