Received-SPF: pass (google.com: domain of linux-kernel+bounces-7174-linux.lists.archive=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99;
Precedence: bulk
MIME-Version: 1.0
References: <20230906102557.3432236-1-alpic@google.com> <20231219090909.2827497-1-alpic@google.com>
 <CALcwBGC9LzzdJeq3SWy9F3g5A32s5uSvJZae4j+rwNQqqLHCKg@mail.gmail.com>
In-Reply-To: <CALcwBGC9LzzdJeq3SWy9F3g5A32s5uSvJZae4j+rwNQqqLHCKg@mail.gmail.com>
From: Stephen Smalley <stephen.smalley.work@gmail.com>
Date: Wed, 20 Dec 2023 10:34:38 -0500
Message-ID: <CAEjxPJ7kLwQAee+J1RJ_AvoVLkJR2L5dyZ0jFJHbazZANWgeyQ@mail.gmail.com>
Subject: Re: [PATCH] security: new security_file_ioctl_compat() hook
To: Alfred Piccioni <alpic@google.com>
Cc: Paul Moore <paul@paul-moore.com>, Eric Paris <eparis@parisplace.org>, 
	LSM List <linux-security-module@vger.kernel.org>, 
	Linux FS Devel <linux-fsdevel@vger.kernel.org>, stable@vger.kernel.org, 
	SElinux list <selinux@vger.kernel.org>, linux-kernel <linux-kernel@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Tue, Dec 19, 2023 at 4:11=E2=80=AFAM Alfred Piccioni <alpic@google.com> =
wrote:
>
> Thanks for taking the time to review! Apologies for the number of
> small mistakes.

NP.

> > Also, IIRC, Paul prefers putting a pair of parentheses after function
> > names to distinguish them, so in the subject line
> > and description it should be security_file_ioctl_compat() and
> > security_file_ioctl(), and you should put a patch version
> > in the [PATCH] prefix e.g. [PATCH v3] to make clear that it is a later
> > version, and usually one doesn't capitalize SELinux
> > or the leading verb in the subject line (just "selinux: introduce").
>
> Changed title to lower-case, prefixed with security, changed slightly
> to fit in summary with new parentheses. Added [PATCH V3] to the
> subject.

Patch description still doesn't include the parentheses after each
function name but probably not worth re-spinning unless Paul says to
do so. I don't see the v3 in the subject line. Seemingly that in
combination with the fact that you replied to the original thread
confuses the b4 tool (b4.docs.kernel.org) such that b4 mbox/am/shazam
ends up selecting the v2 patch instead by default.

> > Actually, since this spans more than just SELinux, the prefix likely
> > needs to reflect that (e.g. security: introduce ...)
> > and the patch should go to the linux-security-module mailing list too
> > and perhaps linux-fsdevel for the ioctl change.
>
> Added cc 'selinux@vger.kernel.org' and cc
> 'linux-kernel@vger.kernel.org'. Thanks!

Just FYI, scripts/get_maintainer.pl /path/to/patch will provide an
over-approximation of who to include on the distribution for patches
based on MAINTAINERS and recent committers. That said, I generally
prune the set it provides. More art than science.

> > I didn't do an audit but does anything need to be updated for the BPF
> > LSM or does it auto-magically pick up new hooks?
>
> I'm unsure. I looked through the BPF LSM and I can't see any way it's
> picking up the file_ioctl hook to begin with. It appears to me
> skimming through the code that it automagically picks it up, but I'm
> not willing to bet the kernel on it.
>
> Do you know who would be a good person to ask about this to make sure?

Looks like it inherited it via the lsm_hook_defs.h.
$ nm security/bpf/hooks.o | grep ioctl
                 U bpf_lsm_file_ioctl
                 U bpf_lsm_file_ioctl_compat