<feed xmlns='http://www.w3.org/2005/Atom'>
<title>linux-stable.git/security, branch linux-4.7.y</title>
<subtitle>Linux kernel stable tree</subtitle>
<link rel='alternate' type='text/html' href='https://git.tavy.me/linux-stable.git/'/>
<entry>
<title>ima: use file_dentry()</title>
<updated>2016-10-16T15:50:38+00:00</updated>
<author>
<name>Miklos Szeredi</name>
<email>mszeredi@redhat.com</email>
</author>
<published>2016-09-16T10:44:20+00:00</published>
<link rel='alternate' type='text/html' href='https://git.tavy.me/linux-stable.git/commit/?id=889995959eb082d515effe001e51bbf2f7324059'/>
<id>889995959eb082d515effe001e51bbf2f7324059</id>
<content type='text'>
commit e71b9dff0634edb127f449e076e883ef24a8c76c upstream.

Ima tries to call -&gt;setxattr() on overlayfs dentry after having locked
underlying inode, which results in a deadlock.

Reported-by: Krisztian Litkey &lt;kli@iki.fi&gt;
Fixes: 4bacc9c9234c ("overlayfs: Make f_path always point to the overlay and f_inode to the underlay")
Signed-off-by: Miklos Szeredi &lt;mszeredi@redhat.com&gt;
Cc: Mimi Zohar &lt;zohar@linux.vnet.ibm.com&gt;
Signed-off-by: Greg Kroah-Hartman &lt;gregkh@linuxfoundation.org&gt;

</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
commit e71b9dff0634edb127f449e076e883ef24a8c76c upstream.

Ima tries to call -&gt;setxattr() on overlayfs dentry after having locked
underlying inode, which results in a deadlock.

Reported-by: Krisztian Litkey &lt;kli@iki.fi&gt;
Fixes: 4bacc9c9234c ("overlayfs: Make f_path always point to the overlay and f_inode to the underlay")
Signed-off-by: Miklos Szeredi &lt;mszeredi@redhat.com&gt;
Cc: Mimi Zohar &lt;zohar@linux.vnet.ibm.com&gt;
Signed-off-by: Greg Kroah-Hartman &lt;gregkh@linuxfoundation.org&gt;

</pre>
</div>
</content>
</entry>
<entry>
<title>KEYS: Fix skcipher IV clobbering</title>
<updated>2016-09-30T08:12:44+00:00</updated>
<author>
<name>Herbert Xu</name>
<email>herbert@gondor.apana.org.au</email>
</author>
<published>2016-09-20T12:35:55+00:00</published>
<link rel='alternate' type='text/html' href='https://git.tavy.me/linux-stable.git/commit/?id=f3edd9b2367a3754e61eefb8ecd457013f44385d'/>
<id>f3edd9b2367a3754e61eefb8ecd457013f44385d</id>
<content type='text'>
commit 456bee986e0a372ad4beed5d3cedb3622633d9df upstream.

The IV must not be modified by the skcipher operation so we need
to duplicate it.

Fixes: c3917fd9dfbc ("KEYS: Use skcipher")
Reported-by: Mimi Zohar &lt;zohar@linux.vnet.ibm.com&gt;
Signed-off-by: Herbert Xu &lt;herbert@gondor.apana.org.au&gt;
Signed-off-by: Greg Kroah-Hartman &lt;gregkh@linuxfoundation.org&gt;

</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
commit 456bee986e0a372ad4beed5d3cedb3622633d9df upstream.

The IV must not be modified by the skcipher operation so we need
to duplicate it.

Fixes: c3917fd9dfbc ("KEYS: Use skcipher")
Reported-by: Mimi Zohar &lt;zohar@linux.vnet.ibm.com&gt;
Signed-off-by: Herbert Xu &lt;herbert@gondor.apana.org.au&gt;
Signed-off-by: Greg Kroah-Hartman &lt;gregkh@linuxfoundation.org&gt;

</pre>
</div>
</content>
</entry>
<entry>
<title>apparmor: fix refcount race when finding a child profile</title>
<updated>2016-09-15T06:20:25+00:00</updated>
<author>
<name>John Johansen</name>
<email>john.johansen@canonical.com</email>
</author>
<published>2015-12-17T02:09:10+00:00</published>
<link rel='alternate' type='text/html' href='https://git.tavy.me/linux-stable.git/commit/?id=a913c940039ce803939bb81f3c3be3aab0d43b5c'/>
<id>a913c940039ce803939bb81f3c3be3aab0d43b5c</id>
<content type='text'>
commit de7c4cc947f9f56f61520ee7edaf380434a98c8d upstream.

