On Tue, Dec 19, 2023 at 4:11 AM Alfred Piccioni <alpic@xxxxxxxxxx> 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@xxxxxxxxxxxxxxx' and cc
> 'linux-kernel@xxxxxxxxxxxxxxx'. 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
