On Mon, Apr 06 2020, Yihao Wu wrote: > Deleting list entry within hlist_for_each_entry_safe is not safe unless > next pointer (tmp) is protected too. It's not, because once hash_lock > is released, cache_clean may delete the entry that tmp points to. Then > cache_purge can walk to a deleted entry and tries to double free it. > > Fix this bug by holding only the deleted entry's reference. > > Signed-off-by: Yihao Wu <wuyihao@xxxxxxxxxxxxxxxxx> > --- > v1->v2: Use Neil's better solution > v2->v3: Fix a checkscript warning > > net/sunrpc/cache.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/net/sunrpc/cache.c b/net/sunrpc/cache.c > index af0ddd28b081..b445874e8e2f 100644 > --- a/net/sunrpc/cache.c > +++ b/net/sunrpc/cache.c > @@ -541,7 +541,9 @@ void cache_purge(struct cache_detail *detail) > dprintk("RPC: %d entries in %s cache\n", detail->entries, detail->name); > for (i = 0; i < detail->hash_size; i++) { > head = &detail->hash_table[i]; > - hlist_for_each_entry_safe(ch, tmp, head, cache_list) { > + while (!hlist_empty(head)) { > + ch = hlist_entry(head->first, struct cache_head, > + cache_list); > sunrpc_begin_cache_remove_entry(ch, detail); > spin_unlock(&detail->hash_lock); > sunrpc_end_cache_remove_entry(ch, detail); > -- > 2.20.1.2432.ga663e714 Reviewed-by: NeilBrown <neilb@xxxxxxx> Thanks for finding the bug and testing the solution! NeilBrown
Attachment:
signature.asc
Description: PGP signature