[Q] eCryptFS race window?

[Q] eCryptFS race window?

[Cyrill Gorcunov]

Hi Michael,

it seems there is a few potential race window in eCryptFS which I was trying
to fix but it requires more deeper eCrypFS knowledge that have (at least only
by understanding eCryptFS in big picture it is possible to fix this problem
by elegant path). So what is the problem - the procedures

	ecryptfs_miscdev_poll
	ecryptfs_miscdev_read

does take ecryptfs_daemon_hash_mux mutex and then daemon->mux _but_ releases
them not in exactly backward order as it should. My patches (not in mainline
but you saw them) was screwed up 'cause mutex_is_locked could release mutex
acquired by another process and that is wrong. But I've a hope that I'm simply
*wrong* about this possible races ;) Take a look please.

		- Cyrill -

Re: [Q] eCryptFS race window?

[Michael Halcrow]

On Tue, May 20, 2008 at 09:03:21PM +0400, Cyrill Gorcunov wrote:
> it seems there is a few potential race window in eCryptFS which I
> was trying to fix but it requires more deeper eCrypFS knowledge that
> have (at least only by understanding eCryptFS in big picture it is
> possible to fix this problem by elegant path). So what is the
> problem - the procedures
> 
> 	ecryptfs_miscdev_poll
> 	ecryptfs_miscdev_read
> 
> does take ecryptfs_daemon_hash_mux mutex and then daemon->mux _but_
> releases them not in exactly backward order as it should. My patches
> (not in mainline but you saw them) was screwed up 'cause
> mutex_is_locked could release mutex acquired by another process and
> that is wrong. But I've a hope that I'm simply *wrong* about this
> possible races ;) Take a look please.

I cannot find an execution path whereby one of the two must re-acquire
ecryptfs_daemon_hash_mux in order to release daemon->mux, so I do not
think we will ever have deadlock between those two functions.

Mike

Re: [Q] eCryptFS race window?

[Cyrill Gorcunov]

[Michael Halcrow - Tue, May 20, 2008 at 01:27:57PM -0500]
| On Tue, May 20, 2008 at 09:03:21PM +0400, Cyrill Gorcunov wrote:
| > it seems there is a few potential race window in eCryptFS which I
| > was trying to fix but it requires more deeper eCrypFS knowledge that
| > have (at least only by understanding eCryptFS in big picture it is
| > possible to fix this problem by elegant path). So what is the
| > problem - the procedures
| > 
| > 	ecryptfs_miscdev_poll
| > 	ecryptfs_miscdev_read
| > 
| > does take ecryptfs_daemon_hash_mux mutex and then daemon->mux _but_
| > releases them not in exactly backward order as it should. My patches
| > (not in mainline but you saw them) was screwed up 'cause
| > mutex_is_locked could release mutex acquired by another process and
| > that is wrong. But I've a hope that I'm simply *wrong* about this
| > possible races ;) Take a look please.
| 
| I cannot find an execution path whereby one of the two must re-acquire
| ecryptfs_daemon_hash_mux in order to release daemon->mux, so I do not
| think we will ever have deadlock between those two functions.
| 
| Mike
| 

ok, that means I'm just wrong, thanks

		- Cyrill -

Re: [Q] eCryptFS race window?

[Cyrill Gorcunov]

[Michael Halcrow - Tue, May 20, 2008 at 01:27:57PM -0500]
| On Tue, May 20, 2008 at 09:03:21PM +0400, Cyrill Gorcunov wrote:
| > it seems there is a few potential race window in eCryptFS which I
| > was trying to fix but it requires more deeper eCrypFS knowledge that
| > have (at least only by understanding eCryptFS in big picture it is
| > possible to fix this problem by elegant path). So what is the
| > problem - the procedures
| > 
| > 	ecryptfs_miscdev_poll
| > 	ecryptfs_miscdev_read
| > 
| > does take ecryptfs_daemon_hash_mux mutex and then daemon->mux _but_
| > releases them not in exactly backward order as it should. My patches
| > (not in mainline but you saw them) was screwed up 'cause
| > mutex_is_locked could release mutex acquired by another process and
| > that is wrong. But I've a hope that I'm simply *wrong* about this
| > possible races ;) Take a look please.
| 
| I cannot find an execution path whereby one of the two must re-acquire
| ecryptfs_daemon_hash_mux in order to release daemon->mux, so I do not
| think we will ever have deadlock between those two functions.
| 
| Mike
| 

Anyway, don't apply my patch you've tested (which I've replied to
Ingo's circular dep report), please, it's screwed ;) The patch Andrew
has taken to its tree is ok.

		- Cyrill -