When finding a child profile via an rcu critical section, the profile
may be put and scheduled for deletion after the child is found but
before its refcount is incremented.

Protect against this by repeating the lookup if the profiles refcount
is 0 and is one its way to deletion.

Signed-off-by: John Johansen &lt;john.johansen@canonical.com&gt;
Acked-by: Seth Arnold &lt;seth.arnold@canonical.com&gt;
Signed-off-by: Greg Kroah-Hartman &lt;gregkh@linuxfoundation.org&gt;

</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
commit de7c4cc947f9f56f61520ee7edaf380434a98c8d upstream.

When finding a child profile via an rcu critical section, the profile
may be put and scheduled for deletion after the child is found but
before its refcount is incremented.

Protect against this by repeating the lookup if the profiles refcount
is 0 and is one its way to deletion.

Signed-off-by: John Johansen &lt;john.johansen@canonical.com&gt;
Acked-by: Seth Arnold &lt;seth.arnold@canonical.com&gt;
Signed-off-by: Greg Kroah-Hartman &lt;gregkh@linuxfoundation.org&gt;

</pre>
</div>
</content>
</entry>
<entry>
<title>apparmor: fix ref count leak when profile sha1 hash is read</title>
<updated>2016-08-16T07:34:58+00:00</updated>
<author>
<name>John Johansen</name>
<email>john.johansen@canonical.com</email>
</author>
<published>2015-11-18T19:41:05+00:00</published>
<link rel='alternate' type='text/html' href='https://git.tavy.me/linux-stable.git/commit/?id=3f0153ace2051b3f62c24503740b935e82451c22'/>
<id>3f0153ace2051b3f62c24503740b935e82451c22</id>
<content type='text'>
commit 0b938a2e2cf0b0a2c8bac9769111545aff0fee97 upstream.

Signed-off-by: John Johansen &lt;john.johansen@canonical.com&gt;
Acked-by: Seth Arnold &lt;seth.arnold@canonical.com&gt;
Signed-off-by: Greg Kroah-Hartman &lt;gregkh@linuxfoundation.org&gt;

</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
commit 0b938a2e2cf0b0a2c8bac9769111545aff0fee97 upstream.

Signed-off-by: John Johansen &lt;john.johansen@canonical.com&gt;
Acked-by: Seth Arnold &lt;seth.arnold@canonical.com&gt;
Signed-off-by: Greg Kroah-Hartman &lt;gregkh@linuxfoundation.org&gt;

</pre>
</div>
</content>
</entry>
<entry>
<title>apparmor: fix oops, validate buffer size in apparmor_setprocattr()</title>
<updated>2016-07-08T00:26:25+00:00</updated>
<author>
<name>Vegard Nossum</name>
<email>vegard.nossum@oracle.com</email>
</author>
<published>2016-07-07T20:41:11+00:00</published>
<link rel='alternate' type='text/html' href='https://git.tavy.me/linux-stable.git/commit/?id=30a46a4647fd1df9cf52e43bf467f0d9265096ca'/>
<id>30a46a4647fd1df9cf52e43bf467f0d9265096ca</id>
<content type='text'>
When proc_pid_attr_write() was changed to use memdup_user apparmor's
(interface violating) assumption that the setprocattr buffer was always
a single page was violated.

The size test is not strictly speaking needed as proc_pid_attr_write()
will reject anything larger, but for the sake of robustness we can keep
it in.

SMACK and SELinux look safe to me, but somebody else should probably
have a look just in case.

Based on original patch from Vegard Nossum &lt;vegard.nossum@oracle.com&gt;
modified for the case that apparmor provides null termination.

