On Fri, Jun 19, 2009 at 04:14:23PM +0800, Li Zefan wrote:
> > rcu_read_lock();
> > if (devcache[i] && devcache[i]->device == device) {
> > ret = devcache[i]->devname;
> > rcu_read_unlock();
> > return ret;
>
> It doesn't seem safe to dereference @ret outside rcu read section.
Note the comments at the beginning of the function:
The caller should use rcu_read_lock() in order to make sure the
device name stays valid until its done with it. We use
rcu_read_lock() as well to make sure we're safe in case the caller
gets sloppy, and because rcu_read_lock() is cheap and can be safely
nested.
I suppose I should change the wording to indicate that it adds a bit
more safety (as in, the crash won't happen inside this function, but
as far as the caller is concerned, all bets are off!)
> > spin_lock(&devname_cache_lock);
> > if (devcache[i]) {
> > if (devcache[i]->device == device) {
> > ret = devcache[i]->devname;
> > spin_unlock(&devname_cache_lock);
> > return ret;
> > }
> > call_rcu(&devcache[i]->rcu, free_devcache);
> > }
> > devcache[i] = kmalloc(sizeof(struct devname_cache), GFP_KERNEL);
>
> kmalloc(GFP_KERNEL) called with spin_lock held..
Good catch, thanks. I'll get a patch in to fix this.
- Ted
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
