| Message ID | 754d6ccb4012fc11c7b9369e66ba7f3e816fdb57.1548400400.git.geliangtang@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | security: keys: add NULL checking for key->type->instantiate | expand |
Geliang Tang <geliangtang@gmail.com> wrote: > key->type->instantiate can be NULL, add NULL checking to prevent > NULL pointer dereference in __key_instantiate_and_link(). Do you have an oops report or test case for this? David
On Wed, Feb 06, 2019 at 10:26:53PM +0000, David Howells wrote: > Geliang Tang <geliangtang@gmail.com> wrote: > > > key->type->instantiate can be NULL, add NULL checking to prevent > > NULL pointer dereference in __key_instantiate_and_link(). > > Do you have an oops report or test case for this? > > David Here is the test module code. Insmod it, we can get the oops. #include <linux/init.h> #include <linux/module.h> #include <linux/key.h> #include <linux/key-type.h> #include <linux/cred.h> #include <linux/seq_file.h> static int test_instantiate(struct key *key, struct key_preparsed_payload *prep) { return 0; } static void test_describe(const struct key *key, struct seq_file *m) { seq_puts(m, key->description); } struct key_type test_key_type = { .name = "test", //.instantiate = test_instantiate, .describe = test_describe, }; static int __init test_init(void) { const struct cred *cred = current_cred(); struct key *key; int ret; register_key_type(&test_key_type); key = key_alloc(&test_key_type, "test", GLOBAL_ROOT_UID, GLOBAL_ROOT_GID, cred, KEY_POS_SEARCH, KEY_ALLOC_NOT_IN_QUOTA, NULL); if (IS_ERR(key)) return -1; pr_info("keyring allocated successfully.\n"); ret = key_instantiate_and_link(key, NULL, sizeof(int), current->cred->user->session_keyring, NULL); if (ret < 0) { key_revoke(key); key_put(key); return ret; } return 0; } static void __exit test_exit(void) { unregister_key_type(&test_key_type); } module_init(test_init); module_exit(test_exit); MODULE_LICENSE("GPL");
On Wed, Feb 06, 2019 at 10:26:53PM +0000, David Howells wrote: > Geliang Tang <geliangtang@gmail.com> wrote: > > > key->type->instantiate can be NULL, add NULL checking to prevent > > NULL pointer dereference in __key_instantiate_and_link(). > > Do you have an oops report or test case for this? > > David Here is the test module code. Insmod it, we can get the oops. #include <linux/init.h> #include <linux/module.h> #include <linux/key.h> #include <linux/key-type.h> #include <linux/cred.h> #include <linux/seq_file.h> static int test_instantiate(struct key *key, struct key_preparsed_payload *prep) { return 0; } static void test_describe(const struct key *key, struct seq_file *m) { seq_puts(m, key->description); } struct key_type test_key_type = { .name = "test", //.instantiate = test_instantiate, .describe = test_describe, }; static int __init test_init(void) { const struct cred *cred = current_cred(); struct key *key; int ret; register_key_type(&test_key_type); key = key_alloc(&test_key_type, "test", GLOBAL_ROOT_UID, GLOBAL_ROOT_GID, cred, KEY_POS_SEARCH, KEY_ALLOC_NOT_IN_QUOTA, NULL); if (IS_ERR(key)) return -1; pr_info("keyring allocated successfully.\n"); ret = key_instantiate_and_link(key, NULL, sizeof(int), current->cred->user->session_keyring, NULL); if (ret < 0) { key_revoke(key); key_put(key); return ret; } return 0; } static void __exit test_exit(void) { unregister_key_type(&test_key_type); } module_init(test_init); module_exit(test_exit); MODULE_LICENSE("GPL");
diff --git a/security/keys/key.c b/security/keys/key.c index 44a80d6741a1..ae3cc11be0d2 100644 --- a/security/keys/key.c +++ b/security/keys/key.c @@ -438,7 +438,8 @@ static int __key_instantiate_and_link(struct key *key, /* can't instantiate twice */ if (key->state == KEY_IS_UNINSTANTIATED) { /* instantiate the key */ - ret = key->type->instantiate(key, prep); + if (key->type->instantiate) + ret = key->type->instantiate(key, prep); if (ret == 0) { /* mark the key as being instantiated */
key->type->instantiate can be NULL, add NULL checking to prevent NULL pointer dereference in __key_instantiate_and_link(). Signed-off-by: Geliang Tang <geliangtang@gmail.com> --- security/keys/key.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)