[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <CAHC9VhQq6jY63kYEQCp2t89Vv+_PDqv54RV6TO_TePDQyU6Vug@mail.gmail.com>
Date: Tue, 27 Jan 2026 16:59:35 -0500
From: Paul Moore <paul@...l-moore.com>
To: Tetsuo Handa <penguin-kernel@...ove.sakura.ne.jp>
Cc: SELinux <selinux@...r.kernel.org>,
linux-security-module <linux-security-module@...r.kernel.org>,
Steffen Klassert <steffen.klassert@...unet.com>, Herbert Xu <herbert@...dor.apana.org.au>,
"David S. Miller" <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>,
Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>, Simon Horman <horms@...nel.org>,
Network Development <netdev@...r.kernel.org>
Subject: Re: [PATCH] xfrm: kill xfrm_dev_{state,policy}_flush_secctx_check()
On Mon, Jan 26, 2026 at 10:51 PM Tetsuo Handa
<penguin-kernel@...ove.sakura.ne.jp> wrote:
> On 2026/01/27 7:33, Paul Moore wrote:
> > On Fri, Jan 23, 2026 at 5:13 AM Tetsuo Handa
> > <penguin-kernel@...ove.sakura.ne.jp> wrote:
> >>
> >> Since xfrm_dev_{state,policy}_flush() are called from only NETDEV_DOWN and
> >> NETDEV_UNREGISTER events, making xfrm_dev_{state,policy}_flush() no-op by
> >> returning an error value from xfrm_dev_{state,policy}_flush_secctx_check()
> >> is pointless. Especially, if xfrm_dev_{state,policy}_flush_secctx_check()
> >> returned an error value upon NETDEV_UNREGISTER event, the system will hung
> >> up with
> >>
> >> unregister_netdevice: waiting for $dev to become free. Usage count = $count
> >>
> >> message because the reference to $dev acquired by
> >> xfrm_dev_{state,policy}_add() cannot be released.
> >>
> >> Signed-off-by: Tetsuo Handa <penguin-kernel@...ove.SAKURA.ne.jp>
> >> ---
> >> net/xfrm/xfrm_policy.c | 35 -----------------------------------
> >> net/xfrm/xfrm_state.c | 33 ---------------------------------
> >> 2 files changed, 68 deletions(-)
> >
> > I didn't make it very far into reviewing this patch, because it looks
> > like xfrm_dev_state_flush() is called by the bonding driver's
> > notification handler, and I don't see that reflected in this patch?
>
> xfrm_dev_{state,policy}_flush() are called from only ...
My apologies, I was looking at the patch too quickly and shortened
xfrm_dev_state_flush_secctx_check() into xfrm_dev_state_flush() when
looking at the callers.
> LSM hook for checking whether to allow deleting a file in tmpfs which is still mounted
> makes sense, LSM hook for checking whether to allow starting unmount of tmpfs makes sense,
> but LSM hook for checking whether to allow releasing memory in tmpfs while unmount operation
> is already in progress causes nothing but a resource leak / denial-of-service kernel bug.
>
> What xfrm_dev_{state,policy}_flush_secctx_check() are causing is something like
> "LSM policy is refusing release of memory used by a file in tmpfs which is already under
> unmount operation".
> xfrm_dev_{state,policy}_flush_secctx_check() are too late to make LSM policy decision.
> A must-not-fail operation has already started before LSM hooks are called.
It sounds like we either need to confirm that
security_xfrm_{policy,state}_delete() is already present in all code
paths that result in SPD/SAD deletions (in a place that can safely
fail and return an error), or we need to place
xfrm_dev_{policy,state}_flush_secctx_check() in a location that can
safely fail. The patch doesn't relocate
xfrm_dev_{policy,state}_flush_secctx_check() and I don't really see
any mention about security_xfrm_{policy,state}_delete() in the patch
or description; have you verified that the LSM xfrm hooks are still
being called to authorize the removals?
--
paul-moore.com
Powered by blists - more mailing lists