[PATCH 0/2] Two pretty trivial NetLabel bugfixes

[PATCH 0/2] Two pretty trivial NetLabel bugfixes

[Paul Moore]

This is a small patchset both in the number of patches (two) and the size of
the patches themselves.  The first patch fixes a potential locking issue with
a NetLabel state variable and the second patch fixes a couple of problems seen
when adding new CIPSO DOI definitions.

In light of the recent NetLabel locking issues I've spent this week going over
all of the NetLabel related locks and I firmly believe this patchset should
address the last of the problems.  However, if anyone with a good knowledge of
socket/sk locking can afford to take a look I would greatly appreciate it.

I tested both of these patches with what I believe to be pretty much all of
the kernel debug options enabled and I have not encountered any problems.
Please consider these for the 2.6.20 release.

--
paul moore
linux security @ hp

--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

[PATCH 0/2] Two pretty trivial NetLabel bugfixes

[Paul Moore]

This is a small patchset both in the number of patches (two) and the size of
the patches themselves.  The first patch fixes a potential locking issue with
a NetLabel state variable and the second patch fixes a couple of problems seen
when adding new CIPSO DOI definitions.

In light of the recent NetLabel locking issues I've spent this week going over
all of the NetLabel related locks and I firmly believe this patchset should
address the last of the problems.  However, if anyone with a good knowledge of
socket/sk locking can afford to take a look I would greatly appreciate it.

I tested both of these patches with what I believe to be pretty much all of
the kernel debug options enabled and I have not encountered any problems.
Please consider these for the 2.6.20 release.

--
paul moore
linux security @ hp

[PATCH 1/2] NetLabel: correct locking in selinux_netlbl_socket_setsid()

[Paul Moore]

The spinlock protecting the update of the "sksec->nlbl_state" variable is not
currently softirq safe which can lead to problems.  This patch fixes this by
changing the spin_{un}lock() functions into spin_{un}lock_bh() functions.

Signed-off-by: Paul Moore <paul.moore@hp.com>
---
 security/selinux/ss/services.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Index: net-2.6.20_bugfix_3/security/selinux/ss/services.c
