Re: kmemcheck caught read from freed memory (cfq_free_io_context)

On Wed, Apr 02 2008, Peter Zijlstra wrote:
On Wed, 2008-04-02 at 13:53 +0200, Jens Axboe wrote:
On Wed, Apr 02 2008, Peter Zijlstra wrote:
On Wed, 2008-04-02 at 13:42 +0200, Jens Axboe wrote:
On Wed, Apr 02 2008, Peter Zijlstra wrote:
On Wed, 2008-04-02 at 13:32 +0200, Jens Axboe wrote:
On Wed, Apr 02 2008, Peter Zijlstra wrote:
On Wed, 2008-04-02 at 13:14 +0200, Jens Axboe wrote:
On Wed, Apr 02 2008, Peter Zijlstra wrote:
On Wed, 2008-04-02 at 13:07 +0200, Jens Axboe wrote:
On Wed, Apr 02 2008, Pekka Enberg wrote:
Hi Paul,

On Wed, Apr 2, 2008 at 1:55 PM, Paul E. McKenney
<paulmck@xxxxxxxxxxxxxxxxxx> wrote:
I will check this when I get back to some bandwidth -- but in the meantime,
does kmemcheck special-case SLAB_DESTROY_BY_RCU? It is legal to access
newly-freed items in that case, as long as you did rcu_read_lock()
before gaining a reference to them and don't hold the reference past
the matching rcu_read_unlock().

No, kmemcheck is work in progress and does not know about
SLAB_DESTROY_BY_RCU yet. The reason I asked Vegard to post the warning
was because Peter, Vegard, and myself identified this particular
warning as a real problem. But yeah, kmemcheck can cause false
positives for RCU for now.

Makes sense, and to me Pauls analysis of the code looks totally correct
- there's no bug there, at least related to hlist traversal and
kmem_cache_free(), since we are under rcu_read_lock() and thus hold off
the grace for freeing.

but what holds off the slab allocator re-issueing that same object and
someone else writing other stuff into it?

Nothing, that's how rcu destry works here. But for the validation to be
WRONG radix_tree_lookup(..., old_key) must return cic for new_key, not
NULL.



A B C

cfq_cic_lookup(cfqd_1, ioc)

rcu_read_lock()
cic = radix_tree_lookup(, cfqd_q);

cfq_cic_free()

cfq_cic_link(cfqd_2, ioc,)

rcu_read_unlock()


and now we have that:

cic->key == cfqd_2


I'm not seeing anything stopping this from happening.

I don't follow your A-B-C here, what do they refer to?

A does a radix_tree_lookup() of cfqd_1 (darn typos)
B does a kfree of the same cic found by A
C does an alloc and gets the same cic as freed by B and inserts it
in a different location.

So that when we return to A, cic->key == cfqd_2 even though we did a
lookup for cfqd_1.

That I follow, my question was if A, B, and C refer to different
processes but with a shared io context? I'm assuming that is correct...

Ah, yeah, whatever is needed to make this race happen :-)

The only place where you'll have multiple processes involved with this
at all is if they share io contexts. That is also why the bug isn't that
critical, since it's not possible right now (CLONE_IO flag must be
used).

There are 3 races here:

1) A continues with another object than intended
(requires CLONE_IO)

2) A does hlist_for_each_rcu() and races with B,C so that
we continue the iteration on a possibly unrelated list.

3) cic is freed after the !cic->key check.

I'm not familiar enough with the code yet to see if 3 really is an
possibility. But from what I can see there is nothing guarding its
existence.

All 3 require CLONE_IO, because if that is not set, there's a 1:1
mapping between a process and io context (no sharing occurs).

--
Jens Axboe

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

Relevant Pages

  • Re: kmemcheck caught read from freed memory (cfq_free_io_context)
    ... On Wed, Apr 02 2008, Peter Zijlstra wrote: ... On Wed, Apr 2, 2008 at 1:55 PM, Paul E. McKenney ... processes but with a shared io context? ... It should, SLAB_DESTROY_BY_RCU is definitely useful, but it is expected ...
    (Linux-Kernel)
  • Re: kmemcheck caught read from freed memory (cfq_free_io_context)
    ... On Wed, Apr 2, 2008 at 1:55 PM, Paul E. McKenney ... processes but with a shared io context? ... The only place where you'll have multiple processes involved with this ... There are 3 races here: ...
    (Linux-Kernel)
  • Re: A critique of the BBC aquatic ape programme and the transcript.
    ... Good point Paul. ... but in the context of a new fauna along with a lot of new species. ... >> process of hominid bipedalism. ... Did you know that chimps are bipedal 100% of the time when they are ...
    (sci.anthropology.paleo)
  • Re: Sixth grade science teaching
    ... >>Neither Grace, nor I, am arguing that memorization is not necessary, ... > Nor are Paul are I suggesting that mathematics _ends_ with getting ... >>context, without relationship to eachother, once you forget, which you ... > as exemplified by Grace's calculus example. ...
    (talk.origins)
  • Re: L
    ... context: in C for instance, NULL has a completely different meaning. ... Since it has a different meaning in C, there is no point bringing C into ... Hi Paul, ... Much of the discussion between Cimode and Dawn appears (as I read ...
    (comp.databases.theory)