Re: 2.6.26-rc4: RIP find_pid_ns+0x6b/0xa0

From: Linus Torvalds
Date: Tue May 27 2008 - 11:03:48 EST




On Tue, 27 May 2008, Oleg Nesterov wrote:

> On 05/27, Alexey Dobriyan wrote:
> >
> > PREEMPT_RCU is in use, again.

I do wonder if PREEMPT_RCU is broken.

> > 0xffffffff802447cb is in find_pid_ns (kernel/pid.c:297).
> > 292 struct hlist_node *elem;
> > 293 struct upid *pnr;
> > 294
> > 295 hlist_for_each_entry_rcu(pnr, elem,
> > 296 &pid_hash[pid_hashfn(nr, ns)], pid_chain)
> > 297 if (pnr->nr == nr && pnr->ns == ns)

> > general protection fault: 0000 [2] PREEMPT SMP DEBUG_PAGEALLOC
> > RDX: 6b6b6b6b6b6b6b6b RSI: ffffffff80566760 RDI: 0000000000003cef

That repeated 0x6b is POISON_FREE, and the code is

cmp -0x10(%rdx),%edi

which is the load of "pnr->nr". So 'pnr' has been free'd.

On Tue, 27 May 2008, Oleg Nesterov wrote:
>
> Is this reproducible?
>
> In theory find_pid() is not safe without rcu_read_lock() if CONFIG_PREEMPT_RCU.
> But we have a lot of "read_lock(tasklist_lock) + find_pid()", this was legal
> and documented. It was actually broken, but happened to work because read_lock()
> implied rcu_read_lock().
>
> Could you look at
>
> [PATCH] fix tasklist + find_pid() with CONFIG_PREEMPT_RCU
> http://marc.info/?t=120162615300012
>
> ?
>
> I am not sure this is the actual reason though, the race is very unlikely.

That is a *very* unlikely race, especially as that bad_fork_free_pid case
would only happen if pid_ns_prepare_proc() fails. And if it fails, it's
still very unlikely to hit, I think.

That said, it does smell like a bug. But I *really* would be much much
happier if even SRCU at least waited for a grace period, so that it would
always be safe to just disable preemption for a "rcu_read_lock()". That
way, things that take spinlocks are safe even with SRCU.

Paul? How hard would it be to make preemptable RCU just honor that classic
RCU behavior?

Linus
--
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/