linux-kernel - Re: [patch] net/xfrm: fix crash in ipsec audit logging

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [day] [month] [year] [list]

Message-Id: <200701022057.l02KvP9s028947@faith.austin.ibm.com>
Date:	Tue, 2 Jan 2007 14:57:25 -0600
From:	Joy Latten <latten@...tin.ibm.com>
To:	hadi@...erus.ca
Cc:	akpm@...l.org, davem@...emloft.net, dmw2@...radead.org,
	linux-kernel@...r.kernel.org, mingo@...e.hu, torvalds@...l.org
Subject: Re: [patch] net/xfrm: fix crash in ipsec audit logging


On Tue, 2006-12-26 at 13:37 -0500, jamal wrote:
>On Tue, 2006-26-12 at 18:56 +0100, Ingo Molnar wrote:
> 
> 
> > +	xfrm_audit_log(NETLINK_CB(skb).loginuid, NETLINK_CB(skb).sid,
> > +		       AUDIT_MAC_IPSEC_DELSPD, delete, xp, NULL);
> > +
> >  	if (!delete) {
> >  		struct sk_buff *resp_skb;
> 
> 
> You could move the call into the else from above if (!delete) maybe?
> Otherwise you have to add back the "if (delete)" check since that
> function could be used to either retrieve (which is not subject to an
> audit) or delete an xp.
> 
> cheers,
> jamal
>  

My apologies as I am just reading my email.

Yes, I think in the else part of the "if (!delete)".

I also added a check to xfrm_audit_log() such that if both xfrm
and policy are NULL, we return. There isn't anything to audit
since we are only auditing creation and deletion of xfrm and 
policy.

Ingo, could you try this patch and let me know if everything works ok
for you. I have built and test in my environment, but not tested as
you are using it.  

Regards,
Joy

Signed-off-by: Joy Latten <latten@...tin.ibm.com>

--------------------------------------------------------------------------

diff -urpN linux-2.6.19.orig/net/xfrm/xfrm_policy.c linux-2.6.19/net/xfrm/xfrm_policy.c
--- linux-2.6.19.orig/net/xfrm/xfrm_policy.c	2007-01-02 14:24:14.000000000 -0600
+++ linux-2.6.19/net/xfrm/xfrm_policy.c	2007-01-02 14:28:24.000000000 -0600
@@ -2003,6 +2003,9 @@ void xfrm_audit_log(uid_t auid, u32 sid,
 	if (audit_enabled == 0)
 		return;
 
+	if ((x == NULL) && (xp == NULL))
+		return;
+
 	audit_buf = audit_log_start(current->audit_context, GFP_ATOMIC, type);
 	if (audit_buf == NULL)
 	return;
diff -urpN linux-2.6.19.orig/net/xfrm/xfrm_user.c linux-2.6.19/net/xfrm/xfrm_user.c
--- linux-2.6.19.orig/net/xfrm/xfrm_user.c	2007-01-02 14:24:14.000000000 -0600
+++ linux-2.6.19/net/xfrm/xfrm_user.c	2007-01-02 14:28:14.000000000 -0600
@@ -1268,10 +1268,6 @@ static int xfrm_get_policy(struct sk_buf
 		xp = xfrm_policy_bysel_ctx(type, p->dir, &p->sel, tmp.security, delete);
 		security_xfrm_policy_free(&tmp);
 	}
-	if (delete)
-		xfrm_audit_log(NETLINK_CB(skb).loginuid, NETLINK_CB(skb).sid,
-			       AUDIT_MAC_IPSEC_DELSPD, (xp) ? 1 : 0, xp, NULL);
-
 	if (xp == NULL)
 		return -ENOENT;
 
@@ -1289,6 +1285,10 @@ static int xfrm_get_policy(struct sk_buf
 	} else {
 		if ((err = security_xfrm_policy_delete(xp)) != 0)
 			goto out;
+
+		xfrm_audit_log(NETLINK_CB(skb).loginuid, NETLINK_CB(skb).sid,
+			       AUDIT_MAC_IPSEC_DELSPD, (xp) ? 1 : 0, xp, NULL);
+
 		c.data.byid = p->index;
 		c.event = nlh->nlmsg_type;
 		c.seq = nlh->nlmsg_seq;

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/