Re: [PATCH v3 1/3] security: Introduce LSM_ORDER_LAST and set it for the integrity LSM
From: Paul Moore <paul@paul-moore.com>
Date: 2023-03-10 16:26:21
Also in:
bpf, linux-integrity, lkml
On Fri, Mar 10, 2023 at 8:39 AM Mimi Zohar [off-list ref] wrote:
On Thu, 2023-03-09 at 17:04 -0500, Paul Moore wrote:quoted
On Thu, Mar 9, 2023 at 8:21 AM Mimi Zohar [off-list ref] wrote:quoted
On Thu, 2023-03-09 at 09:54 +0100, Roberto Sassu wrote:quoted
From: Roberto Sassu <roberto.sassu@huawei.com> Introduce LSM_ORDER_LAST, to satisfy the requirement of LSMs needing to be last, e.g. the 'integrity' LSM, without changing the kernel command line or configuration. Also, set this order for the 'integrity' LSM. While not enforced, this is the only LSM expected to use it. Similarly to LSM_ORDER_FIRST, LSMs with LSM_ORDER_LAST are always enabled and put at the end of the LSM list. Finally, for LSM_ORDER_MUTABLE LSMs, set the found variable to true if an LSM is found, regardless of its order. In this way, the kernel would not wrongly report that the LSM is not built-in in the kernel if its order is LSM_ORDER_LAST. Fixes: 79f7865d844c ("LSM: Introduce "lsm=" for boottime LSM selection") Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>Signed-off-by: Mimi Zohar <zohar@linux.ibm.com>Warning: procedural nitpicking ahead ... The 'Signed-off-by' tag is in reference to the DCO, which makes sense to add if you are a patch author or are merging a patch into a tree, but it doesn't make much sense as a ACK/thumbs-up; this is why we have the 'Acked-by' and 'Reviewed-by' tags. I generally read the 'Acked-by' tag as "I'm the one responsible for a chunk of code affected by this patch and I'm okay with this change" and the 'Reviewed-by' tag as "I looked at this patch and it looks like a good change to me". Perhaps surprisingly to some, while an 'Acked-by' is a requirement for merging in a lot of cases, I appreciate 'Reviewed-by' tags much more as it indicates the patch is getting some third-part eyeballs on it ... so all you lurkers on this list, if you're reviewing patches as they hit your inbox, don't be shy about posting your 'Reviewed-by' tag if your comfortable doing so, we all welcome the help :) https://www.kernel.org/doc/html/latest/process/submitting-patches.html#sign-your-work-the-developer-s-certificate-of-originIn this case, it was a bit unclear who actually was going to upstream this patch set.
FWIW, I wasn't expecting to see your sign-off without a note that you had merged it. Normally I would have expected either an acked-by or a note that you had merged it, a sign-off without a merge notice seemed a little odd to me so I thought I would mention the above :) No harm either way, I just figured a little discussion on process might not be a terrible idea to make sure we are all on the same page.
It's better that you upstream it, but since this affects subsequent IMA and EVM patches, please create a topic branch.
I generally don't do topic branches for work that has been merged into a -next or -stable branch. I prefer to limit topic branches to special-cases where there is some value in keeping a central branch for multiple people to coordinate while the patchset is still in development; once a patchset has progressed far enough to be merged into a -stable or -next branch I stop maintaining the topic branch. In this particular case the changes to the IMA/EVM code looked very minor, so I doubt there would be a significant merge conflict with the IMA/EVM tree during this development cycle, but if you would prefer to take this patchset via the IMA/EVM tree that is okay with me; just let me know so I can ACK the two LSM-related patches (I'm going to review the latest posting today). As a bit of an aside, while this doesn't cover topic branches (once again, I consider those special cases), when managing the LSM tree I follow the process that is documented here: https://github.com/LinuxSecurityModule/kernel/blob/main/README.md [NOTE: the above GH repo is a read-only mirror of the canonical LSM kernel.org repo, it just happens that GH does a better job rendering txt] The main LSM repo process "docs" / pointers can be found in the main README or "about" page: https://git.kernel.org/pub/scm/linux/kernel/git/pcmoore/lsm.git/about If people have suggestions for a different approach to managing the LSM tree I'm always open to discussion. -- paul-moore.com