<feed xmlns='http://www.w3.org/2005/Atom'>
<title>linux-stable.git/security, branch v4.1.50</title>
<subtitle>Linux kernel stable tree</subtitle>
<link rel='alternate' type='text/html' href='https://git.tavy.me/linux-stable.git/'/>
<entry>
<title>selinux: skip bounded transition processing if the policy isn't loaded</title>
<updated>2018-03-04T15:28:24+00:00</updated>
<author>
<name>Paul Moore</name>
<email>paul@paul-moore.com</email>
</author>
<published>2017-12-05T22:17:43+00:00</published>
<link rel='alternate' type='text/html' href='https://git.tavy.me/linux-stable.git/commit/?id=03915a4958d91a7b1c5668bb1773784fcef4777e'/>
<id>03915a4958d91a7b1c5668bb1773784fcef4777e</id>
<content type='text'>
[ Upstream commit 4b14752ec4e0d87126e636384cf37c8dd9df157c ]

We can't do anything reasonable in security_bounded_transition() if we
don't have a policy loaded, and in fact we could run into problems
with some of the code inside expecting a policy.  Fix these problems
like we do many others in security/selinux/ss/services.c by checking
to see if the policy is loaded (ss_initialized) and returning quickly
if it isn't.

Reported-by: syzbot &lt;syzkaller-bugs@googlegroups.com&gt;
Signed-off-by: Paul Moore &lt;paul@paul-moore.com&gt;
Acked-by: Stephen Smalley &lt;sds@tycho.nsa.gov&gt;
Reviewed-by: James Morris &lt;james.l.morris@oracle.com&gt;
Signed-off-by: Sasha Levin &lt;alexander.levin@microsoft.com&gt;
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
[ Upstream commit 4b14752ec4e0d87126e636384cf37c8dd9df157c ]

We can't do anything reasonable in security_bounded_transition() if we
don't have a policy loaded, and in fact we could run into problems
with some of the code inside expecting a policy.  Fix these problems
like we do many others in security/selinux/ss/services.c by checking
to see if the policy is loaded (ss_initialized) and returning quickly
if it isn't.

Reported-by: syzbot &lt;syzkaller-bugs@googlegroups.com&gt;
Signed-off-by: Paul Moore &lt;paul@paul-moore.com&gt;
Acked-by: Stephen Smalley &lt;sds@tycho.nsa.gov&gt;
Reviewed-by: James Morris &lt;james.l.morris@oracle.com&gt;
Signed-off-by: Sasha Levin &lt;alexander.levin@microsoft.com&gt;
</pre>
</div>
</content>
</entry>
<entry>
<title>selinux: ensure the context is NUL terminated in security_context_to_sid_core()</title>
<updated>2018-03-04T15:28:24+00:00</updated>
<author>
<name>Paul Moore</name>
<email>paul@paul-moore.com</email>
</author>
<published>2017-11-28T23:51:12+00:00</published>
<link rel='alternate' type='text/html' href='https://git.tavy.me/linux-stable.git/commit/?id=62850f402a7674c5dbaf82e9ce0bab9d177a6c5f'/>
<id>62850f402a7674c5dbaf82e9ce0bab9d177a6c5f</id>
<content type='text'>
[ Upstream commit ef28df55ac27e1e5cd122e19fa311d886d47a756 ]

The syzbot/syzkaller automated tests found a problem in
security_context_to_sid_core() during early boot (before we load the
SELinux policy) where we could potentially feed context strings without
NUL terminators into the strcmp() function.

We already guard against this during normal operation (after the SELinux
policy has been loaded) by making a copy of the context strings and
explicitly adding a NUL terminator to the end.  The patch extends this
protection to the early boot case (no loaded policy) by moving the context
copy earlier in security_context_to_sid_core().

Reported-by: syzbot &lt;syzkaller@googlegroups.com&gt;
Signed-off-by: Paul Moore &lt;paul@paul-moore.com&gt;
Reviewed-By: William Roberts &lt;william.c.roberts@intel.com&gt;
Signed-off-by: Sasha Levin &lt;alexander.levin@microsoft.com&gt;
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
[ Upstream commit ef28df55ac27e1e5cd122e19fa311d886d47a756 ]

The syzbot/syzkaller automated tests found a problem in
security_context_to_sid_core() during early boot (before we load the
SELinux policy) where we could potentially feed context strings without
NUL terminators into the strcmp() function.

