Eric,
On Thu, 30 Nov 2017, Eric Biggers wrote:
From: Eric Biggers ebiggers@google.com
keyctl_restrict_keyring() allows through a NULL restriction when the "type" is non-NULL, which causes a NULL pointer dereference in asymmetric_lookup_restriction() when it calls strcmp() on the restriction string.
But no key types actually use a "NULL restriction" to mean anything, so update keyctl_restrict_keyring() to reject it with EINVAL.
Since this fixes the bug for the asymmetric key type and ensures that other key types won't make the same mistake, I agree this is the way to fix it. I did not find any issues in the patch.
Thanks,
Mat
Reported-by: syzbot syzkaller@googlegroups.com Fixes: 97d3aa0f3134 ("KEYS: Add a lookup_restriction function for the asymmetric key type") Cc: stable@vger.kernel.org # v4.12+ Signed-off-by: Eric Biggers ebiggers@google.com
security/keys/keyctl.c | 24 ++++++++++-------------- 1 file changed, 10 insertions(+), 14 deletions(-)
diff --git a/security/keys/keyctl.c b/security/keys/keyctl.c index 76d22f726ae4..1ffe60bb2845 100644 --- a/security/keys/keyctl.c +++ b/security/keys/keyctl.c @@ -1588,9 +1588,8 @@ long keyctl_session_to_parent(void)
- The caller must have Setattr permission to change keyring restrictions.
- The requested type name may be a NULL pointer to reject all attempts
- to link to the keyring. If _type is non-NULL, _restriction can be
- NULL or a pointer to a string describing the restriction. If _type is
- NULL, _restriction must also be NULL.
- to link to the keyring. In this case, _restriction must also be NULL.
- Otherwise, both _type and _restriction must be non-NULL.
- Returns 0 if successful.
*/ @@ -1598,7 +1597,6 @@ long keyctl_restrict_keyring(key_serial_t id, const char __user *_type, const char __user *_restriction) { key_ref_t key_ref;
- bool link_reject = !_type; char type[32]; char *restriction = NULL; long ret;
@@ -1607,31 +1605,29 @@ long keyctl_restrict_keyring(key_serial_t id, const char __user *_type, if (IS_ERR(key_ref)) return PTR_ERR(key_ref);
- ret = -EINVAL; if (_type) {
ret = key_get_type_from_user(type, _type, sizeof(type));if (ret < 0)
if (!_restriction) goto error;
}
if (_restriction) {
if (!_type) {ret = -EINVAL;
ret = key_get_type_from_user(type, _type, sizeof(type));if (ret < 0) goto error;
}restriction = strndup_user(_restriction, PAGE_SIZE); if (IS_ERR(restriction)) { ret = PTR_ERR(restriction); goto error; }
- } else {
if (_restriction)goto error;
- ret = keyring_restrict(key_ref, link_reject ? NULL : type, restriction);
- ret = keyring_restrict(key_ref, _type ? type : NULL, restriction); kfree(restriction);
error: key_ref_put(key_ref);
- return ret;
}
-- 2.15.0.531.g2ccb3012c9-goog
-- Mat Martineau Intel OTC