All of lore.kernel.org
 help / color / mirror / Atom feed
From: David Howells <dhowells@redhat.com>
To: Chris von Recklinghausen <crecklin@redhat.com>
Cc: dhowells@redhat.com,
	Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>,
	James Morris <jmorris@namei.org>,
	"Serge E . Hallyn" <serge@hallyn.com>,
	keyrings@vger.kernel.org, linux-security-module@vger.kernel.org,
	linux-kernel@vger.kernel.org, Waiman Long <longman@redhat.com>
Subject: Re: [PATCH] security/keyring: avoid pagefaults in keyring_read_iterator
Date: Mon, 21 Oct 2019 14:21:59 +0000	[thread overview]
Message-ID: <30309.1571667719@warthog.procyon.org.uk> (raw)
In-Reply-To: <20191018184030.8407-1-crecklin@redhat.com>

Chris von Recklinghausen <crecklin@redhat.com> wrote:

> The put_user call from keyring_read_iterator caused a page fault which
> attempts to lock mm->mmap_sem and type->lock_class (key->sem) in the reverse
> order that keyring_read_iterator did, thus causing the circular locking
> dependency.
> 
> Remedy this by using access_ok and __put_user instead of put_user so we'll
> return an error instead of faulting in the page.

I wonder if it's better to create a kernel buffer outside of the lock in
keyctl_read_key().  Hmmm...  The reason I didn't want to do that is that
keyrings have don't have limits on the size.  Maybe that's not actually a
problem, since 1MiB would be able to hold a list of a quarter of a million
keys.

David

WARNING: multiple messages have this Message-ID (diff)
From: David Howells <dhowells@redhat.com>
To: Chris von Recklinghausen <crecklin@redhat.com>
Cc: dhowells@redhat.com,
	Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>,
	James Morris <jmorris@namei.org>,
	"Serge E . Hallyn" <serge@hallyn.com>,
	keyrings@vger.kernel.org, linux-security-module@vger.kernel.org,
	linux-kernel@vger.kernel.org, Waiman Long <longman@redhat.com>
Subject: Re: [PATCH] security/keyring: avoid pagefaults in keyring_read_iterator
Date: Mon, 21 Oct 2019 15:21:59 +0100	[thread overview]
Message-ID: <30309.1571667719@warthog.procyon.org.uk> (raw)
In-Reply-To: <20191018184030.8407-1-crecklin@redhat.com>

Chris von Recklinghausen <crecklin@redhat.com> wrote:

> The put_user call from keyring_read_iterator caused a page fault which
> attempts to lock mm->mmap_sem and type->lock_class (key->sem) in the reverse
> order that keyring_read_iterator did, thus causing the circular locking
> dependency.
> 
> Remedy this by using access_ok and __put_user instead of put_user so we'll
> return an error instead of faulting in the page.

I wonder if it's better to create a kernel buffer outside of the lock in
keyctl_read_key().  Hmmm...  The reason I didn't want to do that is that
keyrings have don't have limits on the size.  Maybe that's not actually a
problem, since 1MiB would be able to hold a list of a quarter of a million
keys.

David


  reply	other threads:[~2019-10-21 14:21 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-10-18 18:40 [PATCH] security/keyring: avoid pagefaults in keyring_read_iterator Chris von Recklinghausen
2019-10-18 18:40 ` Chris von Recklinghausen
2019-10-21 14:21 ` David Howells [this message]
2019-10-21 14:21   ` David Howells
2019-10-21 15:46   ` Chris von Recklinghausen
2019-10-21 15:46     ` Chris von Recklinghausen
2019-10-25 11:10     ` Chris von Recklinghausen
2019-10-25 11:10       ` Chris von Recklinghausen
2019-11-06 15:25       ` Chris von Recklinghausen
2019-11-06 15:25         ` Chris von Recklinghausen
2019-10-21 15:47 ` Jarkko Sakkinen
2019-10-21 15:47   ` Jarkko Sakkinen

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=30309.1571667719@warthog.procyon.org.uk \
    --to=dhowells@redhat.com \
    --cc=crecklin@redhat.com \
    --cc=jarkko.sakkinen@linux.intel.com \
    --cc=jmorris@namei.org \
    --cc=keyrings@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-security-module@vger.kernel.org \
    --cc=longman@redhat.com \
    --cc=serge@hallyn.com \
    /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.