Re: [PATCH RFC 4/6] selinux: improve network lookup failure warnings

[Paul Moore <paul@paul-moore.com>]

On Mar 18, 2025 =?UTF-8?q?Christian=20G=C3=B6ttsche?= <cgoettsche@seltendoof.de> wrote:
> 
> Rate limit the warnings and include additional available information.
> 
> Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
> ---
>  security/selinux/netif.c   | 8 ++++----
>  security/selinux/netnode.c | 4 ++--
>  security/selinux/netport.c | 4 ++--
>  3 files changed, 8 insertions(+), 8 deletions(-)

My apologies that it took so long to get back to this, comments below ...
 
> diff --git a/security/selinux/netnode.c b/security/selinux/netnode.c
> index 8bb456d80dd5..76cf531af110 100644
> --- a/security/selinux/netnode.c
> +++ b/security/selinux/netnode.c
> @@ -228,8 +228,8 @@ static int sel_netnode_sid_slow(const void *addr, u16 family, u32 *sid)
>  
>  	spin_unlock_bh(&sel_netnode_lock);
>  	if (unlikely(ret))
> -		pr_warn("SELinux: failure in %s(), unable to determine network node label\n",
> -			__func__);
> +		pr_warn_ratelimited("SELinux: failure in %s(), unable to determine network node label (%d):  %d\n",
> +				    __func__, family, ret);

Let's leave the message as it is currently written.  I don't believe the
address family is going to be very helpful, and @ret will likely always
be -EINVAL in the error case.

If you wanted to add something to the error message, you could consider
displaying the offending IP address, so long as we can use the pI4/pI6
printk format specifiers to do it; I don't want to have to have a lot of
code in the error path simply to properly format IP addresses.

>  	return ret;
>  }
>  
> diff --git a/security/selinux/netport.c b/security/selinux/netport.c
> index 7d2207384d40..dadf14984fb4 100644
> --- a/security/selinux/netport.c
> +++ b/security/selinux/netport.c
> @@ -162,8 +162,8 @@ static int sel_netport_sid_slow(u8 protocol, u16 pnum, u32 *sid)
>  out:
>  	spin_unlock_bh(&sel_netport_lock);
>  	if (unlikely(ret))
> -		pr_warn("SELinux: failure in %s(), unable to determine network port label\n",
> -			__func__);
> +		pr_warn_ratelimited("SELinux: failure in %s(), unable to determine network port label (%d:%d):  %d\n",
> +				    __func__, protocol, pnum, ret);

Let's drop @ret from here too as really the only thing an admin can do is
ensure the policy has a definition for the port, the reason for the
lookup failure likely isn't very helpful (and looks to be mostly
transient, e.g. ENOMEM and similar).

--
paul-moore.com