Linux FSCRYPT development
 help / color / mirror / Atom feed
From: Eric Biggers <ebiggers3@gmail.com>
To: linux-fscrypt@vger.kernel.org
Cc: "Theodore Y . Ts'o" <tytso@mit.edu>,
	Jaegeuk Kim <jaegeuk@kernel.org>,
	linux-kernel@vger.kernel.org, stable@vger.kernel.org,
	Eric Biggers <ebiggers@google.com>
Subject: Re: [PATCH] fscrypt: lock mutex before checking for bounce page pool
Date: Tue, 12 Sep 2017 22:22:52 -0700	[thread overview]
Message-ID: <20170913052252.GA16966@zzz.localdomain> (raw)
In-Reply-To: <20170706175748.33093-1-ebiggers3@gmail.com>

On Thu, Jul 06, 2017 at 10:57:48AM -0700, Eric Biggers wrote:
> 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.
> 

Ted, can you take this patch?  On Android this bug has been causing a NULL
pointer dereference in ext4_get_encryption_info on boot.  Granted, due to the
way the code has been moved around it no longer would happen in practice in the
latest kernel, but we still need something to backport to 4.4, etc.

Eric

  reply	other threads:[~2017-09-13  5:22 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-07-06 17:57 [PATCH] fscrypt: lock mutex before checking for bounce page pool Eric Biggers
2017-09-13  5:22 ` Eric Biggers [this message]
2017-10-09 20:53   ` Eric Biggers
2017-10-29 10:25 ` Theodore Ts'o

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20170913052252.GA16966@zzz.localdomain \
    --to=ebiggers3@gmail.com \
    --cc=ebiggers@google.com \
    --cc=jaegeuk@kernel.org \
    --cc=linux-fscrypt@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=stable@vger.kernel.org \
    --cc=tytso@mit.edu \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox