On Fri, Mar 10, 2023 at 11:53 AM Stephen Smalley
<stephen.smalley.work@xxxxxxxxx> wrote:
> On Fri, Mar 10, 2023 at 10:11 AM Paul Moore <paul@xxxxxxxxxxxxxx> wrote:
> > On Fri, Mar 10, 2023 at 8:47 AM Stephen Smalley
> > <stephen.smalley.work@xxxxxxxxx> wrote:
> > > On Thu, Mar 9, 2023 at 4:15 PM Paul Moore <paul@xxxxxxxxxxxxxx> wrote:
> > > > On Thu, Mar 9, 2023 at 3:55 PM Stephen Smalley
> > > > <stephen.smalley.work@xxxxxxxxx> wrote:
> > > > > On Thu, Mar 9, 2023 at 3:48 PM Paul Moore <paul@xxxxxxxxxxxxxx> wrote:
> > > > > > On Thu, Mar 9, 2023 at 2:18 PM Stephen Smalley
> > > > > > <stephen.smalley.work@xxxxxxxxx> wrote:
> > > > > > >
> > > > > > > Linus observed that the pervasive passing of selinux_state pointers
> > > > > > > introduced by me in commit aa8e712cee93 ("selinux: wrap global selinux
> > > > > > > state") adds overhead and complexity without providing any
> > > > > > > benefit. The original idea was to pave the way for SELinux namespaces
> > > > > > > but those have not yet been implemented and there isn't currently
> > > > > > > a concrete plan to do so. Remove the passing of the selinux_state
> > > > > > > pointers, reverting to direct use of the single global selinux_state,
> > > > > > > and likewise remove passing of child pointers like the selinux_avc.
> > > > > > > The selinux_policy pointer remains as it is needed for atomic switching
> > > > > > > of policies.
> > > > > > >
> > > > > > > Suggested-by: Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx>
> > > > > > > Signed-off-by: Stephen Smalley <stephen.smalley.work@xxxxxxxxx>
> > > > > > > ---
> > > > > > > security/selinux/avc.c | 197 ++++-----
> > > > > > > security/selinux/hooks.c | 549 ++++++++++---------------
> > > > > > > security/selinux/ibpkey.c | 2 +-
> > > > > > > security/selinux/ima.c | 37 +-
> > > > > > > security/selinux/include/avc.h | 29 +-
> > > > > > > security/selinux/include/avc_ss.h | 3 +-
> > > > > > > security/selinux/include/conditional.h | 4 +-
> > > > > > > security/selinux/include/ima.h | 10 +-
> > > > > > > security/selinux/include/security.h | 171 +++-----
> > > > > > > security/selinux/netif.c | 2 +-
> > > > > > > security/selinux/netlabel.c | 17 +-
> > > > > > > security/selinux/netnode.c | 4 +-
> > > > > > > security/selinux/netport.c | 2 +-
> > > > > > > security/selinux/selinuxfs.c | 208 ++++------
> > > > > > > security/selinux/ss/services.c | 346 +++++++---------
> > > > > > > security/selinux/ss/services.h | 1 -
> > > > > > > security/selinux/status.c | 44 +-
> > > > > > > security/selinux/xfrm.c | 20 +-
> > > > > > > 18 files changed, 651 insertions(+), 995 deletions(-)
> > > > > >
> > > > > > It looks like this patch was a bit too big for the mailing list; I'm
> > > > > > trimming my reply to get this discussion on the list.
> > > > > >
> > > > > > I strongly dislike merging patches that haven't hit the list, but I do
> > > > > > recognize that this is a bit of an unusual case. Have you tried
> > > > > > breaking this up into two (three?) patches? I imagine that should be
> > > > > > possible, although I worry that the time required to do that would be
> > > > > > prohibitive given the change itself.
> > > > > >
> > > > > > If that doesn't work, an alternative might be to file a PR against our
> > > > > > kernel subsystem mirror on GitHub and posting a link to the PR here.
> > > > > > I don't want to encourage this as a general way of submitting SELinux
> > > > > > kernel patches, but I could make an exception here.
> > > > > >
> > > > > > https://github.com/SELinuxProject/selinux-kernel
> > > > >
> > > > > I'm open to suggestions but didn't see an obvious way to split it in a
> > > > > manner that keeps everything in a working state after each patch.
> > > > > checkpatch.pl didn't complain about the size - not sure if that is a
> > > > > change in policy.
> > > >
> > > > I'm not sure checkpatch.pl is the right place for that anyway as every
> > > > list could have a different policy.
> > > >
> > > > FWIW, many years ago I was told to keep the diffstat under a thousand
> > > > lines as a general rule. While I've seen patches go over that limit
> > > > and hit various @vger lists, I personally try to keep my patches under
> > > > that 1k limit, and generally that isn't too hard. This really is a
> > > > bit of a special case I think.
> > > >
> > > > > Created a PR here:
> > > > > https://github.com/SELinuxProject/selinux-kernel/pull/64
> > > >
> > > > Thanks, I'll work from that.
> > >
> > > For those following along on the mailing list, the kernel test robot
> > > reported a problem with the original patch when lockdep is enabled, so
> > > I generated a v2 patch and updated the PR (same link above).
> >
> > That's interesting. Does that mean that your original patch made it
> > to the mailing list but somehow didn't make it to the lore archive? I
> > just (re)checked and there is no record of your patch in the archive
> > or the patchwork instance.
> >
> > How did the kernel test robot get the patch to test? Does it know to
> > go fetch GH PR URLs?
>
> The email from the robot referenced the PR so it appears to be
> watching for PRs to certain GH repositories and automatically testing
> them.
Huh, interesting. Thanks.
--
paul-moore.com
