All of lore.kernel.org
 help / color / mirror / Atom feed
From: David Howells <dhowells@redhat.com>
To: torvalds@linux-foundation.org
Cc: dhowells@redhat.com, jarkko.sakkinen@linux.intel.com,
	longman@redhat.com, keyrings@vger.kernel.org,
	linux-security-module@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: [GIT PULL] keys: Fix key->sem vs mmap_sem issue when reading key
Date: Mon, 30 Mar 2020 12:16:38 +0000	[thread overview]
Message-ID: <1437197.1585570598@warthog.procyon.org.uk> (raw)

Hi Linus,

Here's a couple of patches that fix a circular dependency between holding
key->sem and mm->mmap_sem when reading data from a key.  One potential
issue is that a filesystem looking to use a key inside, say, ->readpages()
could deadlock if the key being read is the key that's required and the
buffer the key is being read into is on a page that needs to be fetched.

The case actually detected is a bit more involved - with a filesystem
calling request_key() and locking the target keyring for write - which
could be being read.

[Note: kbuild spotted a compiler(?) warning that I've not seen before,
 complaining "The scope of the variable 'oldxdr' can be reduced.
 [variableScope]".  It's unhappy that a variable that's declared at the top
 of the function hasn't been moved into an interior for-loop.  Is this
 something we're now requiring?  Anyway, I'd prefer to fix that with a
 follow up patch through the net tree rather than go for a 9th iteration on
 these patches.]

Thanks,
David
---
The following changes since commit 1b649e0bcae71c118c1333e02249a7510ba7f70a:

  Merge git://git.kernel.org/pub/scm/linux/kernel/git/netdev/net (2020-03-25 13:58:05 -0700)

are available in the Git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/dhowells/linux-fs.git tags/keys-fixes-20200329

for you to fetch changes up to 4f0882491a148059a52480e753b7f07fc550e188:

  KEYS: Avoid false positive ENOMEM error on key read (2020-03-29 12:40:41 +0100)

----------------------------------------------------------------
Keyrings fixes

----------------------------------------------------------------
Waiman Long (2):
      KEYS: Don't write out to userspace while holding key semaphore
      KEYS: Avoid false positive ENOMEM error on key read

 include/keys/big_key-type.h               |   2 +-
 include/keys/user-type.h                  |   3 +-
 include/linux/key-type.h                  |   2 +-
 net/dns_resolver/dns_key.c                |   2 +-
 net/rxrpc/key.c                           |  27 +++-----
 security/keys/big_key.c                   |  11 ++--
 security/keys/encrypted-keys/encrypted.c  |   7 +-
 security/keys/internal.h                  |  12 ++++
 security/keys/keyctl.c                    | 103 +++++++++++++++++++++++++-----
 security/keys/keyring.c                   |   6 +-
 security/keys/request_key_auth.c          |   7 +-
 security/keys/trusted-keys/trusted_tpm1.c |  14 +---
 security/keys/user_defined.c              |   5 +-
 13 files changed, 126 insertions(+), 75 deletions(-)

WARNING: multiple messages have this Message-ID (diff)
From: David Howells <dhowells@redhat.com>
To: torvalds@linux-foundation.org
Cc: dhowells@redhat.com, jarkko.sakkinen@linux.intel.com,
	longman@redhat.com, keyrings@vger.kernel.org,
	linux-security-module@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: [GIT PULL] keys: Fix key->sem vs mmap_sem issue when reading key
Date: Mon, 30 Mar 2020 13:16:38 +0100	[thread overview]
Message-ID: <1437197.1585570598@warthog.procyon.org.uk> (raw)

Hi Linus,

Here's a couple of patches that fix a circular dependency between holding
key->sem and mm->mmap_sem when reading data from a key.  One potential
issue is that a filesystem looking to use a key inside, say, ->readpages()
could deadlock if the key being read is the key that's required and the
buffer the key is being read into is on a page that needs to be fetched.

The case actually detected is a bit more involved - with a filesystem
calling request_key() and locking the target keyring for write - which
could be being read.

[Note: kbuild spotted a compiler(?) warning that I've not seen before,
 complaining "The scope of the variable 'oldxdr' can be reduced.
 [variableScope]".  It's unhappy that a variable that's declared at the top
 of the function hasn't been moved into an interior for-loop.  Is this
 something we're now requiring?  Anyway, I'd prefer to fix that with a
 follow up patch through the net tree rather than go for a 9th iteration on
 these patches.]

Thanks,
David
---
The following changes since commit 1b649e0bcae71c118c1333e02249a7510ba7f70a:

  Merge git://git.kernel.org/pub/scm/linux/kernel/git/netdev/net (2020-03-25 13:58:05 -0700)

are available in the Git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/dhowells/linux-fs.git tags/keys-fixes-20200329

for you to fetch changes up to 4f0882491a148059a52480e753b7f07fc550e188:

  KEYS: Avoid false positive ENOMEM error on key read (2020-03-29 12:40:41 +0100)

----------------------------------------------------------------
Keyrings fixes

----------------------------------------------------------------
Waiman Long (2):
      KEYS: Don't write out to userspace while holding key semaphore
      KEYS: Avoid false positive ENOMEM error on key read

 include/keys/big_key-type.h               |   2 +-
 include/keys/user-type.h                  |   3 +-
 include/linux/key-type.h                  |   2 +-
 net/dns_resolver/dns_key.c                |   2 +-
 net/rxrpc/key.c                           |  27 +++-----
 security/keys/big_key.c                   |  11 ++--
 security/keys/encrypted-keys/encrypted.c  |   7 +-
 security/keys/internal.h                  |  12 ++++
 security/keys/keyctl.c                    | 103 +++++++++++++++++++++++++-----
 security/keys/keyring.c                   |   6 +-
 security/keys/request_key_auth.c          |   7 +-
 security/keys/trusted-keys/trusted_tpm1.c |  14 +---
 security/keys/user_defined.c              |   5 +-
 13 files changed, 126 insertions(+), 75 deletions(-)


             reply	other threads:[~2020-03-30 12:16 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-03-30 12:16 David Howells [this message]
2020-03-30 12:16 ` [GIT PULL] keys: Fix key->sem vs mmap_sem issue when reading key David Howells
2020-04-04 20:00 ` Linus Torvalds
2020-04-04 20:00   ` Linus Torvalds
2020-04-05  3:10   ` Waiman Long
2020-04-05  3:10     ` Waiman Long
2020-04-05  9:03   ` David Howells
2020-04-05  9:03     ` David Howells
2020-04-05 17:31     ` Linus Torvalds
2020-04-05 17:31       ` Linus Torvalds
2020-04-06  2:38       ` Waiman Long
2020-04-06  2:38         ` Waiman Long
2020-04-04 20:05 ` pr-tracker-bot
2020-04-04 20:05   ` pr-tracker-bot

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=1437197.1585570598@warthog.procyon.org.uk \
    --to=dhowells@redhat.com \
    --cc=jarkko.sakkinen@linux.intel.com \
    --cc=keyrings@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-security-module@vger.kernel.org \
    --cc=longman@redhat.com \
    --cc=torvalds@linux-foundation.org \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.