We already guard against this during normal operation (after the SELinux
policy has been loaded) by making a copy of the context strings and
explicitly adding a NUL terminator to the end.  The patch extends this
protection to the early boot case (no loaded policy) by moving the context
copy earlier in security_context_to_sid_core().

Reported-by: syzbot &lt;syzkaller@googlegroups.com&gt;
Signed-off-by: Paul Moore &lt;paul@paul-moore.com&gt;
Reviewed-By: William Roberts &lt;william.c.roberts@intel.com&gt;
Signed-off-by: Sasha Levin &lt;alexander.levin@microsoft.com&gt;
</pre>
</div>
</content>
</entry>
<entry>
<title>KEYS: encrypted: fix buffer overread in valid_master_desc()</title>
<updated>2018-03-01T03:09:50+00:00</updated>
<author>
<name>Eric Biggers</name>
<email>ebiggers@google.com</email>
</author>
<published>2017-06-08T13:48:18+00:00</published>
<link rel='alternate' type='text/html' href='https://git.tavy.me/linux-stable.git/commit/?id=1f338384b46b16f9682c8e01f8d158e90e5d6cc2'/>
<id>1f338384b46b16f9682c8e01f8d158e90e5d6cc2</id>
<content type='text'>
[ Upstream commit 794b4bc292f5d31739d89c0202c54e7dc9bc3add ]

With the 'encrypted' key type it was possible for userspace to provide a
data blob ending with a master key description shorter than expected,
e.g. 'keyctl add encrypted desc "new x" @s'.  When validating such a
master key description, validate_master_desc() could read beyond the end
of the buffer.  Fix this by using strncmp() instead of memcmp().  [Also
clean up the code to deduplicate some logic.]

Cc: Mimi Zohar &lt;zohar@linux.vnet.ibm.com&gt;
Signed-off-by: Eric Biggers &lt;ebiggers@google.com&gt;
Signed-off-by: David Howells &lt;dhowells@redhat.com&gt;
Signed-off-by: James Morris &lt;james.l.morris@oracle.com&gt;
Signed-off-by: Sasha Levin &lt;alexander.levin@microsoft.com&gt;
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
[ Upstream commit 794b4bc292f5d31739d89c0202c54e7dc9bc3add ]

With the 'encrypted' key type it was possible for userspace to provide a
data blob ending with a master key description shorter than expected,
e.g. 'keyctl add encrypted desc "new x" @s'.  When validating such a
master key description, validate_master_desc() could read beyond the end
of the buffer.  Fix this by using strncmp() instead of memcmp().  [Also
clean up the code to deduplicate some logic.]

Cc: Mimi Zohar &lt;zohar@linux.vnet.ibm.com&gt;
Signed-off-by: Eric Biggers &lt;ebiggers@google.com&gt;
Signed-off-by: David Howells &lt;dhowells@redhat.com&gt;
Signed-off-by: James Morris &lt;james.l.morris@oracle.com&gt;
Signed-off-by: Sasha Levin &lt;alexander.levin@microsoft.com&gt;
</pre>
</div>
</content>
</entry>
<entry>
<title>ima: fix hash algorithm initialization</title>
<updated>2018-01-17T17:55:17+00:00</updated>
<author>
<name>Boshi Wang</name>
<email>wangboshi@huawei.com</email>
</author>
<published>2017-10-20T08:01:03+00:00</published>
<link rel='alternate' type='text/html' href='https://git.tavy.me/linux-stable.git/commit/?id=1203cd50416157f5dba4742a7c50b2a6c9efd344'/>
<id>1203cd50416157f5dba4742a7c50b2a6c9efd344</id>
<content type='text'>
[ Upstream commit ebe7c0a7be92bbd34c6ff5b55810546a0ee05bee ]

The hash_setup function always sets the hash_setup_done flag, even
when the hash algorithm is invalid.  This prevents the default hash
algorithm defined as CONFIG_IMA_DEFAULT_HASH from being used.

This patch sets hash_setup_done flag only for valid hash algorithms.

Fixes: e7a2ad7eb6f4 "ima: enable support for larger default filedata hash
	algorithms"
Signed-off-by: Boshi Wang &lt;wangboshi@huawei.com&gt;
Signed-off-by: Mimi Zohar &lt;zohar@linux.vnet.ibm.com&gt;

Signed-off-by: Sasha Levin &lt;alexander.levin@microsoft.com&gt;
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
[ Upstream commit ebe7c0a7be92bbd34c6ff5b55810546a0ee05bee ]