Fixes: bb646cdb12e75d82258c2f2e7746d5952d3e321a
Reported-by: Vegard Nossum &lt;vegard.nossum@oracle.com&gt;
Cc: Al Viro &lt;viro@zeniv.linux.org.uk&gt;
Cc: John Johansen &lt;john.johansen@canonical.com&gt;
Cc: Paul Moore &lt;paul@paul-moore.com&gt;
Cc: Stephen Smalley &lt;sds@tycho.nsa.gov&gt;
Cc: Eric Paris &lt;eparis@parisplace.org&gt;
Cc: Casey Schaufler &lt;casey@schaufler-ca.com&gt;
Cc: stable@kernel.org
Signed-off-by: John Johansen &lt;john.johansen@canonical.com&gt;
Reviewed-by: Tyler Hicks &lt;tyhicks@canonical.com&gt;
Signed-off-by: James Morris &lt;james.l.morris@oracle.com&gt;
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
When proc_pid_attr_write() was changed to use memdup_user apparmor's
(interface violating) assumption that the setprocattr buffer was always
a single page was violated.

The size test is not strictly speaking needed as proc_pid_attr_write()
will reject anything larger, but for the sake of robustness we can keep
it in.

SMACK and SELinux look safe to me, but somebody else should probably
have a look just in case.

Based on original patch from Vegard Nossum &lt;vegard.nossum@oracle.com&gt;
modified for the case that apparmor provides null termination.

Fixes: bb646cdb12e75d82258c2f2e7746d5952d3e321a
Reported-by: Vegard Nossum &lt;vegard.nossum@oracle.com&gt;
Cc: Al Viro &lt;viro@zeniv.linux.org.uk&gt;
Cc: John Johansen &lt;john.johansen@canonical.com&gt;
Cc: Paul Moore &lt;paul@paul-moore.com&gt;
Cc: Stephen Smalley &lt;sds@tycho.nsa.gov&gt;
Cc: Eric Paris &lt;eparis@parisplace.org&gt;
Cc: Casey Schaufler &lt;casey@schaufler-ca.com&gt;
Cc: stable@kernel.org
Signed-off-by: John Johansen &lt;john.johansen@canonical.com&gt;
Reviewed-by: Tyler Hicks &lt;tyhicks@canonical.com&gt;
Signed-off-by: James Morris &lt;james.l.morris@oracle.com&gt;
</pre>
</div>
</content>
</entry>
<entry>
<title>KEYS: potential uninitialized variable</title>
<updated>2016-06-17T03:15:04+00:00</updated>
<author>
<name>Dan Carpenter</name>
<email>dan.carpenter@oracle.com</email>
</author>
<published>2016-06-16T14:48:57+00:00</published>
<link rel='alternate' type='text/html' href='https://git.tavy.me/linux-stable.git/commit/?id=38327424b40bcebe2de92d07312c89360ac9229a'/>
<id>38327424b40bcebe2de92d07312c89360ac9229a</id>
<content type='text'>
If __key_link_begin() failed then "edit" would be uninitialized.  I've
added a check to fix that.

This allows a random user to crash the kernel, though it's quite
difficult to achieve.  There are three ways it can be done as the user
would have to cause an error to occur in __key_link():

 (1) Cause the kernel to run out of memory.  In practice, this is difficult
     to achieve without ENOMEM cropping up elsewhere and aborting the
     attempt.

 (2) Revoke the destination keyring between the keyring ID being looked up
     and it being tested for revocation.  In practice, this is difficult to
     time correctly because the KEYCTL_REJECT function can only be used
     from the request-key upcall process.  Further, users can only make use
     of what's in /sbin/request-key.conf, though this does including a
     rejection debugging test - which means that the destination keyring
     has to be the caller's session keyring in practice.

 (3) Have just enough key quota available to create a key, a new session
     keyring for the upcall and a link in the session keyring, but not then
     sufficient quota to create a link in the nominated destination keyring
     so that it fails with EDQUOT.

The bug can be triggered using option (3) above using something like the
following:

	echo 80 &gt;/proc/sys/kernel/keys/root_maxbytes
	keyctl request2 user debug:fred negate @t

The above sets the quota to something much lower (80) to make the bug
easier to trigger, but this is dependent on the system.  Note also that
the name of the keyring created contains a random number that may be
between 1 and 10 characters in size, so may throw the test off by
changing the amount of quota used.

