| Message ID | 1620467481-110575-1-git-send-email-yang.lee@linux.alibaba.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Keys: Remove redundant initialization of cred | expand |
On Sat, May 08, 2021 at 05:51:21PM +0800, Yang Li wrote: > Pointer cred is being initialized however this value is never > read as cred is assigned an updated value from the returned > call to get_current_cred(). Remove the redundant initialization. > > Cleans up clang warning: > > security/keys/request_key.c:119:21: warning: Value stored to 'cred' > during its initialization is never read > [clang-analyzer-deadcode.DeadStores] > > Reported-by: Abaci Robot <abaci@linux.alibaba.com> > Fixes: 'commit bb952bb98a7e ("CRED: Separate per-task-group keyrings from signal_struct")' > Signed-off-by: Yang Li <yang.lee@linux.alibaba.com> > --- > security/keys/request_key.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/security/keys/request_key.c b/security/keys/request_key.c > index 2da4404..873c31f 100644 > --- a/security/keys/request_key.c > +++ b/security/keys/request_key.c > @@ -116,7 +116,7 @@ static int call_sbin_request_key(struct key *authkey, void *aux) > { > static char const request_key[] = "/sbin/request-key"; > struct request_key_auth *rka = get_request_key_auth(authkey); > - const struct cred *cred = current_cred(); > + const struct cred *cred; > key_serial_t prkey, sskey; > struct key *key = rka->target_key, *keyring, *session, *user_session; > char *argv[9], *envp[3], uid_str[12], gid_str[12]; > -- > 1.8.3.1 > > Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org> /Jarkko
Yang Li <yang.lee@linux.alibaba.com> wrote: > - const struct cred *cred = current_cred(); > + const struct cred *cred; Good catch, but it's probably the wrong fix. In that function, there is: const struct cred *cred = current_cred(); ... cred = get_current_cred(); keyring = keyring_alloc(desc, cred->fsuid, cred->fsgid, cred, ...); put_cred(cred); ... sprintf(uid_str, "%d", from_kuid(&init_user_ns, cred->fsuid)); ... So we get the creds again, but with a ref, and then drop after calling keyring_alloc()... and then access cred again, which is dodgy - but we get away with it because cred is still pinned by our task_struct. I think what is actually needed is to remove the get_current_cred() and the put_cred() calls, in which case, you want this: Fixes: d84f4f992cbd ("CRED: Inaugurate COW credentials") David
diff --git a/security/keys/request_key.c b/security/keys/request_key.c index 2da4404..873c31f 100644 --- a/security/keys/request_key.c +++ b/security/keys/request_key.c @@ -116,7 +116,7 @@ static int call_sbin_request_key(struct key *authkey, void *aux) { static char const request_key[] = "/sbin/request-key"; struct request_key_auth *rka = get_request_key_auth(authkey); - const struct cred *cred = current_cred(); + const struct cred *cred; key_serial_t prkey, sskey; struct key *key = rka->target_key, *keyring, *session, *user_session; char *argv[9], *envp[3], uid_str[12], gid_str[12];
Pointer cred is being initialized however this value is never read as cred is assigned an updated value from the returned call to get_current_cred(). Remove the redundant initialization. Cleans up clang warning: security/keys/request_key.c:119:21: warning: Value stored to 'cred' during its initialization is never read [clang-analyzer-deadcode.DeadStores] Reported-by: Abaci Robot <abaci@linux.alibaba.com> Fixes: 'commit bb952bb98a7e ("CRED: Separate per-task-group keyrings from signal_struct")' Signed-off-by: Yang Li <yang.lee@linux.alibaba.com> --- security/keys/request_key.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)