On Tue, 3 Dec 2013 02:25:17 -0800 Christoph Hellwig <hch@xxxxxxxxxxxxx> wrote: > On Mon, Dec 02, 2013 at 03:26:19PM -0500, Jeff Layton wrote: > > The DRC code will attempt to reuse an existing, expired cache entry in > > preference to allocating a new one. It'll then search the cache, and if > > it gets a hit it'll then free the cache entry that it was going to > > reuse. > > > > The cache code doesn't unhash the entry that it's going to reuse > > however, so it's possible for it end up designating an entry for reuse > > and then subsequently freeing the same entry after it finds it. This > > leads it to a later use-after-free situation and usually some list > > corruption warnings or an oops. > > > > Fix this by simply unhashing the entry that we intend to reuse. That > > will mean that it's not findable via a search and should prevent this > > situation from occurring. > > The fix looks reasonable to me, > > Reviewed-by: Christoph Hellwig <hch@xxxxxx> > Thanks. > Btw, it seems like this code would benefit from being converted to > the list_lru structure. Maybe. Most of this code is protected by a single spinlock (cache_lock). The main benefit to switching to list_lru would be that we could move to a per-node lock. But, to make that worthwhile would mean we'd need to redesign the locking and break the cache_lock into multiple locks. Also, the existing code does take pains to reuse an expired entry off the LRU list in preference to allocating a new one. The list_lru code doesn't have a mechanism to scrape the first entry off the LRU list, though I suppose we could add one or abuse the cb_arg in the walk callback as a return pointer. I'm not sure the resulting code would be any clearer however... Maybe when we get to the point that the cache_lock shows up as a bottleneck, it'll make more sense? -- Jeff Layton <jlayton@xxxxxxxxxx> -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