Assuming the failure occurs, something like the following will be seen:

	kfree_debugcheck: out of range ptr 6b6b6b6b6b6b6b68h
	------------[ cut here ]------------
	kernel BUG at ../mm/slab.c:2821!
	...
	RIP: 0010:[&lt;ffffffff811600f9&gt;] kfree_debugcheck+0x20/0x25
	RSP: 0018:ffff8804014a7de8  EFLAGS: 00010092
	RAX: 0000000000000034 RBX: 6b6b6b6b6b6b6b68 RCX: 0000000000000000
	RDX: 0000000000040001 RSI: 00000000000000f6 RDI: 0000000000000300
	RBP: ffff8804014a7df0 R08: 0000000000000001 R09: 0000000000000000
	R10: ffff8804014a7e68 R11: 0000000000000054 R12: 0000000000000202
	R13: ffffffff81318a66 R14: 0000000000000000 R15: 0000000000000001
	...
	Call Trace:
	  kfree+0xde/0x1bc
	  assoc_array_cancel_edit+0x1f/0x36
	  __key_link_end+0x55/0x63
	  key_reject_and_link+0x124/0x155
	  keyctl_reject_key+0xb6/0xe0
	  keyctl_negate_key+0x10/0x12
	  SyS_keyctl+0x9f/0xe7
	  do_syscall_64+0x63/0x13a
	  entry_SYSCALL64_slow_path+0x25/0x25

Fixes: f70e2e06196a ('KEYS: Do preallocation for __key_link()')
Signed-off-by: Dan Carpenter &lt;dan.carpenter@oracle.com&gt;
Signed-off-by: David Howells &lt;dhowells@redhat.com&gt;
cc: stable@vger.kernel.org
Signed-off-by: Linus Torvalds &lt;torvalds@linux-foundation.org&gt;
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
If __key_link_begin() failed then "edit" would be uninitialized.  I've
added a check to fix that.

This allows a random user to crash the kernel, though it's quite
difficult to achieve.  There are three ways it can be done as the user
would have to cause an error to occur in __key_link():

 (1) Cause the kernel to run out of memory.  In practice, this is difficult
     to achieve without ENOMEM cropping up elsewhere and aborting the
     attempt.

 (2) Revoke the destination keyring between the keyring ID being looked up
     and it being tested for revocation.  In practice, this is difficult to
     time correctly because the KEYCTL_REJECT function can only be used
     from the request-key upcall process.  Further, users can only make use
     of what's in /sbin/request-key.conf, though this does including a
     rejection debugging test - which means that the destination keyring
     has to be the caller's session keyring in practice.

 (3) Have just enough key quota available to create a key, a new session
     keyring for the upcall and a link in the session keyring, but not then
     sufficient quota to create a link in the nominated destination keyring
     so that it fails with EDQUOT.

The bug can be triggered using option (3) above using something like the
following:

	echo 80 &gt;/proc/sys/kernel/keys/root_maxbytes
	keyctl request2 user debug:fred negate @t

The above sets the quota to something much lower (80) to make the bug
easier to trigger, but this is dependent on the system.  Note also that
the name of the keyring created contains a random number that may be
between 1 and 10 characters in size, so may throw the test off by
changing the amount of quota used.

Assuming the failure occurs, something like the following will be seen:

	kfree_debugcheck: out of range ptr 6b6b6b6b6b6b6b68h
	------------[ cut here ]------------
	kernel BUG at ../mm/slab.c:2821!
	...
	RIP: 0010:[&lt;ffffffff811600f9&gt;] kfree_debugcheck+0x20/0x25
	RSP: 0018:ffff8804014a7de8  EFLAGS: 00010092
	RAX: 0000000000000034 RBX: 6b6b6b6b6b6b6b68 RCX: 0000000000000000
	RDX: 0000000000040001 RSI: 00000000000000f6 RDI: 0000000000000300
	RBP: ffff8804014a7df0 R08: 0000000000000001 R09: 0000000000000000
	R10: ffff8804014a7e68 R11: 0000000000000054 R12: 0000000000000202
	R13: ffffffff81318a66 R14: 0000000000000000 R15: 0000000000000001
	...
	Call Trace:
	  kfree+0xde/0x1bc
	  assoc_array_cancel_edit+0x1f/0x36
	  __key_link_end+0x55/0x63
	  key_reject_and_link+0x124/0x155
	  keyctl_reject_key+0xb6/0xe0
	  keyctl_negate_key+0x10/0x12
	  SyS_keyctl+0x9f/0xe7
	  do_syscall_64+0x63/0x13a
	  entry_SYSCALL64_slow_path+0x25/0x25

