Skip to content

Commit

Permalink
9P lock: aquire state_lock properly
Browse files Browse the repository at this point in the history
The caller of state_lock() is expected to take the lock on its own.
This should fix the sporadical failure seen in this patchset:
https://review.gerrithub.io/#/c/385433/

==3884==ERROR: AddressSanitizer: heap-use-after-free on address 0x6110003fbf68 at pc 0x00000077cd18 bp 0x7fffc9cbf150 sp 0x7fffc9cbf148
READ of size 8 at 0x6110003fbf68 thread T34
Detaching after fork from child process 3944.
    #0 0x77cd17 in copy_conflict /export/nfs-ganesha/src/SAL/state_lock.c:2239:26
    #1 0x77fa9c in state_lock /export/nfs-ganesha/src/SAL/state_lock.c:2444:5
    #2 0x733404 in _9p_lock /export/nfs-ganesha/src/Protocols/9P/9p_lock.c:168:18
    #3 0x71c904 in _9p_process_buffer /export/nfs-ganesha/src/Protocols/9P/9p_interpreter.c:180:7
    #4 0x5fb386 in _9p_rdma_process_request /export/nfs-ganesha/src/MainNFSD/9p_rdma_callbacks.c:158:8
    #5 0x5c928f in _9p_execute /export/nfs-ganesha/src/MainNFSD/nfs_worker_thread.c:1509:3
    #6 0x5aa899 in worker_run /export/nfs-ganesha/src/MainNFSD/nfs_worker_thread.c:1604:4
    #7 0x88c1c4 in fridgethr_start_routine /export/nfs-ganesha/src/support/fridgethr.c:550:3
    #8 0x7ffff79b16c9 in start_thread (/lib64/libpthread.so.0+0x76c9)
    #9 0x7ffff4ed7f7e in __GI___clone (/lib64/libc.so.6+0x107f7e)

0x6110003fbf68 is located 104 bytes inside of 200-byte region [0x6110003fbf00,0x6110003fbfc8)
freed by thread T26 here:
    #0 0x4e2ae0 in __interceptor_cfree.localalias.1 (/export/nfs-ganesha/build/MainNFSD/ganesha.nfsd+0x4e2ae0)
    #1 0x7731e4 in gsh_free /export/nfs-ganesha/src/include/abstract_mem.h:271:2
    #2 0x7731b6 in lock_entry_dec_ref /export/nfs-ganesha/src/SAL/state_lock.c:714:3
    #3 0x77b06a in remove_from_locklist /export/nfs-ganesha/src/SAL/state_lock.c:774:2
    #4 0x78d39a in free_list /export/nfs-ganesha/src/SAL/state_lock.c:967:3
    #5 0x7848c4 in subtract_lock_from_list /export/nfs-ganesha/src/SAL/state_lock.c:1140:3
    #6 0x7833ac in state_unlock /export/nfs-ganesha/src/SAL/state_lock.c:2716:11
    #7 0x7334fe in _9p_lock /export/nfs-ganesha/src/Protocols/9P/9p_lock.c:187:7
    #8 0x71c904 in _9p_process_buffer /export/nfs-ganesha/src/Protocols/9P/9p_interpreter.c:180:7
    #9 0x5fb386 in _9p_rdma_process_request /export/nfs-ganesha/src/MainNFSD/9p_rdma_callbacks.c:158:8
    #10 0x5c928f in _9p_execute /export/nfs-ganesha/src/MainNFSD/nfs_worker_thread.c:1509:3
    #11 0x5aa899 in worker_run /export/nfs-ganesha/src/MainNFSD/nfs_worker_thread.c:1604:4
    #12 0x88c1c4 in fridgethr_start_routine /export/nfs-ganesha/src/support/fridgethr.c:550:3
    #13 0x7ffff79b16c9 in start_thread (/lib64/libpthread.so.0+0x76c9)

previously allocated by thread T16 here:
    #0 0x4e2c98 in __interceptor_malloc (/export/nfs-ganesha/build/MainNFSD/ganesha.nfsd+0x4e2c98)
    #1 0x77443f in gsh_malloc__ /export/nfs-ganesha/src/include/abstract_mem.h:78:12
    #2 0x780f13 in create_state_lock_entry /export/nfs-ganesha/src/SAL/state_lock.c:579:14
    #3 0x77ffcf in state_lock /export/nfs-ganesha/src/SAL/state_lock.c:2562:16
    #4 0x733404 in _9p_lock /export/nfs-ganesha/src/Protocols/9P/9p_lock.c:168:18
    #5 0x71c904 in _9p_process_buffer /export/nfs-ganesha/src/Protocols/9P/9p_interpreter.c:180:7
    #6 0x5fb386 in _9p_rdma_process_request /export/nfs-ganesha/src/MainNFSD/9p_rdma_callbacks.c:158:8
    #7 0x5c928f in _9p_execute /export/nfs-ganesha/src/MainNFSD/nfs_worker_thread.c:1509:3
    #8 0x5aa899 in worker_run /export/nfs-ganesha/src/MainNFSD/nfs_worker_thread.c:1604:4
    #9 0x88c1c4 in fridgethr_start_routine /export/nfs-ganesha/src/support/fridgethr.c:550:3
    #10 0x7ffff79b16c9 in start_thread (/lib64/libpthread.so.0+0x76c9)

Thanks goes to Malahal for the analysis of the problem

Change-Id: Ie82eb4a5ecf5da3fd3a8d1cd9dbdb99b54842745
Signed-off-by: Dominique Martinet <[email protected]>
  • Loading branch information
Dominique Martinet authored and ffilz committed Nov 10, 2017
1 parent 302ab52 commit 3b857f1
Showing 1 changed file with 2 additions and 1 deletion.
3 changes: 2 additions & 1 deletion src/Protocols/9P/9p_lock.c
Original file line number Diff line number Diff line change
Expand Up @@ -164,10 +164,11 @@ int _9p_lock(struct _9p_request_data *req9p, u32 *plenout, char *preply)
status = _9P_LOCK_GRACE;
break;
}

PTHREAD_RWLOCK_wrlock(&pfid->pentry->state_hdl->state_lock);
state_status = state_lock(pfid->pentry, powner, pfid->state,
STATE_NON_BLOCKING, NULL, &lock,
&holder, &conflict);
PTHREAD_RWLOCK_unlock(&pfid->pentry->state_hdl->state_lock);

if (state_status == STATE_SUCCESS)
status = _9P_LOCK_SUCCESS;
Expand Down

0 comments on commit 3b857f1

Please sign in to comment.