On Wed, Sep 02, 2020 at 08:09:53AM -0600, Tycho Andersen wrote: > 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 <[email protected]> > Reported-by: Christian Brauner <[email protected]> > Signed-off-by: Tycho Andersen <[email protected]> > ---
Thanks for the patch, Tycho! Looks simple enough to me: Acked-by: Christian Brauner <[email protected]> > 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) I think an explicit "inline" coldn't hurt but it's fine either way and no need to resend imho. Probably Kees can just add this when applying if we reall cared. > +{ > + 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 >
