Re: [PATCH v2 1/9] security: add LSM blob and hooks for namespaces
Mickaël Salaün <[email protected]>
| Newsgroups | org.kernel.vger.linux-security-module,org.kernel.vger.linux-fsdevel,org.kernel.vger.linux-kernel |
|---|---|
| Message-ID | <[email protected]> |
On Thu, Jul 09, 2026 at 09:03:58AM -0400, Paul Moore wrote: > On Thu, Jul 9, 2026 at 5:12 AM Mickaël Salaün <[email protected]> wrote: > > On Wed, Jul 08, 2026 at 11:22:17PM -0400, Paul Moore wrote: > > > On May 27, 2026 =?UTF-8?q?Micka=C3=ABl=20Sala=C3=BCn?= <[email protected]> wrote: > > > > > > > > All namespace types now share the same ns_common infrastructure. Extend > > > > this to include a security blob so LSMs can start managing namespaces > > > > uniformly without having to add one-off hooks or security fields to > > > > every individual namespace type. > > ... > > > > > @@ -91,7 +103,10 @@ int __ns_common_init(struct ns_common *ns, u32 ns_type, const struct proc_ns_ope > > > > > > > > void __ns_common_free(struct ns_common *ns) > > > > { > > > > - proc_free_inum(ns->inum); > > > > + security_namespace_free(ns); > > > > + > > > > + if (ns->inum > MNT_NS_INO_SPECIAL_MAX) > > > > + proc_free_inum(ns->inum); > > > > > > The ns->inum check in the if-conditional above isn't quite the same as > > > the is_anon_ns() check it replaces in free_mnt_ns(). You touch on this > > > a bit in the changelog, but that really should be explained in the > > > commit description. > > > > > > ... or honestly, should that change be a separate patch? > > > > I think it's fine, but it's Christian's patch, so I'll let him answer > > and propose a new commit description. > > I don't have a strong opinion on either approach, either a separate > patch or doc update, but one of the two needs to happen. Noted, I'll follow on this with Christian. > > > > > diff --git a/kernel/nsproxy.c b/kernel/nsproxy.c > > > > index d9d3d5973bf5..0f1b208d8eef 100644 > > > > --- a/kernel/nsproxy.c > > > > +++ b/kernel/nsproxy.c > > > > @@ -385,6 +385,12 @@ static int prepare_nsset(unsigned flags, struct nsset *nsset) > > > > > > > > static inline int validate_ns(struct nsset *nsset, struct ns_common *ns) > > > > { > > > > + int ret; > > > > + > > > > + ret = security_namespace_install(nsset, ns); > > > > + if (ret) > > > > + return ret; > > > > + > > > > return ns->ops->install(nsset, ns); > > > > } > > > > > > In the previous revision to the patchset I asked about a > > > security_namespace_switch() hook as we don't know if a namespace is > > > actually attached to a process until we get to switch_task_namespaces(). > > > Perhaps that was answered, but I don't recall reading any mail about that > > > and I'm not able to uncover any responses on lore. > > > > From the cover letter: > > > > no security_namespace_switch() post-hook is > > added in this series: such a hook would only serve LSMs that maintain > > per-task state derived from the active namespace set (SELinux-style > > state tracking), and no current LSM (including this series) needs that. > > Landlock enforces at namespace_install() and namespace_init(), before > > the task-to-nsproxy switch. The hook is left for a separate LSM > > infrastructure proposal once a concrete user emerges. > > > > This follows the guidance of adding new hooks: there must be at least > > one user. > > I'm aware of the new hook guidance, I was the one who documented it :) I know. I guess that means you're ok with that? > > In the future, it's both helpful and polite to reply to the email > asking the question with your response. Adding it to the cover letter > is fine, but to be perfectly honest, once we get past v1 of a > patchset, I don't often read the cover letter very closely unless > there is a significant change. However, I do look back at the > previous posting to ensure all the feedback from everyone has been > either answered or incorporated into the current revision. I forgot to reply to your email, I didn't mean to be impolite, sorry for the inconvenience. It's useful to know that the cover letter is not always part of your review process.