Return a pointer from RCU-protected list in LKM

Question

I'm a bit confused about such code:

struct a {
    // ...
    struct list_head some_list;
    // ...
};

struct e {
    struct list_head next;
    // ... 
};

static void *traverse(struct a *a)
{
    struct e *e;
    
    rcu_read_lock();
    list_for_each_entry_rcu(e, &a->some_list, next) {
        if (...) {
            rcu_read_unlock();
            return e;
        }
    }
    rcu_read_unlock();
    return NULL;
}

In the function traverse we take a lock rcu_read_lock and then iterate over some list until some condition is met, but after this condition is met we unlock rcu_read_unlock RCU and return the e pointer.

The key point that is confusing me is that we unlock RCU read-side critical section, but keep pointer from the list, what if write-side will remove this element and it seems that this pointer e will be broken, isn't it?

AFAIK, the pointer is valid only inside read-side critical section, i.e. between rcu_read_lock and rcu_read_unlock, am I wrong?

P.S.: traverse is called without holding any locks.

in typical cases, code that has a pointer to such a struct does a "get" to increase its reference count so it will not be deleted until the corresponding "put" — stark, Feb 23 '23 at 13:39
You're right, but here I don't have such a counter, so it's interesting for me is there a bug in such a code — k1r1t0, Feb 23 '23 at 13:42
Actually there isn't. This code is not from the kernel itself — k1r1t0, Feb 23 '23 at 18:34
Or even if there is I guess as @MarcoBonelli mentioned it is safe for such a scenario. — k1r1t0, Feb 23 '23 at 18:40

Marco Bonelli · Accepted Answer · 2023-02-23T15:41:49.380

Your assumptions are right, the snippet of code you posted seems "broken". What you usually want to do in such a situation is something like the following:

static void traverse(struct a *a, void (*callback)(struct e *))
{
    struct e *e;
    
    rcu_read_lock();
    list_for_each_entry_rcu(e, &a->some_list, next) {
        if (...) {
            callback(e);
            break;
        }
    }
    rcu_read_unlock();
}

This way you can ensure that whatever operation you need to perform on e, the callback() function that gets called to use it will see a consistent version of the list (of course, assuming that it does not save e somewhere to use it later, otherwise we're back at square one).

Doing return e; after rcu_read_unlock(); can cause trouble as you have noted in your question, but in theory it could still be fine depending on the exact scenario. Whether there's a problem or not only depends on what is done with e after it is returned.

For example, if e is simply checked in the caller with something like if (e != NULL) {...} then that'd be fine. Of course though, one could argue that you could have just made the traverse function return a bool in such case :')

Return a pointer from RCU-protected list in LKM

1 Answers1