Skip to content

Commit

Permalink
cred: Do not default to init_cred in prepare_kernel_cred()
Browse files Browse the repository at this point in the history
A common exploit pattern for ROP attacks is to abuse prepare_kernel_cred()
in order to construct escalated privileges[1]. Instead of providing a
short-hand argument (NULL) to the "daemon" argument to indicate using
init_cred as the base cred, require that "daemon" is always set to
an actual task. Replace all existing callers that were passing NULL
with &init_task.

Future attacks will need to have sufficiently powerful read/write
primitives to have found an appropriately privileged task and written it
to the ROP stack as an argument to succeed, which is similarly difficult
to the prior effort needed to escalate privileges before struct cred
existed: locate the current cred and overwrite the uid member.

This has the added benefit of meaning that prepare_kernel_cred() can no
longer exceed the privileges of the init task, which may have changed from
the original init_cred (e.g. dropping capabilities from the bounding set).

[1] https://google.com/search?q=commit_creds(prepare_kernel_cred(0))

Cc: "Eric W. Biederman" <ebiederm@xmission.com>
Cc: David Howells <dhowells@redhat.com>
Cc: "Rafael J. Wysocki" <rafael@kernel.org>
Cc: Steve French <sfrench@samba.org>
Cc: Ronnie Sahlberg <lsahlber@redhat.com>
Cc: Shyam Prasad N <sprasad@microsoft.com>
Cc: Tom Talpey <tom@talpey.com>
Cc: Namjae Jeon <linkinjeon@kernel.org>
Cc: Trond Myklebust <trond.myklebust@hammerspace.com>
Cc: Anna Schumaker <anna@kernel.org>
Cc: Chuck Lever <chuck.lever@oracle.com>
Cc: Jeff Layton <jlayton@kernel.org>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Eric Dumazet <edumazet@google.com>
Cc: Jakub Kicinski <kuba@kernel.org>
Cc: Paolo Abeni <pabeni@redhat.com>
Cc: "Michal Koutný" <mkoutny@suse.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: linux-cifs@vger.kernel.org
Cc: samba-technical@lists.samba.org
Cc: linux-nfs@vger.kernel.org
Cc: netdev@vger.kernel.org
Signed-off-by: Kees Cook <keescook@chromium.org>
Acked-by: Luis Chamberlain <mcgrof@kernel.org>
Reviewed-by: Sergey Senozhatsky <senozhatsky@chromium.org>
Acked-by: Russ Weight <russell.h.weight@intel.com>
Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Acked-by: Paulo Alcantara (SUSE) <pc@cjr.nz>
Link: https://lore.kernel.org/r/20221026232943.never.775-kees@kernel.org
  • Loading branch information
