Re: [PATCH] selinux: refactor code to return the correct errno

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Thanks for your work.

I've removed the modifications to netif and netport, and update the selinux_netlbl_sock_genattr's comment header.

On 2024/7/12 5:19, Paul Moore wrote:
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
.




[Index of Archives]     [Selinux Refpolicy]     [Linux SGX]     [Fedora Users]     [Fedora Desktop]     [Yosemite Photos]     [Yosemite Camping]     [Yosemite Campsites]     [KDE Users]     [Gnome Users]

  Powered by Linux