Skip to content

Commit

Permalink
fscrypt: lock mutex before checking for bounce page pool
Browse files Browse the repository at this point in the history
fscrypt_initialize(), which allocates the global bounce page pool when
an encrypted file is first accessed, uses "double-checked locking" to
try to avoid locking fscrypt_init_mutex.  However, it doesn't use any
memory barriers, so it's theoretically possible for a thread to observe
a bounce page pool which has not been fully initialized.  This is a
classic bug with "double-checked locking".

While "only a theoretical issue" in the latest kernel, in pre-4.8
kernels the pointer that was checked was not even the last to be
initialized, so it was easily possible for a crash (NULL pointer
dereference) to happen.  This was changed only incidentally by the large
refactor to use fs/crypto/.

Solve both problems in a trivial way that can easily be backported: just
always take the mutex.  It's theoretically less efficient, but it
shouldn't be noticeable in practice as the mutex is only acquired very
briefly once per encrypted file.

Later I'd like to make this use a helper macro like DO_ONCE().  However,
DO_ONCE() runs in atomic context, so we'd need to add a new macro that
allows blocking.

Cc: [email protected] # v4.1+
Signed-off-by: Eric Biggers <[email protected]>
Signed-off-by: Theodore Ts'o <[email protected]>
  • Loading branch information
ebiggers authored and tytso committed Oct 31, 2017
1 parent f4f864c commit a0b3bc8
Showing 1 changed file with 2 additions and 5 deletions.
7 changes: 2 additions & 5 deletions fs/crypto/crypto.c
Original file line number Diff line number Diff line change
Expand Up @@ -410,11 +410,8 @@ int fscrypt_initialize(unsigned int cop_flags)
{
int i, res = -ENOMEM;

/*
* No need to allocate a bounce page pool if there already is one or
* this FS won't use it.
*/
if (cop_flags & FS_CFLG_OWN_PAGES || fscrypt_bounce_page_pool)
/* No need to allocate a bounce page pool if this FS won't use it. */
if (cop_flags & FS_CFLG_OWN_PAGES)
return 0;

mutex_lock(&fscrypt_init_mutex);
Expand Down

0 comments on commit a0b3bc8

Please sign in to comment.