xfrm: Don't pass current->pid to functions expecting a netlink portid

PATCH In review Eric W. Biederman 3 participants 5 messages Sep 8, 2012 – Sep 10, 2012

Subsystems (3)

Networking Network drivers Networking IPsec

lore

Share & tools

lore

Eric W. Biederman

author

2012-09-08 06:43 UTC #0 lore raw

PATCH 1/1 xfrm: Don't pass current->pid to functions expecting a netlink portid

km_policy_expired ultimately calls xfrm_exp_policy_notify which ignores any netlink porrts specified in it's km_event paramater and simply sends a netlink multicast message.

km_state_expired ultimately calls xfrm_exp_state_notify which ignores any netlink ports specified in it's km_event paramater and simply sends a netlink multicast message.

Therefore neither km_policy_expired nor km_state_expired needs the bogus current->pid value that is currently passed as a netlink port and a zero can be passed instead, just like all of the other call sites of km_port_expired and km_state_expired.

Cc Jamal Hadi Salim <[email protected]>

Signed-off-by "Eric W. Biederman" <[email protected]>

---
 net/xfrm/xfrm_user.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

net/xfrm/xfrm_user.c +2 -2

index 4801c81..d7bfd27 100644
@@ -1930,7 +1930,7 @@ static int xfrm_add_pol_expire(struct sk_buff *skb, struct nlmsghdr *nlh,
 		// reset the timers here?
 		WARN(1, "Dont know what to do with soft policy expire\n");
 	}
-	km_policy_expired(xp, p->dir, up->hard, current->pid);
+	km_policy_expired(xp, p->dir, up->hard, 0);
 
 out:
 	xfrm_pol_put(xp);