===================================================================
--- net-2.6.20_bugfix_3.orig/security/selinux/ss/services.c
+++ net-2.6.20_bugfix_3/security/selinux/ss/services.c
@@ -2492,9 +2492,9 @@ static int selinux_netlbl_socket_setsid(
 
 	rc = netlbl_socket_setattr(sock, &secattr);
 	if (rc == 0) {
-		spin_lock(&sksec->nlbl_lock);
+		spin_lock_bh(&sksec->nlbl_lock);
 		sksec->nlbl_state = NLBL_LABELED;
-		spin_unlock(&sksec->nlbl_lock);
+		spin_unlock_bh(&sksec->nlbl_lock);
 	}
 
 netlbl_socket_setsid_return:

--
paul moore
linux security @ hp

--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

[PATCH 1/2] NetLabel: correct locking in selinux_netlbl_socket_setsid()

[Paul Moore]

[-- Attachment #1: netlabel-softirq_lock_safety --]
[-- Type: text/plain, Size: 993 bytes --]

The spinlock protecting the update of the "sksec->nlbl_state" variable is not
currently softirq safe which can lead to problems.  This patch fixes this by
changing the spin_{un}lock() functions into spin_{un}lock_bh() functions.

Signed-off-by: Paul Moore <paul.moore@hp.com>
---
 security/selinux/ss/services.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Index: net-2.6.20_bugfix_3/security/selinux/ss/services.c
===================================================================
--- net-2.6.20_bugfix_3.orig/security/selinux/ss/services.c
+++ net-2.6.20_bugfix_3/security/selinux/ss/services.c
@@ -2492,9 +2492,9 @@ static int selinux_netlbl_socket_setsid(
 
 	rc = netlbl_socket_setattr(sock, &secattr);
 	if (rc == 0) {
-		spin_lock(&sksec->nlbl_lock);
+		spin_lock_bh(&sksec->nlbl_lock);
 		sksec->nlbl_state = NLBL_LABELED;
-		spin_unlock(&sksec->nlbl_lock);
+		spin_unlock_bh(&sksec->nlbl_lock);
 	}
 
 netlbl_socket_setsid_return:

--
paul moore
linux security @ hp

[PATCH 2/2] NetLabel: correct CIPSO tag handling when adding new DOI definitions

[Paul Moore]

The current netlbl_cipsov4_add_common() function has two problems which are
fixed with this patch.  The first is an off-by-one bug where it is possibile to
overflow the doi_def->tags[] array.  The second is a bug where the same 
doi_def->tags[] array was not always fully initialized, which caused sporadic
failures.

Signed-off-by: Paul Moore <paul.moore@hp.com>
---
 net/netlabel/netlabel_cipso_v4.c |    6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

Index: net-2.6.20_bugfix_3/net/netlabel/netlabel_cipso_v4.c
===================================================================
--- net-2.6.20_bugfix_3.orig/net/netlabel/netlabel_cipso_v4.c
+++ net-2.6.20_bugfix_3/net/netlabel/netlabel_cipso_v4.c
@@ -130,12 +130,12 @@ static int netlbl_cipsov4_add_common(str
 
 	nla_for_each_nested(nla, info->attrs[NLBL_CIPSOV4_A_TAGLST], nla_rem)
 		if (nla->nla_type == NLBL_CIPSOV4_A_TAG) {
-			if (iter > CIPSO_V4_TAG_MAXCNT)
+			if (iter >= CIPSO_V4_TAG_MAXCNT)
 				return -EINVAL;
 			doi_def->tags[iter++] = nla_get_u8(nla);
 		}
-	if (iter < CIPSO_V4_TAG_MAXCNT)
-		doi_def->tags[iter] = CIPSO_V4_TAG_INVALID;
+	while (iter < CIPSO_V4_TAG_MAXCNT)
+		doi_def->tags[iter++] = CIPSO_V4_TAG_INVALID;
 
 	return 0;
 }

--
paul moore
linux security @ hp

--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

[PATCH 2/2] NetLabel: correct CIPSO tag handling when adding new DOI definitions

[Paul Moore]

[-- Attachment #1: netlabel-cipso_add_fix --]
[-- Type: text/plain, Size: 1259 bytes --]

The current netlbl_cipsov4_add_common() function has two problems which are
fixed with this patch.  The first is an off-by-one bug where it is possibile to
overflow the doi_def->tags[] array.  The second is a bug where the same 
doi_def->tags[] array was not always fully initialized, which caused sporadic
failures.

Signed-off-by: Paul Moore <paul.moore@hp.com>
---
 net/netlabel/netlabel_cipso_v4.c |    6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

Index: net-2.6.20_bugfix_3/net/netlabel/netlabel_cipso_v4.c
===================================================================
--- net-2.6.20_bugfix_3.orig/net/netlabel/netlabel_cipso_v4.c
+++ net-2.6.20_bugfix_3/net/netlabel/netlabel_cipso_v4.c
@@ -130,12 +130,12 @@ static int netlbl_cipsov4_add_common(str
 
 	nla_for_each_nested(nla, info->attrs[NLBL_CIPSOV4_A_TAGLST], nla_rem)
 		if (nla->nla_type == NLBL_CIPSOV4_A_TAG) {
-			if (iter > CIPSO_V4_TAG_MAXCNT)
+			if (iter >= CIPSO_V4_TAG_MAXCNT)
 				return -EINVAL;
 			doi_def->tags[iter++] = nla_get_u8(nla);
 		}
-	if (iter < CIPSO_V4_TAG_MAXCNT)
-		doi_def->tags[iter] = CIPSO_V4_TAG_INVALID;
+	while (iter < CIPSO_V4_TAG_MAXCNT)
+		doi_def->tags[iter++] = CIPSO_V4_TAG_INVALID;
 
 	return 0;
 }

--
paul moore
linux security @ hp