Re: BUG: Bad page map in process udevd (anon_vma: (null)) in2.6.38-rc4

[Eric Dumazet]

Le samedi 19 fÃvrier 2011 Ã 22:15 -0800, Linus Torvalds a Ãcrit :
> On Sat, Feb 19, 2011 at 6:01 PM, Eric W. Biederman
> <ebiederm@xxxxxxxxxxxx> wrote:
> >
> > So I think the change below to fix dev_deactivate which Eric D. missed
> > will fix this problem.  Now to go test that.
> 
> You know what? I think the whole thing is crap. I did a simple grep
> for 'unregister_netdevice_many()', and they are all buggy.
> 


> Look in net/ipv4/ip_gre.c, net/ipv4/ipip.c,net/ipv4/ipmr.c,
> net/ipv6/sit.c, look in net/ipv6/ip6mr.c, just just about anywhere.
> Those people *all* do basically a list-head on the stack, and then
> they do unregister_netdevice_many() on those things, and they clearly
> expect the list to be gone.

If they use rtnl_unlock() they are fine, since by the time rtnl_unlock()
returns, devices have been freed. LIST_HEAD content is void, or else we
have more serious bugs.

> 
> I suspect that the right thing to do really is to change the semantics
> of those functions that take that kill-list *entirely*. Namely that
> they will literall ykill the list too, not just the entries on the
> list.
> 
> So unregister_netdevice_many() should always return with the list
> empty and destroyed. There is no valid use of a list of netdevices
> after you've unregistered them.
> 
> Now, dev_deactivate_many() actually has uses of that list after
> they've been de-activated (__dev_close_many will deactivate them, and
> then after that do the whole ndo_stop dance too, so I guess all (two)
> callers of that function need to get rid of their list manually. So I
> think your patch to sch_generic.c is good, but I really think the
> semantics of unregister_netdevice_many() should just be changed.
> 
> And I think the networking people need to do some serious code review
> of this whole thing. The whole "let's build a list on the stack, then
> leave it around, and later use it randomly when the stack head pointer
> is long gone" thing is just incredible crapola. We shouldn't be
> finding these things one-by-one as a list debugging thing fires.
> People need tolook at their code and fix it before the bugs start
> triggering.

This code is run with RTNL locked anyway, so we could use a global list
head, like net_todo_list list (net/core/dev.c line 4980)

I believe the dev->unreg_list had a precise meaning when I introduced it
in 2009 (commits 44a0873d52282f24b1894c58c0f157e0f626ddc9,
9b5e383c11b08784eb0087617f880077982ef769,
23289a37e2b127dfc4de1313fba15bb4c9f0cd5b) .

devices were added to the LIST_HEAD, but never removed. (devices were
freed anyway, and list manipulated inside RNTL by a single thread)

But as Eric B. said, it was re-used for other roles.

We need to track these changes precisely and make appropriate fixes.



--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/