• Tycho Andersen's avatar
    seccomp: Make NEW_LISTENER and TSYNC flags exclusive · 7a0df7fb
    Tycho Andersen authored
    As the comment notes, the return codes for TSYNC and NEW_LISTENER
    conflict, because they both return positive values, one in the case of
    success and one in the case of error. So, let's disallow both of these
    flags together.
    
    While this is technically a userspace break, all the users I know
    of are still waiting on me to land this feature in libseccomp, so I
    think it'll be safe. Also, at present my use case doesn't require
    TSYNC at all, so this isn't a big deal to disallow. If someone
    wanted to support this, a path forward would be to add a new flag like
    TSYNC_AND_LISTENER_YES_I_UNDERSTAND_THAT_TSYNC_WILL_JUST_RETURN_EAGAIN,
    but the use cases are so different I don't see it really happening.
    
    Finally, it's worth noting that this does actually fix a UAF issue: at the
    end of seccomp_set_mode_filter(), we have:
    
            if (flags & SECCOMP_FILTER_FLAG_NEW_LISTENER) {
                    if (ret < 0) {
                            listener_f->private_data = NULL;
                            fput(listener_f);
                            put_unused_fd(listener);
                    } else {
                            fd_install(listener, listener_f);
                            ret = listener;
                    }
            }
    out_free:
            seccomp_filter_free(prepared);
    
    But if ret > 0 because TSYNC raced, we'll install the listener fd and then
    free the filter out from underneath it, causing a UAF when the task closes
    it or dies. This patch also switches the condition to be simply if (ret),
    so that if someone does add the flag mentioned above, they won't have to
    remember to fix this too.
    
    Reported-by: syzbot+b562969adb2e04af3442@syzkaller.appspotmail.com
    Fixes: 6a21cc50 ("seccomp: add a return code to trap to userspace")
    CC: stable@vger.kernel.org # v5.0+
    Signed-off-by: default avatarTycho Andersen <tycho@tycho.ws>
    Signed-off-by: default avatarKees Cook <keescook@chromium.org>
    Acked-by: default avatarJames Morris <jamorris@linux.microsoft.com>
    7a0df7fb
seccomp.c 45.6 KB