The hash_setup function always sets the hash_setup_done flag, even
when the hash algorithm is invalid.  This prevents the default hash
algorithm defined as CONFIG_IMA_DEFAULT_HASH from being used.

This patch sets hash_setup_done flag only for valid hash algorithms.

Fixes: e7a2ad7eb6f4 "ima: enable support for larger default filedata hash
	algorithms"
Signed-off-by: Boshi Wang &lt;wangboshi@huawei.com&gt;
Signed-off-by: Mimi Zohar &lt;zohar@linux.vnet.ibm.com&gt;

Signed-off-by: Sasha Levin &lt;alexander.levin@microsoft.com&gt;
</pre>
</div>
</content>
</entry>
<entry>
<title>KEYS: fix dereferencing NULL payload with nonzero length</title>
<updated>2018-01-17T17:55:16+00:00</updated>
<author>
<name>Eric Biggers</name>
<email>ebiggers@google.com</email>
</author>
<published>2017-06-08T13:48:40+00:00</published>
<link rel='alternate' type='text/html' href='https://git.tavy.me/linux-stable.git/commit/?id=c19aa530105b0d780ad72a78a7ef271037bcb774'/>
<id>c19aa530105b0d780ad72a78a7ef271037bcb774</id>
<content type='text'>
[ Upstream commit 5649645d725c73df4302428ee4e02c869248b4c5 ]

sys_add_key() and the KEYCTL_UPDATE operation of sys_keyctl() allowed a
NULL payload with nonzero length to be passed to the key type's
-&gt;preparse(), -&gt;instantiate(), and/or -&gt;update() methods.  Various key
types including asymmetric, cifs.idmap, cifs.spnego, and pkcs7_test did
not handle this case, allowing an unprivileged user to trivially cause a
NULL pointer dereference (kernel oops) if one of these key types was
present.  Fix it by doing the copy_from_user() when 'plen' is nonzero
rather than when '_payload' is non-NULL, causing the syscall to fail
with EFAULT as expected when an invalid buffer is specified.

Cc: stable@vger.kernel.org # 2.6.10+
Signed-off-by: Eric Biggers &lt;ebiggers@google.com&gt;
Signed-off-by: David Howells &lt;dhowells@redhat.com&gt;
Signed-off-by: James Morris &lt;james.l.morris@oracle.com&gt;
Signed-off-by: Sasha Levin &lt;alexander.levin@microsoft.com&gt;
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
[ Upstream commit 5649645d725c73df4302428ee4e02c869248b4c5 ]

sys_add_key() and the KEYCTL_UPDATE operation of sys_keyctl() allowed a
NULL payload with nonzero length to be passed to the key type's
-&gt;preparse(), -&gt;instantiate(), and/or -&gt;update() methods.  Various key
types including asymmetric, cifs.idmap, cifs.spnego, and pkcs7_test did
not handle this case, allowing an unprivileged user to trivially cause a
NULL pointer dereference (kernel oops) if one of these key types was
present.  Fix it by doing the copy_from_user() when 'plen' is nonzero
rather than when '_payload' is non-NULL, causing the syscall to fail
with EFAULT as expected when an invalid buffer is specified.

Cc: stable@vger.kernel.org # 2.6.10+
Signed-off-by: Eric Biggers &lt;ebiggers@google.com&gt;
Signed-off-by: David Howells &lt;dhowells@redhat.com&gt;
Signed-off-by: James Morris &lt;james.l.morris@oracle.com&gt;
Signed-off-by: Sasha Levin &lt;alexander.levin@microsoft.com&gt;
</pre>
</div>
</content>
</entry>
<entry>
<title>ima: do not update security.ima if appraisal status is not INTEGRITY_PASS</title>
<updated>2017-12-07T02:20:15+00:00</updated>
<author>
<name>Roberto Sassu</name>
<email>roberto.sassu@huawei.com</email>
</author>
<published>2017-11-07T10:37:07+00:00</published>
<link rel='alternate' type='text/html' href='https://git.tavy.me/linux-stable.git/commit/?id=312ce4d0bfd851679d4f819ac9c6878a1b0baca6'/>
<id>312ce4d0bfd851679d4f819ac9c6878a1b0baca6</id>
<content type='text'>
[ Upstream commit 020aae3ee58c1af0e7ffc4e2cc9fe4dc630338cb ]

Commit b65a9cfc2c38 ("Untangling ima mess, part 2: deal with counters")
moved the call of ima_file_check() from may_open() to do_filp_open() at a
point where the file descriptor is already opened.

