From patchwork Thu Jul 6 17:57:48 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Eric Biggers X-Patchwork-Id: 9828695 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id B358A602CA for ; Thu, 6 Jul 2017 17:59:39 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 9D1582862A for ; Thu, 6 Jul 2017 17:59:39 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 9148328650; Thu, 6 Jul 2017 17:59:39 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.3 required=2.0 tests=BAYES_00, DKIM_ADSP_CUSTOM_MED, DKIM_SIGNED, FREEMAIL_FROM, RCVD_IN_DNSWL_HI, RCVD_IN_SORBS_SPAM, T_DKIM_INVALID autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 356562862A for ; Thu, 6 Jul 2017 17:59:39 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751765AbdGFR7i (ORCPT ); Thu, 6 Jul 2017 13:59:38 -0400 Received: from mail-pg0-f66.google.com ([74.125.83.66]:36822 "EHLO mail-pg0-f66.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750778AbdGFR7h (ORCPT ); Thu, 6 Jul 2017 13:59:37 -0400 Received: by mail-pg0-f66.google.com with SMTP id u36so1060569pgn.3; Thu, 06 Jul 2017 10:59:37 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:cc:subject:date:message-id; bh=8dKZm3KzSAEVD+mkVw0xw5ku8GN7wTxEsCI8MS4a9tc=; b=Pya9skohi2PNEz908M3548FntqkQrcXT9xn+qhEPuBLSaL6z4kLpN8TRV3SXKRc1e0 m/tUrgLjB1gi5iDvqo3M0J2aiElq3VloxUNo0bQnxjM7Qvx4m/6COZkVJ/djr/ERyu3V 2XFFsv/veYi3KXuEr2kbYVa6YAccYRX3nakwjKp8fxLohSKQDqiv7pYTHGbg7ubbDedr tyjszT3eEEiOXLMLM/eErjm6RI0qik0DGc+P7Q3KuaTdsYnkG57szEhDddghtquGshRu R1Z4du6aPVzlR/bHPf5BY0A2vOrmWSKirnlaOz5CGPwZSZcE4d36pyq17vU+ai5LMtdy Me0w== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id; bh=8dKZm3KzSAEVD+mkVw0xw5ku8GN7wTxEsCI8MS4a9tc=; b=ueT/J+gyuSWz+VwDG78LVukZ8JZUasyCX2RSQMu1ozOQEr9pn1yc5EtkXvXFFeuKHW 3hSk5hiY0QvbIj3ZosXACshiztPvYQt+y4WEz0som7ihrXLjGmgLMarRq4jOHzETxj7a Yp+dnL8BsbQ+skQGWV55Jh0sImbbQ6fj0tI8aqrQrGqjV4hs28itWBPWU3sB2ubttKZu D3D7joCrgR4hap/9vOZjnOuvPdHut70PJUTBHGntSY8x0wTK1gIWQl8n67JwZXmOTZlo wG3mVTmEubJttjM3XZzAp6SlLhsJOk0zjmVcLLVaYX39wsMj5hSf41SRhoGnoZ4Fvw8R 4exg== X-Gm-Message-State: AIVw113VNi+2DaKl/tg6qwEm/SsrlFB1+WR5ReFAiWm/ggMTf4Uuw/j2 fcw3eS+NVRsDhmDDde4= X-Received: by 10.84.229.7 with SMTP id b7mr29946888plk.216.1499363976526; Thu, 06 Jul 2017 10:59:36 -0700 (PDT) Received: from ebiggers-linuxstation.kir.corp.google.com ([100.66.174.81]) by smtp.gmail.com with ESMTPSA id x64sm1831246pfk.20.2017.07.06.10.59.35 (version=TLS1_2 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Thu, 06 Jul 2017 10:59:36 -0700 (PDT) From: Eric Biggers To: linux-fscrypt@vger.kernel.org Cc: "Theodore Y . Ts'o" , Jaegeuk Kim , linux-kernel@vger.kernel.org, stable@vger.kernel.org, Eric Biggers Subject: [PATCH] fscrypt: lock mutex before checking for bounce page pool Date: Thu, 6 Jul 2017 10:57:48 -0700 Message-Id: <20170706175748.33093-1-ebiggers3@gmail.com> X-Mailer: git-send-email 2.13.2.725.g09c95d1e9-goog Sender: linux-fscrypt-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-fscrypt@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP From: Eric Biggers 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: stable@vger.kernel.org # v4.1+ Signed-off-by: Eric Biggers --- fs/crypto/crypto.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/fs/crypto/crypto.c b/fs/crypto/crypto.c index c7835df7e7b8..d262a93d9b31 100644 --- a/fs/crypto/crypto.c +++ b/fs/crypto/crypto.c @@ -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);