* [PATCH v2 0/3] KEYS: fix error handling in request_key_auth_new()
@ 2017-09-21 20:57 ` Eric Biggers
0 siblings, 0 replies; 18+ messages in thread
From: Eric Biggers @ 2017-09-21 20:57 UTC (permalink / raw)
To: linux-security-module
From: Eric Biggers <ebiggers@google.com>
Fix a reference leak and a NULL pointer dereference in the error
handling paths of request_key_auth_new().
Eric Biggers (3):
KEYS: fix cred refcount leak in request_key_auth_new()
KEYS: don't revoke uninstantiated key in request_key_auth_new()
KEYS: use kmemdup() in request_key_auth_new()
security/keys/request_key_auth.c | 73 ++++++++++++++++++----------------------
1 file changed, 32 insertions(+), 41 deletions(-)
--
2.14.1.821.g8fa685d3b7-goog
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH v2 0/3] KEYS: fix error handling in request_key_auth_new()
@ 2017-09-21 20:57 ` Eric Biggers
0 siblings, 0 replies; 18+ messages in thread
From: Eric Biggers @ 2017-09-21 20:57 UTC (permalink / raw)
To: keyrings
Cc: David Howells, Michael Halcrow, linux-security-module,
linux-kernel, Eric Biggers
From: Eric Biggers <ebiggers@google.com>
Fix a reference leak and a NULL pointer dereference in the error
handling paths of request_key_auth_new().
Eric Biggers (3):
KEYS: fix cred refcount leak in request_key_auth_new()
KEYS: don't revoke uninstantiated key in request_key_auth_new()
KEYS: use kmemdup() in request_key_auth_new()
security/keys/request_key_auth.c | 73 ++++++++++++++++++----------------------
1 file changed, 32 insertions(+), 41 deletions(-)
--
2.14.1.821.g8fa685d3b7-goog
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH v2 0/3] KEYS: fix error handling in request_key_auth_new()
@ 2017-09-21 20:57 ` Eric Biggers
0 siblings, 0 replies; 18+ messages in thread
From: Eric Biggers @ 2017-09-21 20:57 UTC (permalink / raw)
To: linux-security-module
From: Eric Biggers <ebiggers@google.com>
Fix a reference leak and a NULL pointer dereference in the error
handling paths of request_key_auth_new().
Eric Biggers (3):
KEYS: fix cred refcount leak in request_key_auth_new()
KEYS: don't revoke uninstantiated key in request_key_auth_new()
KEYS: use kmemdup() in request_key_auth_new()
security/keys/request_key_auth.c | 73 ++++++++++++++++++----------------------
1 file changed, 32 insertions(+), 41 deletions(-)
--
2.14.1.821.g8fa685d3b7-goog
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH v2 1/3] KEYS: fix cred refcount leak in request_key_auth_new()
2017-09-21 20:57 ` Eric Biggers
(?)
@ 2017-09-21 20:57 ` Eric Biggers
-1 siblings, 0 replies; 18+ messages in thread
From: Eric Biggers @ 2017-09-21 20:57 UTC (permalink / raw)
To: linux-security-module
From: Eric Biggers <ebiggers@google.com>
In request_key_auth_new(), if key_alloc() or key_instantiate_and_link()
were to fail, we would leak a reference to the 'struct cred'. Currently
this can only happen if key_alloc() fails to allocate memory. But it
still should be fixed, as it is a more severe bug waiting to happen.
Fix it by cleaning things up to use a helper function which frees a
'struct request_key_auth' correctly.
Fixes: d84f4f992cbd ("CRED: Inaugurate COW credentials")
Signed-off-by: Eric Biggers <ebiggers@google.com>
---
security/keys/request_key_auth.c | 67 ++++++++++++++++++----------------------
1 file changed, 30 insertions(+), 37 deletions(-)
diff --git a/security/keys/request_key_auth.c b/security/keys/request_key_auth.c
index afe9d22ab361..8f12645945ca 100644
--- a/security/keys/request_key_auth.c
+++ b/security/keys/request_key_auth.c
@@ -120,6 +120,18 @@ static void request_key_auth_revoke(struct key *key)
}
}
+static void free_request_key_auth(struct request_key_auth *rka)
+{
+ if (!rka)
+ return;
+ key_put(rka->target_key);
+ key_put(rka->dest_keyring);
+ if (rka->cred)
+ put_cred(rka->cred);
+ kfree(rka->callout_info);
+ kfree(rka);
+}
+
/*
* Destroy an instantiation authorisation token key.
*/
@@ -129,15 +141,7 @@ static void request_key_auth_destroy(struct key *key)
kenter("{%d}", key->serial);
- if (rka->cred) {
- put_cred(rka->cred);
- rka->cred = NULL;
- }
-
- key_put(rka->target_key);
- key_put(rka->dest_keyring);
- kfree(rka->callout_info);
- kfree(rka);
+ free_request_key_auth(rka);
}
/*
@@ -151,22 +155,17 @@ struct key *request_key_auth_new(struct key *target, const void *callout_info,
const struct cred *cred = current->cred;
struct key *authkey = NULL;
char desc[20];
- int ret;
+ int ret = -ENOMEM;
kenter("%d,", target->serial);
/* allocate a auth record */
- rka = kmalloc(sizeof(*rka), GFP_KERNEL);
- if (!rka) {
- kleave(" = -ENOMEM");
- return ERR_PTR(-ENOMEM);
- }
+ rka = kzalloc(sizeof(*rka), GFP_KERNEL);
+ if (!rka)
+ goto error_free_rka;
rka->callout_info = kmalloc(callout_len, GFP_KERNEL);
- if (!rka->callout_info) {
- kleave(" = -ENOMEM");
- kfree(rka);
- return ERR_PTR(-ENOMEM);
- }
+ if (!rka->callout_info)
+ goto error_free_rka;
/* see if the calling process is already servicing the key request of
* another process */
@@ -176,8 +175,12 @@ struct key *request_key_auth_new(struct key *target, const void *callout_info,
/* if the auth key has been revoked, then the key we're
* servicing is already instantiated */
- if (test_bit(KEY_FLAG_REVOKED, &cred->request_key_auth->flags))
- goto auth_key_revoked;
+ if (test_bit(KEY_FLAG_REVOKED,
+ &cred->request_key_auth->flags)) {
+ up_read(&cred->request_key_auth->sem);
+ ret = -EKEYREVOKED;
+ goto error_free_rka;
+ }
irka = cred->request_key_auth->payload.data[0];
rka->cred = get_cred(irka->cred);
@@ -205,32 +208,22 @@ struct key *request_key_auth_new(struct key *target, const void *callout_info,
KEY_USR_VIEW, KEY_ALLOC_NOT_IN_QUOTA, NULL);
if (IS_ERR(authkey)) {
ret = PTR_ERR(authkey);
- goto error_alloc;
+ goto error_free_rka;
}
/* construct the auth key */
ret = key_instantiate_and_link(authkey, rka, 0, NULL, NULL);
if (ret < 0)
- goto error_inst;
+ goto error_put_authkey;
kleave(" = {%d,%d}", authkey->serial, refcount_read(&authkey->usage));
return authkey;
-auth_key_revoked:
- up_read(&cred->request_key_auth->sem);
- kfree(rka->callout_info);
- kfree(rka);
- kleave("= -EKEYREVOKED");
- return ERR_PTR(-EKEYREVOKED);
-
-error_inst:
+error_put_authkey:
key_revoke(authkey);
key_put(authkey);
-error_alloc:
- key_put(rka->target_key);
- key_put(rka->dest_keyring);
- kfree(rka->callout_info);
- kfree(rka);
+error_free_rka:
+ free_request_key_auth(rka);
kleave("= %d", ret);
return ERR_PTR(ret);
}
--
2.14.1.821.g8fa685d3b7-goog
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH v2 1/3] KEYS: fix cred refcount leak in request_key_auth_new()
@ 2017-09-21 20:57 ` Eric Biggers
0 siblings, 0 replies; 18+ messages in thread
From: Eric Biggers @ 2017-09-21 20:57 UTC (permalink / raw)
To: keyrings
Cc: David Howells, Michael Halcrow, linux-security-module,
linux-kernel, Eric Biggers
From: Eric Biggers <ebiggers@google.com>
In request_key_auth_new(), if key_alloc() or key_instantiate_and_link()
were to fail, we would leak a reference to the 'struct cred'. Currently
this can only happen if key_alloc() fails to allocate memory. But it
still should be fixed, as it is a more severe bug waiting to happen.
Fix it by cleaning things up to use a helper function which frees a
'struct request_key_auth' correctly.
Fixes: d84f4f992cbd ("CRED: Inaugurate COW credentials")
Signed-off-by: Eric Biggers <ebiggers@google.com>
---
security/keys/request_key_auth.c | 67 ++++++++++++++++++----------------------
1 file changed, 30 insertions(+), 37 deletions(-)
diff --git a/security/keys/request_key_auth.c b/security/keys/request_key_auth.c
index afe9d22ab361..8f12645945ca 100644
--- a/security/keys/request_key_auth.c
+++ b/security/keys/request_key_auth.c
@@ -120,6 +120,18 @@ static void request_key_auth_revoke(struct key *key)
}
}
+static void free_request_key_auth(struct request_key_auth *rka)
+{
+ if (!rka)
+ return;
+ key_put(rka->target_key);
+ key_put(rka->dest_keyring);
+ if (rka->cred)
+ put_cred(rka->cred);
+ kfree(rka->callout_info);
+ kfree(rka);
+}
+
/*
* Destroy an instantiation authorisation token key.
*/
@@ -129,15 +141,7 @@ static void request_key_auth_destroy(struct key *key)
kenter("{%d}", key->serial);
- if (rka->cred) {
- put_cred(rka->cred);
- rka->cred = NULL;
- }
-
- key_put(rka->target_key);
- key_put(rka->dest_keyring);
- kfree(rka->callout_info);
- kfree(rka);
+ free_request_key_auth(rka);
}
/*
@@ -151,22 +155,17 @@ struct key *request_key_auth_new(struct key *target, const void *callout_info,
const struct cred *cred = current->cred;
struct key *authkey = NULL;
char desc[20];
- int ret;
+ int ret = -ENOMEM;
kenter("%d,", target->serial);
/* allocate a auth record */
- rka = kmalloc(sizeof(*rka), GFP_KERNEL);
- if (!rka) {
- kleave(" = -ENOMEM");
- return ERR_PTR(-ENOMEM);
- }
+ rka = kzalloc(sizeof(*rka), GFP_KERNEL);
+ if (!rka)
+ goto error_free_rka;
rka->callout_info = kmalloc(callout_len, GFP_KERNEL);
- if (!rka->callout_info) {
- kleave(" = -ENOMEM");
- kfree(rka);
- return ERR_PTR(-ENOMEM);
- }
+ if (!rka->callout_info)
+ goto error_free_rka;
/* see if the calling process is already servicing the key request of
* another process */
@@ -176,8 +175,12 @@ struct key *request_key_auth_new(struct key *target, const void *callout_info,
/* if the auth key has been revoked, then the key we're
* servicing is already instantiated */
- if (test_bit(KEY_FLAG_REVOKED, &cred->request_key_auth->flags))
- goto auth_key_revoked;
+ if (test_bit(KEY_FLAG_REVOKED,
+ &cred->request_key_auth->flags)) {
+ up_read(&cred->request_key_auth->sem);
+ ret = -EKEYREVOKED;
+ goto error_free_rka;
+ }
irka = cred->request_key_auth->payload.data[0];
rka->cred = get_cred(irka->cred);
@@ -205,32 +208,22 @@ struct key *request_key_auth_new(struct key *target, const void *callout_info,
KEY_USR_VIEW, KEY_ALLOC_NOT_IN_QUOTA, NULL);
if (IS_ERR(authkey)) {
ret = PTR_ERR(authkey);
- goto error_alloc;
+ goto error_free_rka;
}
/* construct the auth key */
ret = key_instantiate_and_link(authkey, rka, 0, NULL, NULL);
if (ret < 0)
- goto error_inst;
+ goto error_put_authkey;
kleave(" = {%d,%d}", authkey->serial, refcount_read(&authkey->usage));
return authkey;
-auth_key_revoked:
- up_read(&cred->request_key_auth->sem);
- kfree(rka->callout_info);
- kfree(rka);
- kleave("= -EKEYREVOKED");
- return ERR_PTR(-EKEYREVOKED);
-
-error_inst:
+error_put_authkey:
key_revoke(authkey);
key_put(authkey);
-error_alloc:
- key_put(rka->target_key);
- key_put(rka->dest_keyring);
- kfree(rka->callout_info);
- kfree(rka);
+error_free_rka:
+ free_request_key_auth(rka);
kleave("= %d", ret);
return ERR_PTR(ret);
}
--
2.14.1.821.g8fa685d3b7-goog
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH v2 1/3] KEYS: fix cred refcount leak in request_key_auth_new()
@ 2017-09-21 20:57 ` Eric Biggers
0 siblings, 0 replies; 18+ messages in thread
From: Eric Biggers @ 2017-09-21 20:57 UTC (permalink / raw)
To: linux-security-module
From: Eric Biggers <ebiggers@google.com>
In request_key_auth_new(), if key_alloc() or key_instantiate_and_link()
were to fail, we would leak a reference to the 'struct cred'. Currently
this can only happen if key_alloc() fails to allocate memory. But it
still should be fixed, as it is a more severe bug waiting to happen.
Fix it by cleaning things up to use a helper function which frees a
'struct request_key_auth' correctly.
Fixes: d84f4f992cbd ("CRED: Inaugurate COW credentials")
Signed-off-by: Eric Biggers <ebiggers@google.com>
---
security/keys/request_key_auth.c | 67 ++++++++++++++++++----------------------
1 file changed, 30 insertions(+), 37 deletions(-)
diff --git a/security/keys/request_key_auth.c b/security/keys/request_key_auth.c
index afe9d22ab361..8f12645945ca 100644
--- a/security/keys/request_key_auth.c
+++ b/security/keys/request_key_auth.c
@@ -120,6 +120,18 @@ static void request_key_auth_revoke(struct key *key)
}
}
+static void free_request_key_auth(struct request_key_auth *rka)
+{
+ if (!rka)
+ return;
+ key_put(rka->target_key);
+ key_put(rka->dest_keyring);
+ if (rka->cred)
+ put_cred(rka->cred);
+ kfree(rka->callout_info);
+ kfree(rka);
+}
+
/*
* Destroy an instantiation authorisation token key.
*/
@@ -129,15 +141,7 @@ static void request_key_auth_destroy(struct key *key)
kenter("{%d}", key->serial);
- if (rka->cred) {
- put_cred(rka->cred);
- rka->cred = NULL;
- }
-
- key_put(rka->target_key);
- key_put(rka->dest_keyring);
- kfree(rka->callout_info);
- kfree(rka);
+ free_request_key_auth(rka);
}
/*
@@ -151,22 +155,17 @@ struct key *request_key_auth_new(struct key *target, const void *callout_info,
const struct cred *cred = current->cred;
struct key *authkey = NULL;
char desc[20];
- int ret;
+ int ret = -ENOMEM;
kenter("%d,", target->serial);
/* allocate a auth record */
- rka = kmalloc(sizeof(*rka), GFP_KERNEL);
- if (!rka) {
- kleave(" = -ENOMEM");
- return ERR_PTR(-ENOMEM);
- }
+ rka = kzalloc(sizeof(*rka), GFP_KERNEL);
+ if (!rka)
+ goto error_free_rka;
rka->callout_info = kmalloc(callout_len, GFP_KERNEL);
- if (!rka->callout_info) {
- kleave(" = -ENOMEM");
- kfree(rka);
- return ERR_PTR(-ENOMEM);
- }
+ if (!rka->callout_info)
+ goto error_free_rka;
/* see if the calling process is already servicing the key request of
* another process */
@@ -176,8 +175,12 @@ struct key *request_key_auth_new(struct key *target, const void *callout_info,
/* if the auth key has been revoked, then the key we're
* servicing is already instantiated */
- if (test_bit(KEY_FLAG_REVOKED, &cred->request_key_auth->flags))
- goto auth_key_revoked;
+ if (test_bit(KEY_FLAG_REVOKED,
+ &cred->request_key_auth->flags)) {
+ up_read(&cred->request_key_auth->sem);
+ ret = -EKEYREVOKED;
+ goto error_free_rka;
+ }
irka = cred->request_key_auth->payload.data[0];
rka->cred = get_cred(irka->cred);
@@ -205,32 +208,22 @@ struct key *request_key_auth_new(struct key *target, const void *callout_info,
KEY_USR_VIEW, KEY_ALLOC_NOT_IN_QUOTA, NULL);
if (IS_ERR(authkey)) {
ret = PTR_ERR(authkey);
- goto error_alloc;
+ goto error_free_rka;
}
/* construct the auth key */
ret = key_instantiate_and_link(authkey, rka, 0, NULL, NULL);
if (ret < 0)
- goto error_inst;
+ goto error_put_authkey;
kleave(" = {%d,%d}", authkey->serial, refcount_read(&authkey->usage));
return authkey;
-auth_key_revoked:
- up_read(&cred->request_key_auth->sem);
- kfree(rka->callout_info);
- kfree(rka);
- kleave("= -EKEYREVOKED");
- return ERR_PTR(-EKEYREVOKED);
-
-error_inst:
+error_put_authkey:
key_revoke(authkey);
key_put(authkey);
-error_alloc:
- key_put(rka->target_key);
- key_put(rka->dest_keyring);
- kfree(rka->callout_info);
- kfree(rka);
+error_free_rka:
+ free_request_key_auth(rka);
kleave("= %d", ret);
return ERR_PTR(ret);
}
--
2.14.1.821.g8fa685d3b7-goog
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH v2 2/3] KEYS: don't revoke uninstantiated key in request_key_auth_new()
2017-09-21 20:57 ` Eric Biggers
(?)
@ 2017-09-21 20:57 ` Eric Biggers
-1 siblings, 0 replies; 18+ messages in thread
From: Eric Biggers @ 2017-09-21 20:57 UTC (permalink / raw)
To: linux-security-module
From: Eric Biggers <ebiggers@google.com>
If key_instantiate_and_link() were to fail (which fortunately isn't
possible currently), the call to key_revoke(authkey) would crash with a
NULL pointer dereference in request_key_auth_revoke() because the key
has not yet been instantiated.
Fix this by removing the call to key_revoke(). key_put() is sufficient,
as it's not possible for an uninstantiated authkey to have been used for
anything yet.
Fixes: b5f545c880a2 ("[PATCH] keys: Permit running process to instantiate keys")
Signed-off-by: Eric Biggers <ebiggers@google.com>
---
security/keys/request_key_auth.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/security/keys/request_key_auth.c b/security/keys/request_key_auth.c
index 8f12645945ca..2df92b619a38 100644
--- a/security/keys/request_key_auth.c
+++ b/security/keys/request_key_auth.c
@@ -220,7 +220,6 @@ struct key *request_key_auth_new(struct key *target, const void *callout_info,
return authkey;
error_put_authkey:
- key_revoke(authkey);
key_put(authkey);
error_free_rka:
free_request_key_auth(rka);
--
2.14.1.821.g8fa685d3b7-goog
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH v2 2/3] KEYS: don't revoke uninstantiated key in request_key_auth_new()
@ 2017-09-21 20:57 ` Eric Biggers
0 siblings, 0 replies; 18+ messages in thread
From: Eric Biggers @ 2017-09-21 20:57 UTC (permalink / raw)
To: keyrings
Cc: David Howells, Michael Halcrow, linux-security-module,
linux-kernel, Eric Biggers
From: Eric Biggers <ebiggers@google.com>
If key_instantiate_and_link() were to fail (which fortunately isn't
possible currently), the call to key_revoke(authkey) would crash with a
NULL pointer dereference in request_key_auth_revoke() because the key
has not yet been instantiated.
Fix this by removing the call to key_revoke(). key_put() is sufficient,
as it's not possible for an uninstantiated authkey to have been used for
anything yet.
Fixes: b5f545c880a2 ("[PATCH] keys: Permit running process to instantiate keys")
Signed-off-by: Eric Biggers <ebiggers@google.com>
---
security/keys/request_key_auth.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/security/keys/request_key_auth.c b/security/keys/request_key_auth.c
index 8f12645945ca..2df92b619a38 100644
--- a/security/keys/request_key_auth.c
+++ b/security/keys/request_key_auth.c
@@ -220,7 +220,6 @@ struct key *request_key_auth_new(struct key *target, const void *callout_info,
return authkey;
error_put_authkey:
- key_revoke(authkey);
key_put(authkey);
error_free_rka:
free_request_key_auth(rka);
--
2.14.1.821.g8fa685d3b7-goog
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH v2 2/3] KEYS: don't revoke uninstantiated key in request_key_auth_new()
@ 2017-09-21 20:57 ` Eric Biggers
0 siblings, 0 replies; 18+ messages in thread
From: Eric Biggers @ 2017-09-21 20:57 UTC (permalink / raw)
To: linux-security-module
From: Eric Biggers <ebiggers@google.com>
If key_instantiate_and_link() were to fail (which fortunately isn't
possible currently), the call to key_revoke(authkey) would crash with a
NULL pointer dereference in request_key_auth_revoke() because the key
has not yet been instantiated.
Fix this by removing the call to key_revoke(). key_put() is sufficient,
as it's not possible for an uninstantiated authkey to have been used for
anything yet.
Fixes: b5f545c880a2 ("[PATCH] keys: Permit running process to instantiate keys")
Signed-off-by: Eric Biggers <ebiggers@google.com>
---
security/keys/request_key_auth.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/security/keys/request_key_auth.c b/security/keys/request_key_auth.c
index 8f12645945ca..2df92b619a38 100644
--- a/security/keys/request_key_auth.c
+++ b/security/keys/request_key_auth.c
@@ -220,7 +220,6 @@ struct key *request_key_auth_new(struct key *target, const void *callout_info,
return authkey;
error_put_authkey:
- key_revoke(authkey);
key_put(authkey);
error_free_rka:
free_request_key_auth(rka);
--
2.14.1.821.g8fa685d3b7-goog
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH v2 3/3] KEYS: use kmemdup() in request_key_auth_new()
2017-09-21 20:57 ` Eric Biggers
(?)
@ 2017-09-21 20:57 ` Eric Biggers
-1 siblings, 0 replies; 18+ messages in thread
From: Eric Biggers @ 2017-09-21 20:57 UTC (permalink / raw)
To: linux-security-module
From: Eric Biggers <ebiggers@google.com>
kmemdup() is preferred to kmalloc() followed by memcpy().
Signed-off-by: Eric Biggers <ebiggers@google.com>
---
security/keys/request_key_auth.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/security/keys/request_key_auth.c b/security/keys/request_key_auth.c
index 2df92b619a38..9d536f9e5222 100644
--- a/security/keys/request_key_auth.c
+++ b/security/keys/request_key_auth.c
@@ -163,9 +163,10 @@ struct key *request_key_auth_new(struct key *target, const void *callout_info,
rka = kzalloc(sizeof(*rka), GFP_KERNEL);
if (!rka)
goto error_free_rka;
- rka->callout_info = kmalloc(callout_len, GFP_KERNEL);
+ rka->callout_info = kmemdup(callout_info, callout_len, GFP_KERNEL);
if (!rka->callout_info)
goto error_free_rka;
+ rka->callout_len = callout_len;
/* see if the calling process is already servicing the key request of
* another process */
@@ -196,8 +197,6 @@ struct key *request_key_auth_new(struct key *target, const void *callout_info,
rka->target_key = key_get(target);
rka->dest_keyring = key_get(dest_keyring);
- memcpy(rka->callout_info, callout_info, callout_len);
- rka->callout_len = callout_len;
/* allocate the auth key */
sprintf(desc, "%x", target->serial);
--
2.14.1.821.g8fa685d3b7-goog
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH v2 3/3] KEYS: use kmemdup() in request_key_auth_new()
@ 2017-09-21 20:57 ` Eric Biggers
0 siblings, 0 replies; 18+ messages in thread
From: Eric Biggers @ 2017-09-21 20:57 UTC (permalink / raw)
To: keyrings
Cc: David Howells, Michael Halcrow, linux-security-module,
linux-kernel, Eric Biggers
From: Eric Biggers <ebiggers@google.com>
kmemdup() is preferred to kmalloc() followed by memcpy().
Signed-off-by: Eric Biggers <ebiggers@google.com>
---
security/keys/request_key_auth.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/security/keys/request_key_auth.c b/security/keys/request_key_auth.c
index 2df92b619a38..9d536f9e5222 100644
--- a/security/keys/request_key_auth.c
+++ b/security/keys/request_key_auth.c
@@ -163,9 +163,10 @@ struct key *request_key_auth_new(struct key *target, const void *callout_info,
rka = kzalloc(sizeof(*rka), GFP_KERNEL);
if (!rka)
goto error_free_rka;
- rka->callout_info = kmalloc(callout_len, GFP_KERNEL);
+ rka->callout_info = kmemdup(callout_info, callout_len, GFP_KERNEL);
if (!rka->callout_info)
goto error_free_rka;
+ rka->callout_len = callout_len;
/* see if the calling process is already servicing the key request of
* another process */
@@ -196,8 +197,6 @@ struct key *request_key_auth_new(struct key *target, const void *callout_info,
rka->target_key = key_get(target);
rka->dest_keyring = key_get(dest_keyring);
- memcpy(rka->callout_info, callout_info, callout_len);
- rka->callout_len = callout_len;
/* allocate the auth key */
sprintf(desc, "%x", target->serial);
--
2.14.1.821.g8fa685d3b7-goog
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH v2 3/3] KEYS: use kmemdup() in request_key_auth_new()
@ 2017-09-21 20:57 ` Eric Biggers
0 siblings, 0 replies; 18+ messages in thread
From: Eric Biggers @ 2017-09-21 20:57 UTC (permalink / raw)
To: linux-security-module
From: Eric Biggers <ebiggers@google.com>
kmemdup() is preferred to kmalloc() followed by memcpy().
Signed-off-by: Eric Biggers <ebiggers@google.com>
---
security/keys/request_key_auth.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/security/keys/request_key_auth.c b/security/keys/request_key_auth.c
index 2df92b619a38..9d536f9e5222 100644
--- a/security/keys/request_key_auth.c
+++ b/security/keys/request_key_auth.c
@@ -163,9 +163,10 @@ struct key *request_key_auth_new(struct key *target, const void *callout_info,
rka = kzalloc(sizeof(*rka), GFP_KERNEL);
if (!rka)
goto error_free_rka;
- rka->callout_info = kmalloc(callout_len, GFP_KERNEL);
+ rka->callout_info = kmemdup(callout_info, callout_len, GFP_KERNEL);
if (!rka->callout_info)
goto error_free_rka;
+ rka->callout_len = callout_len;
/* see if the calling process is already servicing the key request of
* another process */
@@ -196,8 +197,6 @@ struct key *request_key_auth_new(struct key *target, const void *callout_info,
rka->target_key = key_get(target);
rka->dest_keyring = key_get(dest_keyring);
- memcpy(rka->callout_info, callout_info, callout_len);
- rka->callout_len = callout_len;
/* allocate the auth key */
sprintf(desc, "%x", target->serial);
--
2.14.1.821.g8fa685d3b7-goog
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH v2 2/3] KEYS: don't revoke uninstantiated key in request_key_auth_new()
2017-09-21 20:57 ` Eric Biggers
(?)
@ 2017-09-25 14:14 ` David Howells
-1 siblings, 0 replies; 18+ messages in thread
From: David Howells @ 2017-09-25 14:14 UTC (permalink / raw)
To: linux-security-module
Eric Biggers <ebiggers3@gmail.com> wrote:
> Fix this by removing the call to key_revoke(). key_put() is sufficient,
> as it's not possible for an uninstantiated authkey to have been used for
> anything yet.
Note that the owner can call keyctl_describe() upon it, though that shouldn't
wait because KEY_LOOKUP_PARTIAL would be specified.
However, this is probably fair enough, and it should be gc'd quickly because
it'll lose its last ref.
David
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2 2/3] KEYS: don't revoke uninstantiated key in request_key_auth_new()
@ 2017-09-25 14:14 ` David Howells
0 siblings, 0 replies; 18+ messages in thread
From: David Howells @ 2017-09-25 14:14 UTC (permalink / raw)
To: Eric Biggers
Cc: dhowells, keyrings, Michael Halcrow, linux-security-module,
linux-kernel, Eric Biggers
Eric Biggers <ebiggers3@gmail.com> wrote:
> Fix this by removing the call to key_revoke(). key_put() is sufficient,
> as it's not possible for an uninstantiated authkey to have been used for
> anything yet.
Note that the owner can call keyctl_describe() upon it, though that shouldn't
wait because KEY_LOOKUP_PARTIAL would be specified.
However, this is probably fair enough, and it should be gc'd quickly because
it'll lose its last ref.
David
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH v2 2/3] KEYS: don't revoke uninstantiated key in request_key_auth_new()
@ 2017-09-25 14:14 ` David Howells
0 siblings, 0 replies; 18+ messages in thread
From: David Howells @ 2017-09-25 14:14 UTC (permalink / raw)
To: linux-security-module
Eric Biggers <ebiggers3@gmail.com> wrote:
> Fix this by removing the call to key_revoke(). key_put() is sufficient,
> as it's not possible for an uninstantiated authkey to have been used for
> anything yet.
Note that the owner can call keyctl_describe() upon it, though that shouldn't
wait because KEY_LOOKUP_PARTIAL would be specified.
However, this is probably fair enough, and it should be gc'd quickly because
it'll lose its last ref.
David
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2 1/3] KEYS: fix cred refcount leak in request_key_auth_new()
2017-09-21 20:57 ` Eric Biggers
(?)
@ 2017-09-25 14:18 ` David Howells
-1 siblings, 0 replies; 18+ messages in thread
From: David Howells @ 2017-09-25 14:18 UTC (permalink / raw)
To: linux-security-module
Eric Biggers <ebiggers3@gmail.com> wrote:
> + rka = kzalloc(sizeof(*rka), GFP_KERNEL);
> + if (!rka)
> + goto error_free_rka;
But you haven't allocated an rka if you're in the error path. I'll fix that
to jump over the release.
David
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2 1/3] KEYS: fix cred refcount leak in request_key_auth_new()
@ 2017-09-25 14:18 ` David Howells
0 siblings, 0 replies; 18+ messages in thread
From: David Howells @ 2017-09-25 14:18 UTC (permalink / raw)
To: Eric Biggers
Cc: dhowells, keyrings, Michael Halcrow, linux-security-module,
linux-kernel, Eric Biggers
Eric Biggers <ebiggers3@gmail.com> wrote:
> + rka = kzalloc(sizeof(*rka), GFP_KERNEL);
> + if (!rka)
> + goto error_free_rka;
But you haven't allocated an rka if you're in the error path. I'll fix that
to jump over the release.
David
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH v2 1/3] KEYS: fix cred refcount leak in request_key_auth_new()
@ 2017-09-25 14:18 ` David Howells
0 siblings, 0 replies; 18+ messages in thread
From: David Howells @ 2017-09-25 14:18 UTC (permalink / raw)
To: linux-security-module
Eric Biggers <ebiggers3@gmail.com> wrote:
> + rka = kzalloc(sizeof(*rka), GFP_KERNEL);
> + if (!rka)
> + goto error_free_rka;
But you haven't allocated an rka if you're in the error path. I'll fix that
to jump over the release.
David
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 18+ messages in thread
end of thread, other threads:[~2017-09-25 14:19 UTC | newest]
Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-09-21 20:57 [PATCH v2 0/3] KEYS: fix error handling in request_key_auth_new() Eric Biggers
2017-09-21 20:57 ` Eric Biggers
2017-09-21 20:57 ` Eric Biggers
2017-09-21 20:57 ` [PATCH v2 1/3] KEYS: fix cred refcount leak " Eric Biggers
2017-09-21 20:57 ` Eric Biggers
2017-09-21 20:57 ` Eric Biggers
2017-09-21 20:57 ` [PATCH v2 2/3] KEYS: don't revoke uninstantiated key " Eric Biggers
2017-09-21 20:57 ` Eric Biggers
2017-09-21 20:57 ` Eric Biggers
2017-09-21 20:57 ` [PATCH v2 3/3] KEYS: use kmemdup() " Eric Biggers
2017-09-21 20:57 ` Eric Biggers
2017-09-21 20:57 ` Eric Biggers
2017-09-25 14:14 ` [PATCH v2 2/3] KEYS: don't revoke uninstantiated key " David Howells
2017-09-25 14:14 ` David Howells
2017-09-25 14:14 ` David Howells
2017-09-25 14:18 ` [PATCH v2 1/3] KEYS: fix cred refcount leak " David Howells
2017-09-25 14:18 ` David Howells
2017-09-25 14:18 ` David Howells
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.