All of lore.kernel.org
 help / color / mirror / Atom feed
From: Tycho Andersen <tycho@tycho.pizza>
To: Kees Cook <keescook@chromium.org>
Cc: linux-kernel@vger.kernel.org,
	Christian Brauner <christian@brauner.io>,
	Tycho Andersen <tycho@tycho.pizza>,
	Christian Brauner <christian.brauner@ubuntu.com>
Subject: [PATCH] seccomp: don't leave dangling ->notif if file allocation fails
Date: Wed,  2 Sep 2020 08:09:53 -0600	[thread overview]
Message-ID: <20200902140953.1201956-1-tycho@tycho.pizza> (raw)

Christian and Kees both pointed out that this is a bit sloppy to open-code
both places, and Christian points out that we leave a dangling pointer to
->notif if file allocation fails. Since we check ->notif for null in order
to determine if it's ok to install a filter, this means people won't be
able to install a filter if the file allocation fails for some reason, even
if they subsequently should be able to.

To fix this, let's hoist this free+null into its own little helper and use
it.

Reported-by: Kees Cook <keescook@chromium.org>
Reported-by: Christian Brauner <christian.brauner@ubuntu.com>
Signed-off-by: Tycho Andersen <tycho@tycho.pizza>
---
 kernel/seccomp.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/kernel/seccomp.c b/kernel/seccomp.c
index bb0dd9ae699a..676d4af62103 100644
--- a/kernel/seccomp.c
+++ b/kernel/seccomp.c
@@ -1109,6 +1109,12 @@ static long seccomp_set_mode_strict(void)
 }
 
 #ifdef CONFIG_SECCOMP_FILTER
+static void seccomp_notify_free(struct seccomp_filter *filter)
+{
+	kfree(filter->notif);
+	filter->notif = NULL;
+}
+
 static void seccomp_notify_detach(struct seccomp_filter *filter)
 {
 	struct seccomp_knotif *knotif;
@@ -1138,8 +1144,7 @@ static void seccomp_notify_detach(struct seccomp_filter *filter)
 		complete(&knotif->ready);
 	}
 
-	kfree(filter->notif);
-	filter->notif = NULL;
+	seccomp_notify_free(filter);
 	mutex_unlock(&filter->notify_lock);
 }
 
@@ -1494,7 +1499,7 @@ static struct file *init_listener(struct seccomp_filter *filter)
 
 out_notif:
 	if (IS_ERR(ret))
-		kfree(filter->notif);
+		seccomp_notify_free(filter);
 out:
 	return ret;
 }

base-commit: 7b6aa0bb62fd6fd50f2d14136136262d28fb2dfe
-- 
2.25.1


             reply	other threads:[~2020-09-02 14:53 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-09-02 14:09 Tycho Andersen [this message]
2020-09-02 14:22 ` [PATCH] seccomp: don't leave dangling ->notif if file allocation fails Christian Brauner
2020-09-08 18:41 ` Kees Cook

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200902140953.1201956-1-tycho@tycho.pizza \
    --to=tycho@tycho.pizza \
    --cc=christian.brauner@ubuntu.com \
    --cc=christian@brauner.io \
    --cc=keescook@chromium.org \
    --cc=linux-kernel@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.