On Jul 10, 2024 Gaosheng Cui <cuigaosheng1@xxxxxxxxxx> wrote:
>
> Refactor the code in sel_netif_sid_slow to get the correct errno
> when an error occurs.
>
> Add some similar modifications to selinux_netlbl_sock_genattr and
> sel_netport_sid_slow.
>
> Signed-off-by: Gaosheng Cui <cuigaosheng1@xxxxxxxxxx>
> ---
> security/selinux/netif.c | 16 ++++++++++------
> security/selinux/netlabel.c | 16 ++++++++--------
> security/selinux/netport.c | 12 +++++++-----
> 3 files changed, 25 insertions(+), 19 deletions(-)
>
> diff --git a/security/selinux/netif.c b/security/selinux/netif.c
> index 43a0d3594b72..6d8544d8c63c 100644
> --- a/security/selinux/netif.c
> +++ b/security/selinux/netif.c
> @@ -156,14 +156,18 @@ static int sel_netif_sid_slow(struct net *ns, int ifindex, u32 *sid)
> ret = security_netif_sid(dev->name, sid);
> if (ret != 0)
> goto out;
> +
> new = kzalloc(sizeof(*new), GFP_ATOMIC);
> - if (new) {
> - new->nsec.ns = ns;
> - new->nsec.ifindex = ifindex;
> - new->nsec.sid = *sid;
> - if (sel_netif_insert(new))
> - kfree(new);
> + if (!new) {
> + ret = -ENOMEM;
> + goto out;
> }
> + new->nsec.ns = ns;
> + new->nsec.ifindex = ifindex;
> + new->nsec.sid = *sid;
> + ret = sel_netif_insert(new);
> + if (ret)
> + kfree(new);
The case where we fail add the new netif to the cache should not return
an error as we were able to successfully lookup the SELinux SID for the
netif and return it to the caller. Yes, we were not able to add it to
the cache, but that doesn't mean we should fail the operation by
returning an error code.
> out:
> spin_unlock_bh(&sel_netif_lock);
> diff --git a/security/selinux/netlabel.c b/security/selinux/netlabel.c
> index 55885634e880..40b5dcbd97d4 100644
> --- a/security/selinux/netlabel.c
> +++ b/security/selinux/netlabel.c
> @@ -76,11 +76,12 @@ static struct netlbl_lsm_secattr *selinux_netlbl_sock_genattr(struct sock *sk)
>
> secattr = netlbl_secattr_alloc(GFP_ATOMIC);
> if (secattr == NULL)
> - return NULL;
> + return ERR_PTR(-ENOMEM);
> +
> rc = security_netlbl_sid_to_secattr(sksec->sid, secattr);
> if (rc != 0) {
> netlbl_secattr_free(secattr);
> - return NULL;
> + return ERR_PTR(rc);
> }
> sksec->nlbl_secattr = secattr;
You need to update the function's comment header to indicate that it
returns error codes encoded with ERR_PTR() on failure.
> diff --git a/security/selinux/netport.c b/security/selinux/netport.c
> index 2e22ad9c2bd0..a75a479515fb 100644
> --- a/security/selinux/netport.c
> +++ b/security/selinux/netport.c
> @@ -152,12 +152,14 @@ static int sel_netport_sid_slow(u8 protocol, u16 pnum, u32 *sid)
> if (ret != 0)
> goto out;
> new = kzalloc(sizeof(*new), GFP_ATOMIC);
> - if (new) {
> - new->psec.port = pnum;
> - new->psec.protocol = protocol;
> - new->psec.sid = *sid;
> - sel_netport_insert(new);
> + if (!new) {
> + ret = -ENOMEM;
> + goto out;
> }
> + new->psec.port = pnum;
> + new->psec.protocol = protocol;
> + new->psec.sid = *sid;
> + sel_netport_insert(new);
Same logic as sel_netif_sid_slow().
--
paul-moore.com