kees committed Nov 1, 2022
1 parent e9a40e1 commit 5a17f04
Show file tree
Hide file tree
Showing 9 changed files with 16 additions and 17 deletions.
2 changes: 1 addition & 1 deletion drivers/base/firmware_loader/main.c
Original file line number Diff line number Diff line change
Expand Up @@ -821,7 +821,7 @@ _request_firmware(const struct firmware **firmware_p, const char *name,
* called by a driver when serving an unrelated request from userland, we use
* the kernel credentials to read the file.
*/
kern_cred = prepare_kernel_cred(NULL);
kern_cred = prepare_kernel_cred(&init_task);
if (!kern_cred) {
ret = -ENOMEM;
goto out;
Expand Down
2 changes: 1 addition & 1 deletion fs/cifs/cifs_spnego.c
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,7 @@ init_cifs_spnego(void)
* spnego upcalls.
*/

cred = prepare_kernel_cred(NULL);
cred = prepare_kernel_cred(&init_task);
if (!cred)
return -ENOMEM;

Expand Down
2 changes: 1 addition & 1 deletion fs/cifs/cifsacl.c
Original file line number Diff line number Diff line change
Expand Up @@ -465,7 +465,7 @@ init_cifs_idmap(void)
* this is used to prevent malicious redirections from being installed
* with add_key().
*/
cred = prepare_kernel_cred(NULL);
cred = prepare_kernel_cred(&init_task);
if (!cred)
return -ENOMEM;

Expand Down
2 changes: 1 addition & 1 deletion fs/ksmbd/smb_common.c
Original file line number Diff line number Diff line change
Expand Up @@ -623,7 +623,7 @@ int ksmbd_override_fsids(struct ksmbd_work *work)
if (share->force_gid != KSMBD_SHARE_INVALID_GID)
gid = share->force_gid;

cred = prepare_kernel_cred(NULL);
cred = prepare_kernel_cred(&init_task);
if (!cred)
return -ENOMEM;

Expand Down
4 changes: 2 additions & 2 deletions fs/nfs/flexfilelayout/flexfilelayout.c
Original file line number Diff line number Diff line change
Expand Up @@ -493,10 +493,10 @@ ff_layout_alloc_lseg(struct pnfs_layout_hdr *lh,
gid = make_kgid(&init_user_ns, id);

if (gfp_flags & __GFP_FS)
kcred = prepare_kernel_cred(NULL);
kcred = prepare_kernel_cred(&init_task);
else {
unsigned int nofs_flags = memalloc_nofs_save();
kcred = prepare_kernel_cred(NULL);
kcred = prepare_kernel_cred(&init_task);
memalloc_nofs_restore(nofs_flags);
}
rc = -ENOMEM;
Expand Down
2 changes: 1 addition & 1 deletion fs/nfs/nfs4idmap.c
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,7 @@ int nfs_idmap_init(void)
printk(KERN_NOTICE "NFS: Registering the %s key type\n",
key_type_id_resolver.name);

cred = prepare_kernel_cred(NULL);
cred = prepare_kernel_cred(&init_task);
if (!cred)
return -ENOMEM;

Expand Down
2 changes: 1 addition & 1 deletion fs/nfsd/nfs4callback.c
Original file line number Diff line number Diff line change
Expand Up @@ -870,7 +870,7 @@ static const struct cred *get_backchannel_cred(struct nfs4_client *clp, struct r
} else {
struct cred *kcred;

kcred = prepare_kernel_cred(NULL);
kcred = prepare_kernel_cred(&init_task);
if (!kcred)
return NULL;

Expand Down
15 changes: 7 additions & 8 deletions kernel/cred.c
Original file line number Diff line number Diff line change
Expand Up @@ -701,9 +701,9 @@ void __init cred_init(void)
* override a task's own credentials so that work can be done on behalf of that
* task that requires a different subjective context.
*
* @daemon is used to provide a base for the security record, but can be NULL.
* If @daemon is supplied, then the security data will be derived from that;
* otherwise they'll be set to 0 and no groups, full capabilities and no keys.
* @daemon is used to provide a base cred, with the security data derived from
* that; if this is "&init_task", they'll be set to 0, no groups, full
* capabilities, and no keys.
*
* The caller may change these controls afterwards if desired.
*
Expand All @@ -714,17 +714,16 @@ struct cred *prepare_kernel_cred(struct task_struct *daemon)
const struct cred *old;
struct cred *new;

if (WARN_ON_ONCE(!daemon))
return NULL;

new = kmem_cache_alloc(cred_jar, GFP_KERNEL);
if (!new)
return NULL;

kdebug("prepare_kernel_cred() alloc %p", new);

if (daemon)
old = get_task_cred(daemon);
else
old = get_cred(&init_cred);

old = get_task_cred(daemon);
validate_creds(old);

*new = *old;
Expand Down
2 changes: 1 addition & 1 deletion net/dns_resolver/dns_key.c
Original file line number Diff line number Diff line change
Expand Up @@ -337,7 +337,7 @@ static int __init init_dns_resolver(void)
* this is used to prevent malicious redirections from being installed
* with add_key().
*/
cred = prepare_kernel_cred(NULL);
cred = prepare_kernel_cred(&init_task);
if (!cred)
return -ENOMEM;

Expand Down

0 comments on commit 5a17f04

Please sign in to comment.