Re: [PATCH v3] cred: Propagate security_prepare_creds() error code
From: Eric Biggers <ebiggers@kernel.org>
Date: 2022-06-09 23:18:59
Also in:
keyrings, linux-cifs, linux-doc, linux-fsdevel, linux-mm, linux-nfs, linux-unionfs, lkml, netdev, selinux
On Wed, Jun 08, 2022 at 10:09:42AM -0500, Frederick Lawler wrote:
quoted hunk ↗ jump to hunk
diff --git a/fs/aio.c b/fs/aio.c index 3c249b938632..5abbe88c3ca7 100644 --- a/fs/aio.c +++ b/fs/aio.c@@ -1620,6 +1620,8 @@ static void aio_fsync_work(struct work_struct *work) static int aio_fsync(struct fsync_iocb *req, const struct iocb *iocb, bool datasync) { + int err; + if (unlikely(iocb->aio_buf || iocb->aio_offset || iocb->aio_nbytes || iocb->aio_rw_flags)) return -EINVAL;@@ -1628,8 +1630,11 @@ static int aio_fsync(struct fsync_iocb *req, const struct iocb *iocb, return -EINVAL; req->creds = prepare_creds(); - if (!req->creds) - return -ENOMEM; + if (IS_ERR(req->creds)) { + err = PTR_ERR(req->creds); + req->creds = NULL; + return err; + }
This part is a little ugly. How about doing: creds = prepare_creds(); if (IS_ERR(creds)) return PTR_ERR(creds); req->creds = creds;
quoted hunk ↗ jump to hunk
diff --git a/fs/exec.c b/fs/exec.c index 0989fb8472a1..02624783e40e 100644 --- a/fs/exec.c +++ b/fs/exec.c@@ -1468,15 +1468,19 @@ EXPORT_SYMBOL(finalize_exec); */ static int prepare_bprm_creds(struct linux_binprm *bprm) { + int err = -ERESTARTNOINTR; if (mutex_lock_interruptible(¤t->signal->cred_guard_mutex)) - return -ERESTARTNOINTR; + return err; bprm->cred = prepare_exec_creds(); - if (likely(bprm->cred)) - return 0; + if (IS_ERR(bprm->cred)) { + err = PTR_ERR(bprm->cred); + bprm->cred = NULL; + mutex_unlock(¤t->signal->cred_guard_mutex); + return err; + } - mutex_unlock(¤t->signal->cred_guard_mutex); - return -ENOMEM; + return 0; }
Similarly:
static int prepare_bprm_creds(struct linux_binprm *bprm)
{
struct cred *cred;
if (mutex_lock_interruptible(¤t->signal->cred_guard_mutex))
return -ERESTARTNOINTR;
cred = prepare_exec_creds();
if (IS_ERR(cred)) {
mutex_unlock(¤t->signal->cred_guard_mutex);
return PTR_ERR(cred);
}
bprm->cred = cred;
return 0;
}
quoted hunk ↗ jump to hunk
diff --git a/kernel/nsproxy.c b/kernel/nsproxy.c index eec72ca962e2..6cf75aa83b6c 100644 --- a/kernel/nsproxy.c +++ b/kernel/nsproxy.c@@ -311,6 +311,7 @@ static void put_nsset(struct nsset *nsset) static int prepare_nsset(unsigned flags, struct nsset *nsset) { + int err = -ENOMEM; struct task_struct *me = current; nsset->nsproxy = create_new_namespaces(0, me, current_user_ns(), me->fs);@@ -324,6 +325,12 @@ static int prepare_nsset(unsigned flags, struct nsset *nsset) if (!nsset->cred) goto out; + if (IS_ERR(nsset->cred)) { + err = PTR_ERR(nsset->cred); + nsset->cred = NULL; + goto out; + }
Why is the NULL check above being kept? Also, drivers/crypto/ccp/sev-dev.c needs to be updated. - Eric