linux-kernel - Re: [PATCH v3 3/3] selftests/seccomp: Test kernel catches garbage on SECCOMP_IOCTL_NOTIF

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <201912301023.CEBE3A5@keescook>
Date:   Mon, 30 Dec 2019 10:29:13 -0800
From:   Kees Cook <keescook@...omium.org>
To:     Sargun Dhillon <sargun@...gun.me>
Cc:     Christian Brauner <christian.brauner@...ntu.com>,
        LKML <linux-kernel@...r.kernel.org>,
        Linux API <linux-api@...r.kernel.org>,
        Jann Horn <jannh@...gle.com>, Aleksa Sarai <cyphar@...har.com>,
        Tycho Andersen <tycho@...ho.ws>
Subject: Re: [PATCH v3 3/3] selftests/seccomp: Test kernel catches garbage on
 SECCOMP_IOCTL_NOTIF_RECV

On Sun, Dec 29, 2019 at 03:42:17PM -0800, Sargun Dhillon wrote:
> On Sun, Dec 29, 2019 at 11:43 AM Christian Brauner
> <christian.brauner@...ntu.com> wrote:
> >
> > On Sun, Dec 29, 2019 at 11:06:25AM -0800, Sargun Dhillon wrote:
> > > On Sun, Dec 29, 2019 at 12:14 PM Christian Brauner
> > > <christian.brauner@...ntu.com> wrote:
> > > > Does that even work if no dup() syscall has been made and trapped?
> > > Yes, the first check that occurs is the check which checks if
> > > seccom_notif has been
> > > zeroed out. This happens before any of the other work.
> >
> > Ah, then sure I don't mind doing it this way. Though plumbing it
> > directly into TEST(user_notification_basic) like I did below seems
> > cleaner to me.
> >
> > >
> > > > This looks like it would give you ENOENT...
> > > This ioctl is a blocking ioctl. It'll block until there is a wakeup.
> > > In this case, the wakeup
> > > will never come, but that doesn't mean we get an ENOENT.
> >
> > Yeah, but that wold mean the test will hang weirdly if it bypasses the
> > check. Sure it'll timeout but meh. I think I would prefer to have this
> > done as part of the basic test where we know that there is an event but
> > _shrug_.

I'd like to design the tests to not _depend_ on the timeout to catch bad
behaviors. The timeout is there for us to not break a test runner when
we forget weird corner cases.

> My one worry about this is that the behaviour should be if the input
> (seccomp_notif) is invalid, it should immediately bail out, whether
> or not there is an event waiting. If we add it to basic_test, then
> it would hide the erroneous behaviour if bailout isn't immediate.

I'm not following this paragraph. The ioctl's zero check is immediate,
so this test should fail the EXPECT (but continue to the next "correct"
ioctl) -- it's not an ASSERT (which would stop the test). I think
Christian's test might be a cleaner approach, unless I'm still missing
something here?

-- 
Kees Cook