This breaks the assumption made by IMA that file descriptors being closed
belong to files whose access was granted by ima_file_check(). The
consequence is that security.ima and security.evm are updated with good
values, regardless of the current appraisal status.

For example, if a file does not have security.ima, IMA will create it after
opening the file for writing, even if access is denied. Access to the file
will be allowed afterwards.

Avoid this issue by checking the appraisal status before updating
security.ima.

Cc: stable@vger.kernel.org
Signed-off-by: Roberto Sassu &lt;roberto.sassu@huawei.com&gt;
Signed-off-by: Mimi Zohar &lt;zohar@linux.vnet.ibm.com&gt;
Signed-off-by: James Morris &lt;james.l.morris@oracle.com&gt;
Signed-off-by: Sasha Levin &lt;alexander.levin@verizon.com&gt;
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
[ Upstream commit 020aae3ee58c1af0e7ffc4e2cc9fe4dc630338cb ]

Commit b65a9cfc2c38 ("Untangling ima mess, part 2: deal with counters")
moved the call of ima_file_check() from may_open() to do_filp_open() at a
point where the file descriptor is already opened.

This breaks the assumption made by IMA that file descriptors being closed
belong to files whose access was granted by ima_file_check(). The
consequence is that security.ima and security.evm are updated with good
values, regardless of the current appraisal status.

For example, if a file does not have security.ima, IMA will create it after
opening the file for writing, even if access is denied. Access to the file
will be allowed afterwards.

Avoid this issue by checking the appraisal status before updating
security.ima.

Cc: stable@vger.kernel.org
Signed-off-by: Roberto Sassu &lt;roberto.sassu@huawei.com&gt;
Signed-off-by: Mimi Zohar &lt;zohar@linux.vnet.ibm.com&gt;
Signed-off-by: James Morris &lt;james.l.morris@oracle.com&gt;
Signed-off-by: Sasha Levin &lt;alexander.levin@verizon.com&gt;
</pre>
</div>
</content>
</entry>
<entry>
<title>security/keys: add CONFIG_KEYS_COMPAT to Kconfig</title>
<updated>2017-12-07T02:20:08+00:00</updated>
<author>
<name>Bilal Amarni</name>
<email>bilal.amarni@gmail.com</email>
</author>
<published>2017-06-08T13:47:26+00:00</published>
<link rel='alternate' type='text/html' href='https://git.tavy.me/linux-stable.git/commit/?id=58e1fb13e2bcde09f732055c9a2414cd9e222a4e'/>
<id>58e1fb13e2bcde09f732055c9a2414cd9e222a4e</id>
<content type='text'>
[ Upstream commit 47b2c3fff4932e6fc17ce13d51a43c6969714e20 ]

CONFIG_KEYS_COMPAT is defined in arch-specific Kconfigs and is missing for
several 64-bit architectures : mips, parisc, tile.

At the moment and for those architectures, calling in 32-bit userspace the
keyctl syscall would return an ENOSYS error.

This patch moves the CONFIG_KEYS_COMPAT option to security/keys/Kconfig, to
make sure the compatibility wrapper is registered by default for any 64-bit
architecture as long as it is configured with CONFIG_COMPAT.

[DH: Modified to remove arm64 compat enablement also as requested by Eric
 Biggers]

Signed-off-by: Bilal Amarni &lt;bilal.amarni@gmail.com&gt;
Signed-off-by: David Howells &lt;dhowells@redhat.com&gt;
Reviewed-by: Arnd Bergmann &lt;arnd@arndb.de&gt;
cc: Eric Biggers &lt;ebiggers3@gmail.com&gt;
Signed-off-by: James Morris &lt;james.l.morris@oracle.com&gt;
Signed-off-by: Sasha Levin &lt;alexander.levin@verizon.com&gt;
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
[ Upstream commit 47b2c3fff4932e6fc17ce13d51a43c6969714e20 ]

CONFIG_KEYS_COMPAT is defined in arch-specific Kconfigs and is missing for
several 64-bit architectures : mips, parisc, tile.

At the moment and for those architectures, calling in 32-bit userspace the
keyctl syscall would return an ENOSYS error.

This patch moves the CONFIG_KEYS_COMPAT option to security/keys/Kconfig, to
make sure the compatibility wrapper is registered by default for any 64-bit
architecture as long as it is configured with CONFIG_COMPAT.

[DH: Modified to remove arm64 compat enablement also as requested by Eric
 Biggers]

