On Wed, Mar 06, 2019 at 10:02:25PM +0100, Christian Brauner wrote: > On Wed, Mar 6, 2019 at 9:46 PM Tycho Andersen <ty...@tycho.ws> wrote: > > > > On Wed, Mar 06, 2019 at 09:39:35PM +0100, Christian Brauner wrote: > > > > + > > > > /* Prepare the new filter before holding any locks. */ > > > > prepared = seccomp_prepare_user_filter(filter); > > > > if (IS_ERR(prepared)) > > > > @@ -1302,7 +1315,7 @@ static long seccomp_set_mode_filter(unsigned int > > > > flags, > > > > mutex_unlock(¤t->signal->cred_guard_mutex); > > > > out_put_fd: > > > > if (flags & SECCOMP_FILTER_FLAG_NEW_LISTENER) { > > > > - if (ret < 0) { > > > > + if (ret) { > > > > > > Why that change but keep checking if (ret < 0) further up? > > > > Not sure what you mean here. The only other place I see that we check > > something is < 0 in that function is the return value of > > get_unused_fd_flags(), which looks right to me? > > The change just seemed it had nothing to do with the rest of the patch. > Just making sure this didn't happen on accident and would cause regressions.
No, not on accident :). See the second half of the patch notes. I can split it out into two separate patches if that makes more sense. In fact this hunk alone fixes the UAF, but you still get non-sensical return results even if it doesn't do anything terrible, hence the first hunk. Cheers, Tycho