linux-kernel - Re: [GIT PULL] SELinux patches for v5.16

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]

Date:   Mon, 1 Nov 2021 23:13:35 -0400
From:   Paul Moore <paul@...l-moore.com>
To:     Linus Torvalds <torvalds@...ux-foundation.org>
Cc:     SElinux list <selinux@...r.kernel.org>,
        LSM List <linux-security-module@...r.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [GIT PULL] SELinux patches for v5.16

On Mon, Nov 1, 2021 at 8:44 PM Linus Torvalds
<torvalds@...ux-foundation.org> wrote:
> On Mon, Nov 1, 2021 at 4:59 PM Paul Moore <paul@...l-moore.com> wrote:
> >
> > - Add LSM/SELinux/Smack controls and auditing for io-uring.
>
> I started doing the merge resolution, and then I noted that there is
> no sign that this has been discussed with the io_uring developers at
> all.

I felt I addressed that in the pull request cover letter, although it
appears not in a way that you found adequate.  More on this below, but
here is the discussion history, with lore links:

*** Initial Draft (RFC) (May 2021)
https://lore.kernel.org/linux-security-module/162163367115.8379.8459012634106035341.stgit@sifl/

In the initial RFC you will see a lot of discussion with Jens Axboe
and Pavel Begunkov discussing the patchset and potential changes to
the solution.  Jens summarized his opinion on resolving this in the
message below, you'll note Jens approach is what was implemented and
sent to you via PR.

* Jens' Summary
https://lore.kernel.org/linux-security-module/46381e4e-a65d-f217-1d0d-43d1fa8a99aa@kernel.dk/

  "Sorry for the lack of response here, but to sum up my
   order of preference:

   1) It's probably better to just make the audit an opt-out
      in io_op_defs for each opcode, and avoid needing boiler
      plate code for each op handler. The opt-out would ensure
      that new opcodes get it by default it someone doesn't
      know what it is, and the io_op_defs addition would mean
      that it's in generic code rather then in the handlers.
      Yes it's a bit slower, but it's saner imho.

   2) With the above, I'm fine with adding this to io_uring.
      I don't think going the route of mutual exclusion in
      kconfig helps anyone, it'd be counter productive to
      both sides."

*** Second Revision (RFC) (Aug 2021)
https://lore.kernel.org/linux-security-module/162871480969.63873.9434591871437326374.stgit@olly/

This patchset implemented the approach described by Jens as well as
incorporated all of the feedback from the initial RFC.  There was some
additional discussion among the LSM/audit crowd but no additional
comments from the io-uring devs despite being on the To/CC line and
the cover letter explicitly asking for their ACKs.

*** Third Revision (Sept 2021)
https://lore.kernel.org/linux-security-module/163159032713.470089.11728103630366176255.stgit@olly/

The third revision only had minor changes compared to the second, no
direct comments to this revision although related comments continued
to be made on prevision revisions.  The io-uring developers continue
to be on the To/CC line, with no comments.

*** Fourth Revision (Sept 2021)
https://lore.kernel.org/linux-security-module/163172413301.88001.16054830862146685573.stgit@olly/

The fourth revision also only had minor changes.  The patchset
continued to keep the io_uring devs on the To/CC line and there was an
explicit plea to ask for their review/ACK/etc. but none was ever sent.

> Maybe there have been extensive discussions. I wouldn't know. There's
> no acks, no links, no nothing in the commit messages.
>
> So I ended up deciding not to pull at all after all.
>
> You really can't just decide "let's add random audit hooks to this"
> without talking to the maintainers.

*sigh*

I can promise you I've never done that, nor would I ever consider it.

> And if you _did_ talk to maintainers, and got the go-ahead, why is
> there absolutely zero sign of that in the commits?

I felt the comment in the pull request was sufficient, however based
on your response it clearly isn't.  Would you like me to edit the
commits to add various discussion tags, is this follow-up sufficient,
or would you like me to do something else?  Just let me know what you
need to feel good about merging this pull request.

-- 
paul moore
www.paul-moore.com