Hi,
one student (in CC) found out a suspicioous locking dependence in
ecryptfs code while debugging/running a static lockdep analyzer.
I'm unable to say whether it is only a theoretical issue or a real bug,
any ideas?
Here it comes:
function ecryptfs_send_message:
------------------------------
ecryptfs_daemon_hash_mux <- ecryptfs_msg_ctx_lists_mux
(in function ecryptfs_send_message_locked)
function ecryptfs_wait_for_response:
-----------------------------------
cryptfs_msg_ctx_lists_mux <- msg_ctx->mux
function ecryptfs_process_response:
----------------------------------
msg_ctx->mux <- ecryptfs_daemon_hash_mux
<- means lock dependency
Jiri Slaby wrote:
> Hi,
>
> one student (in CC) found out a suspicioous locking dependence in
> ecryptfs code while debugging/running a static lockdep analyzer.
>
> I'm unable to say whether it is only a theoretical issue or a real bug,
> any ideas?
Hi Jiri - This looks to be a real bug. Thanks for pointing it out!
>
> Here it comes:
>
> function ecryptfs_send_message:
> ------------------------------
> ecryptfs_daemon_hash_mux <- ecryptfs_msg_ctx_lists_mux
> (in function ecryptfs_send_message_locked)
It would be nice if we could keep from having to hold the
ecryptfs_daemon_hash_mux for the entire execution of
ecryptfs_send_message_locked(). Maybe we could just hold it in
ecryptfs_send_message(), while it calls ecryptfs_find_daemon_by_euid().
I think we would need to grab the daemon->mux before releasing the
ecryptfs_daemon_hash_mux to make sure that a QUIT message wasn't
received and the daemon freed before we add the msg_ctx to the
daemon->msg_ctx_out_queue in ecryptfs_send_miscdev().
It would remove the first lock dependency you pointed out, but I'll need
some more time to determine if it creates another bad lock loop. Thanks
again!
Tyler
>
> function ecryptfs_wait_for_response:
> -----------------------------------
> cryptfs_msg_ctx_lists_mux <- msg_ctx->mux
>
> function ecryptfs_process_response:
> ----------------------------------
> msg_ctx->mux <- ecryptfs_daemon_hash_mux
>
>
> <- means lock dependency