Skip to contentSkip to conversation

selinux_xfrm_sec_ctx_alloc: null-terminate context string

PATCH In review Venkat Yekkirala 4 participants 5 messages Jul 24, 2007 SELinux security module

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 selinux_xfrm_sec_ctx_alloc: null-terminate context string

xfrm_audit_log() expects the context string to be null-terminated which currently doesn't happen with user-supplied contexts.

---
 security/selinux/xfrm.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
security/selinux/xfrm.c +2 -1
@@ -216,7 +216,7 @@ static int selinux_xfrm_sec_ctx_alloc(st
 		return -ENOMEM;
 
 	*ctxp = ctx = kmalloc(sizeof(*ctx) +
-			      uctx->ctx_len,
+			      uctx->ctx_len + 1,
 			      GFP_KERNEL);
 
 	if (!ctx)
@@ -229,6 +229,7 @@ static int selinux_xfrm_sec_ctx_alloc(st
 	memcpy(ctx->ctx_str,
 	       uctx+1,
 	       ctx->ctx_len);
+	ctx->ctx_str[ctx->ctx_len] = 0;
 	rc = security_context_to_sid(ctx->ctx_str,
 				     ctx->ctx_len,
 				     &ctx->ctx_sid);

--
This message was distributed to subscribers of the selinux mailing list. If you no longer wish to subscribe, send mail to [email protected] with the words "unsubscribe selinux" without quotes as the message.

#1 lore raw
Quoted 20 lines · Venkat Yekkirala

xfrm_audit_log() expects the context string to be null-terminated which currently doesn't happen with user-supplied contexts.

---
 security/selinux/xfrm.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
security/selinux/xfrm.c +1 -1
@@ -216,7 +216,7 @@ static int selinux_xfrm_sec_ctx_alloc(st
 		return -ENOMEM;
 
 	*ctxp = ctx = kmalloc(sizeof(*ctx) +
-			      uctx->ctx_len,
+			      uctx->ctx_len + 1,
 			      GFP_KERNEL);
 
 	if (!ctx)

security/selinux/xfrm.c static int selinux_xfrm_sec_ctx_alloc(st · line 235

Quoted 7 lines
security/selinux/xfrm.c +1 -0
@@ -229,6 +229,7 @@ static int selinux_xfrm_sec_ctx_alloc(st
 	memcpy(ctx->ctx_str,
 	       uctx+1,
 	       ctx->ctx_len);
+	ctx->ctx_str[ctx->ctx_len] = 0;
 	rc = security_context_to_sid(ctx->ctx_str,
 				     ctx->ctx_len,
 				     &ctx->ctx_sid);

At the moment I am working on a re-work of the xfrm_audit_log() so that we can audit RFC-required events as well. I will add this fix into the new patch and send to netdev. Venkat, I will include your ack and cc you in the submission. Let me know if this is ok.

Regards,
Joy

--
This message was distributed to subscribers of the selinux mailing list. If you no longer wish to subscribe, send mail to [email protected] with the words "unsubscribe selinux" without quotes as the message.

#2 lore raw
Quoted 19 lines · Joy Latten
Quoted 18 lines · Venkat Yekkirala

xfrm_audit_log() expects the context string to be null-terminated which currently doesn't happen with user-supplied contexts.

---
 security/selinux/xfrm.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
security/selinux/xfrm.c +1 -1
@@ -216,7 +216,7 @@ static int selinux_xfrm_sec_ctx_alloc(st
 		return -ENOMEM;
 
 	*ctxp = ctx = kmalloc(sizeof(*ctx) +
-			      uctx->ctx_len,
+			      uctx->ctx_len + 1,
 			      GFP_KERNEL);
 
Quoted 15 lines

security/selinux/xfrm.c static int selinux_xfrm_sec_ctx_alloc(st · line 235

Quoted 9 lines
security/selinux/xfrm.c +1 -0
 	if (!ctx)
@@ -229,6 +229,7 @@ static int selinux_xfrm_sec_ctx_alloc(st
 	memcpy(ctx->ctx_str,
 	       uctx+1,
 	       ctx->ctx_len);
+	ctx->ctx_str[ctx->ctx_len] = 0;
 	rc = security_context_to_sid(ctx->ctx_str,
 				     ctx->ctx_len,
 				     &ctx->ctx_sid);

At the moment I am working on a re-work of the xfrm_audit_log() so that we can audit RFC-required events as well. I will add this fix into the new patch and send to netdev. Venkat, I will include your ack and cc you in the submission. Let me know if this is ok.

Oops! This patch is not in xfrm_audit_log() code. So, I won't be including it in my patch or anything. Sorry, didn't have my coffee yet. :-)

Regards,
Joy

--
This message was distributed to subscribers of the selinux mailing list. If you no longer wish to subscribe, send mail to [email protected] with the words "unsubscribe selinux" without quotes as the message.

Acked-by on v1 1/1
#3 lore raw
In reply to Venkat Yekkirala
Quoted 4 lines · Venkat Yekkirala

xfrm_audit_log() expects the context string to be null-terminated which currently doesn't happen with user-supplied contexts.

Quoted 23 lines
---
 security/selinux/xfrm.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
security/selinux/xfrm.c +2 -1
@@ -216,7 +216,7 @@ static int selinux_xfrm_sec_ctx_alloc(st
 		return -ENOMEM;
 
 	*ctxp = ctx = kmalloc(sizeof(*ctx) +
-			      uctx->ctx_len,
+			      uctx->ctx_len + 1,
 			      GFP_KERNEL);
 
 	if (!ctx)
@@ -229,6 +229,7 @@ static int selinux_xfrm_sec_ctx_alloc(st
 	memcpy(ctx->ctx_str,
 	       uctx+1,
 	       ctx->ctx_len);
+	ctx->ctx_str[ctx->ctx_len] = 0;
 	rc = security_context_to_sid(ctx->ctx_str,
 				     ctx->ctx_len,
 				     &ctx->ctx_sid);
Signature · 8 lines
Stephen Smalley
National Security Agency


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to [email protected] with
the words "unsubscribe selinux" without quotes as the message.
#4 lore raw
Quoted 7 lines · Stephen Smalley
Quoted 4 lines · Venkat Yekkirala

xfrm_audit_log() expects the context string to be null-terminated which currently doesn't happen with user-supplied contexts.

Applied to
git://git.kernel.org/pub/scm/linux/kernel/git/jmorris/selinux-2.6.git#for-linus

Signature · 7 lines
James Morris
<[email protected]>

--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to [email protected] with
the words "unsubscribe selinux" without quotes as the message.