KEYS: reject NULL restriction string when type is specified

commit 18026d866801d0c52e5550210563222bd6c7191d upstream.

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.

Reported-by: syzbot <syzkaller@googlegroups.com>
Fixes: 97d3aa0f3134 ("KEYS: Add a lookup_restriction function for the asymmetric key type")
Signed-off-by: Eric Biggers <ebiggers@google.com>
Signed-off-by: David Howells <dhowells@redhat.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

diff --git a/security/keys/keyctl.c b/security/keys/keyctl.c
index 76d22f726ae49d7e112c648c59e9c0d3124063f1..1ffe60bb2845f97638157b01ed7fcc4f45714312 100644 (file)
--- a/security/keys/keyctl.c
+++ b/security/keys/keyctl.c
@@ -1588,9 +1588,8 @@ error_keyring:
  * 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;
 }