Fixes: f70e2e06196a ('KEYS: Do preallocation for __key_link()')
Signed-off-by: Dan Carpenter &lt;dan.carpenter@oracle.com&gt;
Signed-off-by: David Howells &lt;dhowells@redhat.com&gt;
cc: stable@vger.kernel.org
Signed-off-by: Linus Torvalds &lt;torvalds@linux-foundation.org&gt;
</pre>
</div>
</content>
</entry>
<entry>
<title>KEYS: Add placeholder for KDF usage with DH</title>
<updated>2016-06-03T06:14:34+00:00</updated>
<author>
<name>Stephan Mueller</name>
<email>smueller@chronox.de</email>
</author>
<published>2016-05-26T21:38:12+00:00</published>
<link rel='alternate' type='text/html' href='https://git.tavy.me/linux-stable.git/commit/?id=4693fc734d675c5518ea9bd4c9623db45bc37402'/>
<id>4693fc734d675c5518ea9bd4c9623db45bc37402</id>
<content type='text'>
The values computed during Diffie-Hellman key exchange are often used
in combination with key derivation functions to create cryptographic
keys.  Add a placeholder for a later implementation to configure a
key derivation function that will transform the Diffie-Hellman
result returned by the KEYCTL_DH_COMPUTE command.

[This patch was stripped down from a patch produced by Mat Martineau that
 had a bug in the compat code - so for the moment Stephan's patch simply
 requires that the placeholder argument must be NULL]

Original-signed-off-by: Mat Martineau &lt;mathew.j.martineau@linux.intel.com&gt;
Signed-off-by: Stephan Mueller &lt;smueller@chronox.de&gt;
Signed-off-by: David Howells &lt;dhowells@redhat.com&gt;
Signed-off-by: James Morris &lt;james.l.morris@oracle.com&gt;
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
The values computed during Diffie-Hellman key exchange are often used
in combination with key derivation functions to create cryptographic
keys.  Add a placeholder for a later implementation to configure a
key derivation function that will transform the Diffie-Hellman
result returned by the KEYCTL_DH_COMPUTE command.

[This patch was stripped down from a patch produced by Mat Martineau that
 had a bug in the compat code - so for the moment Stephan's patch simply
 requires that the placeholder argument must be NULL]

Original-signed-off-by: Mat Martineau &lt;mathew.j.martineau@linux.intel.com&gt;
Signed-off-by: Stephan Mueller &lt;smueller@chronox.de&gt;
Signed-off-by: David Howells &lt;dhowells@redhat.com&gt;
Signed-off-by: James Morris &lt;james.l.morris@oracle.com&gt;
</pre>
</div>
</content>
</entry>
<entry>
<title>Merge branch 'for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/viro/vfs</title>
<updated>2016-05-28T00:14:05+00:00</updated>
<author>
<name>Linus Torvalds</name>
<email>torvalds@linux-foundation.org</email>
</author>
<published>2016-05-28T00:14:05+00:00</published>
<link rel='alternate' type='text/html' href='https://git.tavy.me/linux-stable.git/commit/?id=d102a56edba7a3f236454716fa09920e66772044'/>
<id>d102a56edba7a3f236454716fa09920e66772044</id>
<content type='text'>
Pull vfs fixes from Al Viro:
 "Followups to the parallel lookup work:

   - update docs

   - restore killability of the places that used to take -&gt;i_mutex
     killably now that we have down_write_killable() merged

   - Additionally, it turns out that I missed a prerequisite for
     security_d_instantiate() stuff - -&gt;getxattr() wasn't the only thing
     that could be called before dentry is attached to inode; with smack
     we needed the same treatment applied to -&gt;setxattr() as well"

* 'for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/viro/vfs:
  switch -&gt;setxattr() to passing dentry and inode separately
  switch xattr_handler-&gt;set() to passing dentry and inode separately
  restore killability of old mutex_lock_killable(&amp;inode-&gt;i_mutex) users
  add down_write_killable_nested()
  update D/f/directory-locking
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
Pull vfs fixes from Al Viro:
 "Followups to the parallel lookup work:

   - update docs

   - restore killability of the places that used to take -&gt;i_mutex
     killably now that we have down_write_killable() merged

   - Additionally, it turns out that I missed a prerequisite for
     security_d_instantiate() stuff - -&gt;getxattr() wasn't the only thing
     that could be called before dentry is attached to inode; with smack
     we needed the same treatment applied to -&gt;setxattr() as well"

