From: Tycho Andersen <tycho@tycho.ws>
To: Sargun Dhillon <sargun@sargun.me>
Cc: LKML <linux-kernel@vger.kernel.org>,
Linux API <linux-api@vger.kernel.org>,
Jann Horn <jannh@google.com>,
Christian Brauner <christian.brauner@ubuntu.com>,
Kees Cook <keescook@chromium.org>,
Aleksa Sarai <cyphar@cyphar.com>
Subject: Re: [PATCH v2 1/2] samples, selftests/seccomp: Zero out seccomp_notif
Date: Sat, 28 Dec 2019 17:18:18 -0700 [thread overview]
Message-ID: <20191229001818.GC6746@cisco> (raw)
In-Reply-To: <CAMp4zn91GoB=1eTbc_ux4eNs2-QFm+JocodgFQYUiiXL7H4m9w@mail.gmail.com>
On Sat, Dec 28, 2019 at 07:10:29PM -0500, Sargun Dhillon wrote:
> On Sat, Dec 28, 2019 at 1:18 PM Tycho Andersen <tycho@tycho.ws> wrote:
> >
> > On Sat, Dec 28, 2019 at 01:48:39AM +0000, Sargun Dhillon wrote:
> > > The seccomp_notif structure should be zeroed out prior to calling the
> > > SECCOMP_IOCTL_NOTIF_RECV ioctl. Previously, the kernel did not check
> > > whether these structures were zeroed out or not, so these worked.
> > >
> > > Signed-off-by: Sargun Dhillon <sargun@sargun.me>
> > > Cc: Kees Cook <keescook@chromium.org>
> > > ---
> > > samples/seccomp/user-trap.c | 2 +-
> > > tools/testing/selftests/seccomp/seccomp_bpf.c | 2 ++
> > > 2 files changed, 3 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/samples/seccomp/user-trap.c b/samples/seccomp/user-trap.c
> > > index 6d0125ca8af7..0ca8fb37cd79 100644
> > > --- a/samples/seccomp/user-trap.c
> > > +++ b/samples/seccomp/user-trap.c
> > > @@ -298,7 +298,6 @@ int main(void)
> > > req = malloc(sizes.seccomp_notif);
> > > if (!req)
> > > goto out_close;
> > > - memset(req, 0, sizeof(*req));
> > >
> > > resp = malloc(sizes.seccomp_notif_resp);
> > > if (!resp)
> > > @@ -306,6 +305,7 @@ int main(void)
> > > memset(resp, 0, sizeof(*resp));
> >
> > I know it's unrelated, but it's probably worth sending a patch to fix
> > this to be sizes.seccomp_notif_resp instead of sizeof(*resp), since if
> > the kernel is older this will over-zero things. I can do that, or you
> > can add the patch to this series, just let me know which.
>
> I was thinking about this, and initially, I chose to make the smaller
> change. I think it might make more sense to combine the patch,
> given that the memset behaviour is "incorrect" if we do it based on
> sizeof(*req), or sizeof(*resp).
>
> I'll go ahead and respin this patch with the change to call memset
> based on sizes.
I think it would be good to keep it as a separate patch, since it's an
unrelated bug fix. That way if we have to revert these because of some
breakage, we won't lose the fix.
Cheers,
Tycho
next prev parent reply other threads:[~2019-12-29 0:18 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-12-28 1:48 [PATCH v2 1/2] samples, selftests/seccomp: Zero out seccomp_notif Sargun Dhillon
2019-12-28 9:25 ` Christian Brauner
2019-12-28 18:18 ` Tycho Andersen
2019-12-29 0:10 ` Sargun Dhillon
2019-12-29 0:18 ` Tycho Andersen [this message]
2019-12-30 19:14 ` Sargun Dhillon
2019-12-30 19:33 ` 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=20191229001818.GC6746@cisco \
--to=tycho@tycho.ws \
--cc=christian.brauner@ubuntu.com \
--cc=cyphar@cyphar.com \
--cc=jannh@google.com \
--cc=keescook@chromium.org \
--cc=linux-api@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=sargun@sargun.me \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).