Signed-off-by: Bilal Amarni &lt;bilal.amarni@gmail.com&gt;
Signed-off-by: David Howells &lt;dhowells@redhat.com&gt;
Reviewed-by: Arnd Bergmann &lt;arnd@arndb.de&gt;
cc: Eric Biggers &lt;ebiggers3@gmail.com&gt;
Signed-off-by: James Morris &lt;james.l.morris@oracle.com&gt;
Signed-off-by: Sasha Levin &lt;alexander.levin@verizon.com&gt;
</pre>
</div>
</content>
</entry>
<entry>
<title>KEYS: return full count in keyring_read() if buffer is too small</title>
<updated>2017-12-07T02:19:53+00:00</updated>
<author>
<name>Eric Biggers</name>
<email>ebiggers@google.com</email>
</author>
<published>2017-11-02T00:47:03+00:00</published>
<link rel='alternate' type='text/html' href='https://git.tavy.me/linux-stable.git/commit/?id=b0bee71210700175866ada0cb179f3bf63b80ceb'/>
<id>b0bee71210700175866ada0cb179f3bf63b80ceb</id>
<content type='text'>
[ Upstream commit 3239b6f29bdfb4b0a2ba59df995fc9e6f4df7f1f ]

Commit e645016abc80 ("KEYS: fix writing past end of user-supplied buffer
in keyring_read()") made keyring_read() stop corrupting userspace memory
when the user-supplied buffer is too small.  However it also made the
return value in that case be the short buffer size rather than the size
required, yet keyctl_read() is actually documented to return the size
required.  Therefore, switch it over to the documented behavior.

Note that for now we continue to have it fill the short buffer, since it
did that before (pre-v3.13) and dump_key_tree_aux() in keyutils arguably
relies on it.

Fixes: e645016abc80 ("KEYS: fix writing past end of user-supplied buffer in keyring_read()")
Reported-by: Ben Hutchings &lt;ben@decadent.org.uk&gt;
Cc: &lt;stable@vger.kernel.org&gt; # v3.13+
Signed-off-by: Eric Biggers &lt;ebiggers@google.com&gt;
Signed-off-by: David Howells &lt;dhowells@redhat.com&gt;
Reviewed-by: James Morris &lt;james.l.morris@oracle.com&gt;
Signed-off-by: James Morris &lt;james.l.morris@oracle.com&gt;
Signed-off-by: Sasha Levin &lt;alexander.levin@verizon.com&gt;
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
[ Upstream commit 3239b6f29bdfb4b0a2ba59df995fc9e6f4df7f1f ]

Commit e645016abc80 ("KEYS: fix writing past end of user-supplied buffer
in keyring_read()") made keyring_read() stop corrupting userspace memory
when the user-supplied buffer is too small.  However it also made the
return value in that case be the short buffer size rather than the size
required, yet keyctl_read() is actually documented to return the size
required.  Therefore, switch it over to the documented behavior.

Note that for now we continue to have it fill the short buffer, since it
did that before (pre-v3.13) and dump_key_tree_aux() in keyutils arguably
relies on it.

Fixes: e645016abc80 ("KEYS: fix writing past end of user-supplied buffer in keyring_read()")
Reported-by: Ben Hutchings &lt;ben@decadent.org.uk&gt;
Cc: &lt;stable@vger.kernel.org&gt; # v3.13+
Signed-off-by: Eric Biggers &lt;ebiggers@google.com&gt;
Signed-off-by: David Howells &lt;dhowells@redhat.com&gt;
Reviewed-by: James Morris &lt;james.l.morris@oracle.com&gt;
Signed-off-by: James Morris &lt;james.l.morris@oracle.com&gt;
Signed-off-by: Sasha Levin &lt;alexander.levin@verizon.com&gt;
</pre>
</div>
</content>
</entry>
<entry>
<title>KEYS: encrypted: fix dereference of NULL user_key_payload</title>
<updated>2017-12-07T02:19:52+00:00</updated>
<author>
<name>Eric Biggers</name>
<email>ebiggers@google.com</email>
</author>
<published>2017-10-09T19:37:49+00:00</published>
<link rel='alternate' type='text/html' href='https://git.tavy.me/linux-stable.git/commit/?id=0149e2bdac423ee6488ed361500b0847197dd7f3'/>
<id>0149e2bdac423ee6488ed361500b0847197dd7f3</id>
<content type='text'>
[ Upstream commit 13923d0865ca96312197962522e88bc0aedccd74 ]

A key of type "encrypted" references a "master key" which is used to
encrypt and decrypt the encrypted key's payload.  However, when we
accessed the master key's payload, we failed to handle the case where
the master key has been revoked, which sets the payload pointer to NULL.
Note that request_key() *does* skip revoked keys, but there is still a
window where the key can be revoked before we acquire its semaphore.

Fix it by checking for a NULL payload, treating it like a key which was
already revoked at the time it was requested.

This was an issue for master keys of type "user" only.  Master keys can
also be of type "trusted", but those cannot be revoked.

Fixes: 7e70cb497850 ("keys: add new key-type encrypted")
Reviewed-by: James Morris &lt;james.l.morris@oracle.com&gt;
Cc: &lt;stable@vger.kernel.org&gt;    [v2.6.38+]
Cc: Mimi Zohar &lt;zohar@linux.vnet.ibm.com&gt;
Cc: David Safford &lt;safford@us.ibm.com&gt;
Signed-off-by: Eric Biggers &lt;ebiggers@google.com&gt;
Signed-off-by: David Howells &lt;dhowells@redhat.com&gt;
Signed-off-by: Sasha Levin &lt;alexander.levin@verizon.com&gt;
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
[ Upstream commit 13923d0865ca96312197962522e88bc0aedccd74 ]

A key of type "encrypted" references a "master key" which is used to
encrypt and decrypt the encrypted key's payload.  However, when we
accessed the master key's payload, we failed to handle the case where
the master key has been revoked, which sets the payload pointer to NULL.
Note that request_key() *does* skip revoked keys, but there is still a
window where the key can be revoked before we acquire its semaphore.

Fix it by checking for a NULL payload, treating it like a key which was
already revoked at the time it was requested.

This was an issue for master keys of type "user" only.  Master keys can
also be of type "trusted", but those cannot be revoked.

Fixes: 7e70cb497850 ("keys: add new key-type encrypted")
Reviewed-by: James Morris &lt;james.l.morris@oracle.com&gt;
Cc: &lt;stable@vger.kernel.org&gt;    [v2.6.38+]
Cc: Mimi Zohar &lt;zohar@linux.vnet.ibm.com&gt;
Cc: David Safford &lt;safford@us.ibm.com&gt;
Signed-off-by: Eric Biggers &lt;ebiggers@google.com&gt;
Signed-off-by: David Howells &lt;dhowells@redhat.com&gt;
Signed-off-by: Sasha Levin &lt;alexander.levin@verizon.com&gt;
</pre>
</div>
</content>
</entry>
<entry>
<title>KEYS: don't let add_key() update an uninstantiated key</title>
<updated>2017-11-06T04:54:36+00:00</updated>
<author>
<name>David Howells</name>
<email>dhowells@redhat.com</email>
</author>
<published>2017-10-12T15:00:41+00:00</published>
<link rel='alternate' type='text/html' href='https://git.tavy.me/linux-stable.git/commit/?id=7aeb11ac706537b29b704c344572c3f9ac908389'/>
<id>7aeb11ac706537b29b704c344572c3f9ac908389</id>
<content type='text'>
[ Upstream commit 60ff5b2f547af3828aebafd54daded44cfb0807a ]

Currently, when passed a key that already exists, add_key() will call the
key's -&gt;update() method if such exists.  But this is heavily broken in the
case where the key is uninstantiated because it doesn't call
__key_instantiate_and_link().  Consequently, it doesn't do most of the
things that are supposed to happen when the key is instantiated, such as
setting the instantiation state, clearing KEY_FLAG_USER_CONSTRUCT and
awakening tasks waiting on it, and incrementing key-&gt;user-&gt;nikeys.

It also never takes key_construction_mutex, which means that
-&gt;instantiate() can run concurrently with -&gt;update() on the same key.  In
the case of the "user" and "logon" key types this causes a memory leak, at
best.  Maybe even worse, the -&gt;update() methods of the "encrypted" and
"trusted" key types actually just dereference a NULL pointer when passed an
uninstantiated key.

Change key_create_or_update() to wait interruptibly for the key to finish
construction before continuing.

This patch only affects *uninstantiated* keys.  For now we still allow a
negatively instantiated key to be updated (thereby positively
instantiating it), although that's broken too (the next patch fixes it)
and I'm not sure that anyone actually uses that functionality either.

Here is a simple reproducer for the bug using the "encrypted" key type
(requires CONFIG_ENCRYPTED_KEYS=y), though as noted above the bug
pertained to more than just the "encrypted" key type:

    #include &lt;stdlib.h&gt;
    #include &lt;unistd.h&gt;
    #include &lt;keyutils.h&gt;

    int main(void)
    {
        int ringid = keyctl_join_session_keyring(NULL);

        if (fork()) {
            for (;;) {
                const char payload[] = "update user:foo 32";

                usleep(rand() % 10000);
                add_key("encrypted", "desc", payload, sizeof(payload), ringid);
                keyctl_clear(ringid);
            }
        } else {
            for (;;)
                request_key("encrypted", "desc", "callout_info", ringid);
        }
    }

It causes:

    BUG: unable to handle kernel NULL pointer dereference at 0000000000000018
    IP: encrypted_update+0xb0/0x170
    PGD 7a178067 P4D 7a178067 PUD 77269067 PMD 0
    PREEMPT SMP
    CPU: 0 PID: 340 Comm: reproduce Tainted: G      D         4.14.0-rc1-00025-g428490e38b2e #796
    Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
    task: ffff8a467a39a340 task.stack: ffffb15c40770000
    RIP: 0010:encrypted_update+0xb0/0x170
    RSP: 0018:ffffb15c40773de8 EFLAGS: 00010246
    RAX: 0000000000000000 RBX: ffff8a467a275b00 RCX: 0000000000000000
    RDX: 0000000000000005 RSI: ffff8a467a275b14 RDI: ffffffffb742f303
    RBP: ffffb15c40773e20 R08: 0000000000000000 R09: ffff8a467a275b17
    R10: 0000000000000020 R11: 0000000000000000 R12: 0000000000000000
    R13: 0000000000000000 R14: ffff8a4677057180 R15: ffff8a467a275b0f
    FS:  00007f5d7fb08700(0000) GS:ffff8a467f200000(0000) knlGS:0000000000000000
    CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
    CR2: 0000000000000018 CR3: 0000000077262005 CR4: 00000000001606f0
    Call Trace:
     key_create_or_update+0x2bc/0x460
     SyS_add_key+0x10c/0x1d0
     entry_SYSCALL_64_fastpath+0x1f/0xbe
    RIP: 0033:0x7f5d7f211259
    RSP: 002b:00007ffed03904c8 EFLAGS: 00000246 ORIG_RAX: 00000000000000f8
    RAX: ffffffffffffffda RBX: 000000003b2a7955 RCX: 00007f5d7f211259
    RDX: 00000000004009e4 RSI: 00000000004009ff RDI: 0000000000400a04
    RBP: 0000000068db8bad R08: 000000003b2a7955 R09: 0000000000000004
    R10: 000000000000001a R11: 0000000000000246 R12: 0000000000400868
    R13: 00007ffed03905d0 R14: 0000000000000000 R15: 0000000000000000
    Code: 77 28 e8 64 34 1f 00 45 31 c0 31 c9 48 8d 55 c8 48 89 df 48 8d 75 d0 e8 ff f9 ff ff 85 c0 41 89 c4 0f 88 84 00 00 00 4c 8b 7d c8 &lt;49&gt; 8b 75 18 4c 89 ff e8 24 f8 ff ff 85 c0 41 89 c4 78 6d 49 8b
    RIP: encrypted_update+0xb0/0x170 RSP: ffffb15c40773de8
    CR2: 0000000000000018

Cc: &lt;stable@vger.kernel.org&gt; # v2.6.12+
Reported-by: Eric Biggers &lt;ebiggers@google.com&gt;
Signed-off-by: David Howells &lt;dhowells@redhat.com&gt;
cc: Eric Biggers &lt;ebiggers@google.com&gt;
Signed-off-by: Sasha Levin &lt;alexander.levin@verizon.com&gt;
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
[ Upstream commit 60ff5b2f547af3828aebafd54daded44cfb0807a ]

Currently, when passed a key that already exists, add_key() will call the
key's -&gt;update() method if such exists.  But this is heavily broken in the
case where the key is uninstantiated because it doesn't call
__key_instantiate_and_link().  Consequently, it doesn't do most of the
things that are supposed to happen when the key is instantiated, such as
setting the instantiation state, clearing KEY_FLAG_USER_CONSTRUCT and
awakening tasks waiting on it, and incrementing key-&gt;user-&gt;nikeys.

It also never takes key_construction_mutex, which means that
-&gt;instantiate() can run concurrently with -&gt;update() on the same key.  In
the case of the "user" and "logon" key types this causes a memory leak, at
best.  Maybe even worse, the -&gt;update() methods of the "encrypted" and
"trusted" key types actually just dereference a NULL pointer when passed an
uninstantiated key.

Change key_create_or_update() to wait interruptibly for the key to finish
construction before continuing.

This patch only affects *uninstantiated* keys.  For now we still allow a
negatively instantiated key to be updated (thereby positively
instantiating it), although that's broken too (the next patch fixes it)
and I'm not sure that anyone actually uses that functionality either.

Here is a simple reproducer for the bug using the "encrypted" key type
(requires CONFIG_ENCRYPTED_KEYS=y), though as noted above the bug
pertained to more than just the "encrypted" key type:

    #include &lt;stdlib.h&gt;
    #include &lt;unistd.h&gt;
    #include &lt;keyutils.h&gt;

    int main(void)
    {
        int ringid = keyctl_join_session_keyring(NULL);

        if (fork()) {
            for (;;) {
                const char payload[] = "update user:foo 32";

                usleep(rand() % 10000);
                add_key("encrypted", "desc", payload, sizeof(payload), ringid);
                keyctl_clear(ringid);
            }
        } else {
            for (;;)
                request_key("encrypted", "desc", "callout_info", ringid);
        }
    }

It causes:

    BUG: unable to handle kernel NULL pointer dereference at 0000000000000018
    IP: encrypted_update+0xb0/0x170
    PGD 7a178067 P4D 7a178067 PUD 77269067 PMD 0
    PREEMPT SMP
    CPU: 0 PID: 340 Comm: reproduce Tainted: G      D         4.14.0-rc1-00025-g428490e38b2e #796
    Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
    task: ffff8a467a39a340 task.stack: ffffb15c40770000
    RIP: 0010:encrypted_update+0xb0/0x170
    RSP: 0018:ffffb15c40773de8 EFLAGS: 00010246
    RAX: 0000000000000000 RBX: ffff8a467a275b00 RCX: 0000000000000000
    RDX: 0000000000000005 RSI: ffff8a467a275b14 RDI: ffffffffb742f303
    RBP: ffffb15c40773e20 R08: 0000000000000000 R09: ffff8a467a275b17
    R10: 0000000000000020 R11: 0000000000000000 R12: 0000000000000000
    R13: 0000000000000000 R14: ffff8a4677057180 R15: ffff8a467a275b0f
    FS:  00007f5d7fb08700(0000) GS:ffff8a467f200000(0000) knlGS:0000000000000000
    CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
    CR2: 0000000000000018 CR3: 0000000077262005 CR4: 00000000001606f0
    Call Trace:
     key_create_or_update+0x2bc/0x460
     SyS_add_key+0x10c/0x1d0
     entry_SYSCALL_64_fastpath+0x1f/0xbe
    RIP: 0033:0x7f5d7f211259
    RSP: 002b:00007ffed03904c8 EFLAGS: 00000246 ORIG_RAX: 00000000000000f8
    RAX: ffffffffffffffda RBX: 000000003b2a7955 RCX: 00007f5d7f211259
    RDX: 00000000004009e4 RSI: 00000000004009ff RDI: 0000000000400a04
    RBP: 0000000068db8bad R08: 000000003b2a7955 R09: 0000000000000004
    R10: 000000000000001a R11: 0000000000000246 R12: 0000000000400868
    R13: 00007ffed03905d0 R14: 0000000000000000 R15: 0000000000000000
    Code: 77 28 e8 64 34 1f 00 45 31 c0 31 c9 48 8d 55 c8 48 89 df 48 8d 75 d0 e8 ff f9 ff ff 85 c0 41 89 c4 0f 88 84 00 00 00 4c 8b 7d c8 &lt;49&gt; 8b 75 18 4c 89 ff e8 24 f8 ff ff 85 c0 41 89 c4 78 6d 49 8b
    RIP: encrypted_update+0xb0/0x170 RSP: ffffb15c40773de8
    CR2: 0000000000000018

Cc: &lt;stable@vger.kernel.org&gt; # v2.6.12+
Reported-by: Eric Biggers &lt;ebiggers@google.com&gt;
Signed-off-by: David Howells &lt;dhowells@redhat.com&gt;
cc: Eric Biggers &lt;ebiggers@google.com&gt;
Signed-off-by: Sasha Levin &lt;alexander.levin@verizon.com&gt;
</pre>
</div>
</content>
</entry>
</feed>