* 'for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/viro/vfs:
  switch -&gt;setxattr() to passing dentry and inode separately
  switch xattr_handler-&gt;set() to passing dentry and inode separately
  restore killability of old mutex_lock_killable(&amp;inode-&gt;i_mutex) users
  add down_write_killable_nested()
  update D/f/directory-locking
</pre>
</div>
</content>
</entry>
<entry>
<title>switch -&gt;setxattr() to passing dentry and inode separately</title>
<updated>2016-05-28T00:09:16+00:00</updated>
<author>
<name>Al Viro</name>
<email>viro@zeniv.linux.org.uk</email>
</author>
<published>2016-05-27T15:06:05+00:00</published>
<link rel='alternate' type='text/html' href='https://git.tavy.me/linux-stable.git/commit/?id=3767e255b390d72f9a33c08d9e86c5f21f25860f'/>
<id>3767e255b390d72f9a33c08d9e86c5f21f25860f</id>
<content type='text'>
smack -&gt;d_instantiate() uses -&gt;setxattr(), so to be able to call it before
we'd hashed the new dentry and attached it to inode, we need -&gt;setxattr()
instances getting the inode as an explicit argument rather than obtaining
it from dentry.

Similar change for -&gt;getxattr() had been done in commit ce23e64.  Unlike
-&gt;getxattr() (which is used by both selinux and smack instances of
-&gt;d_instantiate()) -&gt;setxattr() is used only by smack one and unfortunately
it got missed back then.

Reported-by: Seung-Woo Kim &lt;sw0312.kim@samsung.com&gt;
Tested-by: Casey Schaufler &lt;casey@schaufler-ca.com&gt;
Signed-off-by: Al Viro &lt;viro@zeniv.linux.org.uk&gt;
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
smack -&gt;d_instantiate() uses -&gt;setxattr(), so to be able to call it before
we'd hashed the new dentry and attached it to inode, we need -&gt;setxattr()
instances getting the inode as an explicit argument rather than obtaining
it from dentry.

Similar change for -&gt;getxattr() had been done in commit ce23e64.  Unlike
-&gt;getxattr() (which is used by both selinux and smack instances of
-&gt;d_instantiate()) -&gt;setxattr() is used only by smack one and unfortunately
it got missed back then.

Reported-by: Seung-Woo Kim &lt;sw0312.kim@samsung.com&gt;
Tested-by: Casey Schaufler &lt;casey@schaufler-ca.com&gt;
Signed-off-by: Al Viro &lt;viro@zeniv.linux.org.uk&gt;
</pre>
</div>
</content>
</entry>
<entry>
<title>Yama: fix double-spinlock and user access in atomic context</title>
<updated>2016-05-25T23:56:18+00:00</updated>
<author>
<name>Jann Horn</name>
<email>jann@thejh.net</email>
</author>
<published>2016-05-22T04:01:34+00:00</published>
<link rel='alternate' type='text/html' href='https://git.tavy.me/linux-stable.git/commit/?id=dca6b4149181baaa363b9a7ce7c550840bb3bc83'/>
<id>dca6b4149181baaa363b9a7ce7c550840bb3bc83</id>
<content type='text'>
Commit 8a56038c2aef ("Yama: consolidate error reporting") causes lockups
when someone hits a Yama denial. Call chain:

process_vm_readv -&gt; process_vm_rw -&gt; process_vm_rw_core -&gt; mm_access
-&gt; ptrace_may_access
task_lock(...) is taken
__ptrace_may_access -&gt; security_ptrace_access_check
-&gt; yama_ptrace_access_check -&gt; report_access -&gt; kstrdup_quotable_cmdline
-&gt; get_cmdline -&gt; access_process_vm -&gt; get_task_mm
task_lock(...) is taken again

task_lock(p) just calls spin_lock(&amp;p-&gt;alloc_lock), so at this point,
spin_lock() is called on a lock that is already held by the current
process.

Also: Since the alloc_lock is a spinlock, sleeping inside
security_ptrace_access_check hooks is probably not allowed at all? So it's
not even possible to print the cmdline from in there because that might
involve paging in userspace memory.

