Mercurial > illumos > illumos-gate
changeset 2724:99be9a0b86f6
6469119 race between arc_buf_clone() and arc_buf_add_ref() results in NULL pointer dereference
author | maybee |
---|---|
date | Wed, 13 Sep 2006 18:42:50 -0700 |
parents | 200331b43252 |
children | d08730c348e9 |
files | usr/src/uts/common/fs/zfs/arc.c |
diffstat | 1 files changed, 43 insertions(+), 55 deletions(-) [+] |
line wrap: on
line diff
--- a/usr/src/uts/common/fs/zfs/arc.c Wed Sep 13 15:20:10 2006 -0700 +++ b/usr/src/uts/common/fs/zfs/arc.c Wed Sep 13 18:42:50 2006 -0700 @@ -722,31 +722,30 @@ void arc_buf_add_ref(arc_buf_t *buf, void* tag) { - arc_buf_hdr_t *hdr; + arc_buf_hdr_t *hdr = buf->b_hdr; kmutex_t *hash_lock; - mutex_enter(&arc_eviction_mtx); - hdr = buf->b_hdr; + /* + * Check to see if this buffer is currently being evicted via + * arc_do_user_evicts(). We can do this without holding any + * locks because if we happen to obtain the header before its + * cleared, we will find b_data is NULL later. + */ + if (hdr == NULL) + return; + + hash_lock = HDR_LOCK(hdr); + mutex_enter(hash_lock); if (buf->b_data == NULL) { /* * This buffer is evicted. */ - mutex_exit(&arc_eviction_mtx); + mutex_exit(hash_lock); return; - } else { - /* - * Prevent this buffer from being evicted - * while we add a reference. - */ - buf->b_hdr = NULL; } - mutex_exit(&arc_eviction_mtx); - ASSERT(hdr->b_state != arc.anon); - hash_lock = HDR_LOCK(hdr); - mutex_enter(hash_lock); - ASSERT(!GHOST_STATE(hdr->b_state)); - buf->b_hdr = hdr; + ASSERT(buf->b_hdr == hdr); + ASSERT(hdr->b_state == arc.mru || hdr->b_state == arc.mfu); add_reference(hdr, hash_lock, tag); arc_access(hdr, hash_lock); mutex_exit(hash_lock); @@ -961,19 +960,6 @@ } if (buf->b_efunc) { mutex_enter(&arc_eviction_mtx); - /* - * arc_buf_add_ref() could derail - * this eviction. - */ - if (buf->b_hdr == NULL) { - mutex_exit(&arc_eviction_mtx); - bytes_evicted -= ab->b_size; - if (recycle) - steal = NULL; - if (!have_lock) - mutex_exit(hash_lock); - goto derailed; - } arc_buf_destroy(buf, recycle, FALSE); ab->b_buf = buf->b_next; buf->b_next = arc_eviction_list; @@ -995,8 +981,6 @@ } else { missed += 1; } -derailed: - /* null statement */; } mutex_exit(&evicted_state->mtx); mutex_exit(&state->mtx); @@ -1998,44 +1982,48 @@ int arc_buf_evict(arc_buf_t *buf) { - arc_buf_hdr_t *hdr; + arc_buf_hdr_t *hdr = buf->b_hdr; kmutex_t *hash_lock; arc_buf_t **bufp; - mutex_enter(&arc_eviction_mtx); - hdr = buf->b_hdr; if (hdr == NULL) { /* * We are in arc_do_user_evicts(). - * NOTE: We can't be in arc_buf_add_ref() because - * that would violate the interface rules. */ ASSERT(buf->b_data == NULL); - mutex_exit(&arc_eviction_mtx); return (0); - } else if (buf->b_data == NULL) { - arc_buf_t copy = *buf; /* structure assignment */ - /* - * We are on the eviction list. Process this buffer - * now but let arc_do_user_evicts() do the reaping. - */ - buf->b_efunc = NULL; - buf->b_hdr = NULL; - mutex_exit(&arc_eviction_mtx); - VERIFY(copy.b_efunc(©) == 0); - return (1); - } else { - /* - * Prevent a race with arc_evict() - */ - ASSERT3U(refcount_count(&hdr->b_refcnt), <, hdr->b_datacnt); - buf->b_hdr = NULL; } - mutex_exit(&arc_eviction_mtx); hash_lock = HDR_LOCK(hdr); mutex_enter(hash_lock); + if (buf->b_data == NULL) { + /* + * We are on the eviction list. + */ + mutex_exit(hash_lock); + mutex_enter(&arc_eviction_mtx); + if (buf->b_hdr == NULL) { + /* + * We are already in arc_do_user_evicts(). + */ + mutex_exit(&arc_eviction_mtx); + return (0); + } else { + arc_buf_t copy = *buf; /* structure assignment */ + /* + * Process this buffer now + * but let arc_do_user_evicts() do the reaping. + */ + buf->b_efunc = NULL; + mutex_exit(&arc_eviction_mtx); + VERIFY(copy.b_efunc(©) == 0); + return (1); + } + } + + ASSERT(buf->b_hdr == hdr); + ASSERT3U(refcount_count(&hdr->b_refcnt), <, hdr->b_datacnt); ASSERT(hdr->b_state == arc.mru || hdr->b_state == arc.mfu); /*