@@ -1958,7 +1958,7 @@ static int xfrm_add_sa_expire(struct sk_buff *skb, struct nlmsghdr *nlh,
 	err = -EINVAL;
 	if (x->km.state != XFRM_STATE_VALID)
 		goto out;
-	km_state_expired(x, ue->hard, current->pid);
+	km_state_expired(x, ue->hard, 0);
 
 	if (ue->hard) {
 		uid_t loginuid = audit_get_loginuid(current);

Signature · 1 line

1.7.5.4

Eric W. Biederman

author

2012-09-08 07:03 UTC #1 lore raw

Ignore this patch. The issue is real this fix is wrong.

Quoted 22 lines

net/xfrm/xfrm_user.c +2 -2

index 4801c81..d7bfd27 100644
@@ -1930,7 +1930,7 @@ static int xfrm_add_pol_expire(struct sk_buff *skb, struct nlmsghdr *nlh,
 		// reset the timers here?
 		WARN(1, "Dont know what to do with soft policy expire\n");
 	}
-	km_policy_expired(xp, p->dir, up->hard, current->pid);
+	km_policy_expired(xp, p->dir, up->hard, 0);
 
 out:
 	xfrm_pol_put(xp);

@@ -1958,7 +1958,7 @@ static int xfrm_add_sa_expire(struct sk_buff *skb, struct nlmsghdr *nlh,
 	err = -EINVAL;
 	if (x->km.state != XFRM_STATE_VALID)
 		goto out;
-	km_state_expired(x, ue->hard, current->pid);
+	km_state_expired(x, ue->hard, 0);
 
 	if (ue->hard) {
 		uid_t loginuid = audit_get_loginuid(current);

Eric W. Biederman

author

2012-09-08 07:17 UTC #2 lore raw

PATCH 1/1 xfrm: Report user triggered expirations against the users socket

When a policy expiration is triggered from user space the request travles through km_policy_expired and ultimately into xfrm_exp_policy_notify which calls build_polexpire. build_polexpire uses the netlink port passed to km_policy_expired as the source port for the netlink message it builds.

When a state expiration is triggered from user space the request travles through km_state_expired and ultimately into xfrm_exp_state_notify which calls build_expire. build_expire uses the netlink port passed to km_state_expired as the source port for the netlink message it builds.

Pass nlh->nlmsg_pid from the user generated netlink message that requested the expiration to km_policy_expired and km_state_expired instead of current->pid which is not a netlink port number.

Cc Jamal Hadi Salim <[email protected]>

Signed-off-by "Eric W. Biederman" <[email protected]>

---
 net/xfrm/xfrm_user.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

net/xfrm/xfrm_user.c +2 -2

index 4801c81..c1fbdbf 100644
@@ -1930,7 +1930,7 @@ static int xfrm_add_pol_expire(struct sk_buff *skb, struct nlmsghdr *nlh,
 		// reset the timers here?
 		WARN(1, "Dont know what to do with soft policy expire\n");
 	}
-	km_policy_expired(xp, p->dir, up->hard, current->pid);
+	km_policy_expired(xp, p->dir, up->hard, nlh->nlmsg_pid);
 
 out:
 	xfrm_pol_put(xp);

@@ -1958,7 +1958,7 @@ static int xfrm_add_sa_expire(struct sk_buff *skb, struct nlmsghdr *nlh,
 	err = -EINVAL;
 	if (x->km.state != XFRM_STATE_VALID)
 		goto out;
-	km_state_expired(x, ue->hard, current->pid);
+	km_state_expired(x, ue->hard, nlh->nlmsg_pid);
 
 	if (ue->hard) {
 		uid_t loginuid = audit_get_loginuid(current);

Signature · 1 line

1.7.5.4

Jamal Hadi Salim

Acked-by on v1 1/1

2012-09-08 11:48 UTC #3 lore raw

Re: [PATCH] xfrm: Report user triggered expirations against the users socket

Quoted 11 lines · Eric W. Biederman

When a policy expiration is triggered from user space the request travles through km_policy_expired and ultimately into xfrm_exp_policy_notify which calls build_polexpire. build_polexpire uses the netlink port passed to km_policy_expired as the source port for the netlink message it builds.
When a state expiration is triggered from user space the request travles through km_state_expired and ultimately into xfrm_exp_state_notify which calls build_expire. build_expire uses the netlink port passed to km_state_expired as the source port for the netlink message it builds.

Quoted 7 lines

Pass nlh->nlmsg_pid from the user generated netlink message that requested the expiration to km_policy_expired and km_state_expired instead of current->pid which is not a netlink port number.
Cc Jamal Hadi Salim <[email protected]>
Signed-off-by "Eric W. Biederman" <[email protected]>

I suppose.

Acked-by Jamal Hadi Salim <[email protected]>

cheers,
jamal

David Miller

maintainer Applied

2012-09-10 19:34 UTC #4 lore raw

Re: [PATCH] xfrm: Report user triggered expirations against the users socket

From: Jamal Hadi Salim <[email protected]>
Date: Sat, 08 Sep 2012 07:48:14 -0400

Quoted 19 lines · Eric W. Biederman

Quoted 18 lines
When a policy expiration is triggered from user space the request travles through km_policy_expired and ultimately into xfrm_exp_policy_notify which calls build_polexpire. build_polexpire uses the netlink port passed to km_policy_expired as the source port for
the netlink message it builds.
When a state expiration is triggered from user space the request travles
through km_state_expired and ultimately into xfrm_exp_state_notify which
calls build_expire. build_expire uses the netlink port passed to km_state_expired as the source port for the netlink message it builds.
Pass nlh->nlmsg_pid from the user generated netlink message that requested the expiration to km_policy_expired and km_state_expired instead of current->pid which is not a netlink port number.

Quoted 6 lines

Quoted 3 lines
Cc Jamal Hadi Salim <[email protected]>
Signed-off-by "Eric W. Biederman" <[email protected]>
I suppose.
Acked-by Jamal Hadi Salim <[email protected]>

Applied to net-next, thanks.