It would be tempting to rewrite ptrace_may_access() to drop the alloc_lock
before calling the LSM, but even then, ptrace_may_access() itself might be
called from various contexts in which you're not allowed to sleep; for
example, as far as I understand, to be able to hold a reference to another
task, usually an RCU read lock will be taken (see e.g. kcmp() and
get_robust_list()), so that also prohibits sleeping. (And using e.g. FUSE,
a user can cause pagefault handling to take arbitrary amounts of time -
see https://bugs.chromium.org/p/project-zero/issues/detail?id=808.)

Therefore, AFAIK, in order to print the name of a process below
security_ptrace_access_check(), you'd have to either grab a reference to
the mm_struct and defer the access violation reporting or just use the
"comm" value that's stored in kernelspace and accessible without big
complications. (Or you could try to use some kind of atomic remote VM
access that fails if the memory isn't paged in, similar to
copy_from_user_inatomic(), and if necessary fall back to comm, but
that'd be kind of ugly because the comm/cmdline choice would look
pretty random to the user.)

Fix it by deferring reporting of the access violation until current
exits kernelspace the next time.

v2: Don't oops on PTRACE_TRACEME, call report_access under
task_lock(current). Also fix nonsensical comment. And don't use
GPF_ATOMIC for memory allocation with no locks held.
This patch is tested both for ptrace attach and ptrace traceme.

Fixes: 8a56038c2aef ("Yama: consolidate error reporting")
Signed-off-by: Jann Horn &lt;jann@thejh.net&gt;
Acked-by: Kees Cook &lt;keescook@chromium.org&gt;
Signed-off-by: James Morris &lt;james.l.morris@oracle.com&gt;
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
Commit 8a56038c2aef ("Yama: consolidate error reporting") causes lockups
when someone hits a Yama denial. Call chain:

process_vm_readv -&gt; process_vm_rw -&gt; process_vm_rw_core -&gt; mm_access
-&gt; ptrace_may_access
task_lock(...) is taken
__ptrace_may_access -&gt; security_ptrace_access_check
-&gt; yama_ptrace_access_check -&gt; report_access -&gt; kstrdup_quotable_cmdline
-&gt; get_cmdline -&gt; access_process_vm -&gt; get_task_mm
task_lock(...) is taken again

task_lock(p) just calls spin_lock(&amp;p-&gt;alloc_lock), so at this point,
spin_lock() is called on a lock that is already held by the current
process.

Also: Since the alloc_lock is a spinlock, sleeping inside
security_ptrace_access_check hooks is probably not allowed at all? So it's
not even possible to print the cmdline from in there because that might
involve paging in userspace memory.

It would be tempting to rewrite ptrace_may_access() to drop the alloc_lock
before calling the LSM, but even then, ptrace_may_access() itself might be
called from various contexts in which you're not allowed to sleep; for
example, as far as I understand, to be able to hold a reference to another
task, usually an RCU read lock will be taken (see e.g. kcmp() and
get_robust_list()), so that also prohibits sleeping. (And using e.g. FUSE,
a user can cause pagefault handling to take arbitrary amounts of time -
see https://bugs.chromium.org/p/project-zero/issues/detail?id=808.)

Therefore, AFAIK, in order to print the name of a process below
security_ptrace_access_check(), you'd have to either grab a reference to
the mm_struct and defer the access violation reporting or just use the
"comm" value that's stored in kernelspace and accessible without big
complications. (Or you could try to use some kind of atomic remote VM
access that fails if the memory isn't paged in, similar to
copy_from_user_inatomic(), and if necessary fall back to comm, but
that'd be kind of ugly because the comm/cmdline choice would look
pretty random to the user.)

Fix it by deferring reporting of the access violation until current
exits kernelspace the next time.

v2: Don't oops on PTRACE_TRACEME, call report_access under
task_lock(current). Also fix nonsensical comment. And don't use
GPF_ATOMIC for memory allocation with no locks held.
This patch is tested both for ptrace attach and ptrace traceme.

Fixes: 8a56038c2aef ("Yama: consolidate error reporting")
Signed-off-by: Jann Horn &lt;jann@thejh.net&gt;
Acked-by: Kees Cook &lt;keescook@chromium.org&gt;
Signed-off-by: James Morris &lt;james.l.morris@oracle.com&gt;
</pre>
</div>
</content>
</entry>
</feed>
