* [PATCH net-next] scm: Don't use struct ucred in NETLINK_CB and struct scm_cookie.
@ 2012-09-07 4:20 Eric W. Biederman
2012-09-07 15:07 ` Eric Dumazet
2012-09-07 18:42 ` David Miller
0 siblings, 2 replies; 6+ messages in thread
From: Eric W. Biederman @ 2012-09-07 4:20 UTC (permalink / raw)
To: David Miller; +Cc: netdev, Serge E. Hallyn, Eric Dumazet
Passing uids and gids on NETLINK_CB from a process in one user
namespace to a process in another user namespace can result in the
wrong uid or gid being presented to userspace. Avoid that problem by
passing kuids and kgids instead.
- define struct scm_creds for use in scm_cookie and netlink_skb_parms
that holds uid and gid information in kuid_t and kgid_t.
- Modify scm_set_cred to fill out scm_creds by heand instead of using
cred_to_ucred to fill out struct ucred. This conversion ensures
userspace does not get incorrect uid or gid values to look at.
- Modify scm_recv to convert from struct scm_creds to struct ucred
before copying credential values to userspace.
- Modify __scm_send to populate struct scm_creds on in the scm_cookie,
instead of just copying struct ucred from userspace.
- Modify netlink_sendmsg to copy scm_creds instead of struct ucred
into the NETLINK_CB.
Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>
---
include/linux/netlink.h | 3 ++-
include/net/scm.h | 23 +++++++++++++++++++----
net/core/scm.c | 17 +++++++++++------
net/netlink/af_netlink.c | 2 +-
4 files changed, 33 insertions(+), 12 deletions(-)
diff --git a/include/linux/netlink.h b/include/linux/netlink.h
index c9fdde2..df73cf4 100644
--- a/include/linux/netlink.h
+++ b/include/linux/netlink.h
@@ -153,6 +153,7 @@ struct nlattr {
#include <linux/capability.h>
#include <linux/skbuff.h>
+#include <net/scm.h>
struct net;
@@ -162,7 +163,7 @@ static inline struct nlmsghdr *nlmsg_hdr(const struct sk_buff *skb)
}
struct netlink_skb_parms {
- struct ucred creds; /* Skb credentials */
+ struct scm_creds creds; /* Skb credentials */
__u32 pid;
__u32 dst_group;
struct sock *ssk;
diff --git a/include/net/scm.h b/include/net/scm.h
index 079d788..000b6f7 100644
--- a/include/net/scm.h
+++ b/include/net/scm.h
@@ -12,6 +12,12 @@
*/
#define SCM_MAX_FD 253
+struct scm_creds {
+ u32 pid;
+ kuid_t uid;
+ kgid_t gid;
+};
+
struct scm_fp_list {
short count;
short max;
@@ -22,7 +28,7 @@ struct scm_cookie {
struct pid *pid; /* Skb credentials */
const struct cred *cred;
struct scm_fp_list *fp; /* Passed files */
- struct ucred creds; /* Skb credentials */
+ struct scm_creds creds; /* Skb credentials */
#ifdef CONFIG_SECURITY_NETWORK
u32 secid; /* Passed security ID */
#endif
@@ -49,7 +55,9 @@ static __inline__ void scm_set_cred(struct scm_cookie *scm,
{
scm->pid = get_pid(pid);
scm->cred = cred ? get_cred(cred) : NULL;
- cred_to_ucred(pid, cred, &scm->creds);
+ scm->creds.pid = pid_vnr(pid);
+ scm->creds.uid = cred ? cred->euid : INVALID_UID;
+ scm->creds.gid = cred ? cred->egid : INVALID_GID;
}
static __inline__ void scm_destroy_cred(struct scm_cookie *scm)
@@ -110,8 +118,15 @@ static __inline__ void scm_recv(struct socket *sock, struct msghdr *msg,
return;
}
- if (test_bit(SOCK_PASSCRED, &sock->flags))
- put_cmsg(msg, SOL_SOCKET, SCM_CREDENTIALS, sizeof(scm->creds), &scm->creds);
+ if (test_bit(SOCK_PASSCRED, &sock->flags)) {
+ struct user_namespace *current_ns = current_user_ns();
+ struct ucred ucreds = {
+ .pid = scm->creds.pid,
+ .uid = from_kuid_munged(current_ns, scm->creds.uid),
+ .gid = from_kgid_munged(current_ns, scm->creds.gid),
+ };
+ put_cmsg(msg, SOL_SOCKET, SCM_CREDENTIALS, sizeof(ucreds), &ucreds);
+ }
scm_destroy_cred(scm);
diff --git a/net/core/scm.c b/net/core/scm.c
index 5472ae7..64b41d8 100644
--- a/net/core/scm.c
+++ b/net/core/scm.c
@@ -155,19 +155,21 @@ int __scm_send(struct socket *sock, struct msghdr *msg, struct scm_cookie *p)
break;
case SCM_CREDENTIALS:
{
+ struct ucred creds;
kuid_t uid;
kgid_t gid;
if (cmsg->cmsg_len != CMSG_LEN(sizeof(struct ucred)))
goto error;
- memcpy(&p->creds, CMSG_DATA(cmsg), sizeof(struct ucred));
- err = scm_check_creds(&p->creds);
+ memcpy(&creds, CMSG_DATA(cmsg), sizeof(struct ucred));
+ err = scm_check_creds(&creds);
if (err)
goto error;
- if (!p->pid || pid_vnr(p->pid) != p->creds.pid) {
+ p->creds.pid = creds.pid;
+ if (!p->pid || pid_vnr(p->pid) != creds.pid) {
struct pid *pid;
err = -ESRCH;
- pid = find_get_pid(p->creds.pid);
+ pid = find_get_pid(creds.pid);
if (!pid)
goto error;
put_pid(p->pid);
@@ -175,11 +177,14 @@ int __scm_send(struct socket *sock, struct msghdr *msg, struct scm_cookie *p)
}
err = -EINVAL;
- uid = make_kuid(current_user_ns(), p->creds.uid);
- gid = make_kgid(current_user_ns(), p->creds.gid);
+ uid = make_kuid(current_user_ns(), creds.uid);
+ gid = make_kgid(current_user_ns(), creds.gid);
if (!uid_valid(uid) || !gid_valid(gid))
goto error;
+ p->creds.uid = uid;
+ p->creds.gid = gid;
+
if (!p->cred ||
!uid_eq(p->cred->euid, uid) ||
!gid_eq(p->cred->egid, gid)) {
diff --git a/net/netlink/af_netlink.c b/net/netlink/af_netlink.c
index 7cb7867..6473267 100644
--- a/net/netlink/af_netlink.c
+++ b/net/netlink/af_netlink.c
@@ -1398,7 +1398,7 @@ static int netlink_sendmsg(struct kiocb *kiocb, struct socket *sock,
NETLINK_CB(skb).pid = nlk->pid;
NETLINK_CB(skb).dst_group = dst_group;
- memcpy(NETLINK_CREDS(skb), &siocb->scm->creds, sizeof(struct ucred));
+ NETLINK_CB(skb).creds = siocb->scm->creds;
err = -EFAULT;
if (memcpy_fromiovec(skb_put(skb, len), msg->msg_iov, len)) {
--
1.7.5.4
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH net-next] scm: Don't use struct ucred in NETLINK_CB and struct scm_cookie.
2012-09-07 4:20 [PATCH net-next] scm: Don't use struct ucred in NETLINK_CB and struct scm_cookie Eric W. Biederman
@ 2012-09-07 15:07 ` Eric Dumazet
2012-09-07 18:41 ` David Miller
2012-09-07 18:42 ` David Miller
1 sibling, 1 reply; 6+ messages in thread
From: Eric Dumazet @ 2012-09-07 15:07 UTC (permalink / raw)
To: Eric W. Biederman; +Cc: David Miller, netdev, Serge E. Hallyn
On Thu, 2012-09-06 at 21:20 -0700, Eric W. Biederman wrote:
> Passing uids and gids on NETLINK_CB from a process in one user
> namespace to a process in another user namespace can result in the
> wrong uid or gid being presented to userspace. Avoid that problem by
> passing kuids and kgids instead.
>
> - define struct scm_creds for use in scm_cookie and netlink_skb_parms
> that holds uid and gid information in kuid_t and kgid_t.
>
> - Modify scm_set_cred to fill out scm_creds by heand instead of using
> cred_to_ucred to fill out struct ucred. This conversion ensures
> userspace does not get incorrect uid or gid values to look at.
>
> - Modify scm_recv to convert from struct scm_creds to struct ucred
> before copying credential values to userspace.
>
> - Modify __scm_send to populate struct scm_creds on in the scm_cookie,
> instead of just copying struct ucred from userspace.
>
> - Modify netlink_sendmsg to copy scm_creds instead of struct ucred
> into the NETLINK_CB.
>
> Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>
> diff --git a/net/netlink/af_netlink.c b/net/netlink/af_netlink.c
> index 7cb7867..6473267 100644
> --- a/net/netlink/af_netlink.c
> +++ b/net/netlink/af_netlink.c
> @@ -1398,7 +1398,7 @@ static int netlink_sendmsg(struct kiocb *kiocb, struct socket *sock,
>
> NETLINK_CB(skb).pid = nlk->pid;
> NETLINK_CB(skb).dst_group = dst_group;
> - memcpy(NETLINK_CREDS(skb), &siocb->scm->creds, sizeof(struct ucred));
> + NETLINK_CB(skb).creds = siocb->scm->creds;
>
> err = -EFAULT;
> if (memcpy_fromiovec(skb_put(skb, len), msg->msg_iov, len)) {
Seems fine to me, but I am not sure why you kept NETLINK_CREDS()
defined/used once.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net-next] scm: Don't use struct ucred in NETLINK_CB and struct scm_cookie.
2012-09-07 15:07 ` Eric Dumazet
@ 2012-09-07 18:41 ` David Miller
2012-09-07 21:25 ` Eric W. Biederman
0 siblings, 1 reply; 6+ messages in thread
From: David Miller @ 2012-09-07 18:41 UTC (permalink / raw)
To: eric.dumazet; +Cc: ebiederm, netdev, serge
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Fri, 07 Sep 2012 17:07:26 +0200
> Seems fine to me, but I am not sure why you kept NETLINK_CREDS()
> defined/used once.
There are two remaining users, kernel/audit.c and scsi netlink.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net-next] scm: Don't use struct ucred in NETLINK_CB and struct scm_cookie.
2012-09-07 4:20 [PATCH net-next] scm: Don't use struct ucred in NETLINK_CB and struct scm_cookie Eric W. Biederman
2012-09-07 15:07 ` Eric Dumazet
@ 2012-09-07 18:42 ` David Miller
1 sibling, 0 replies; 6+ messages in thread
From: David Miller @ 2012-09-07 18:42 UTC (permalink / raw)
To: ebiederm; +Cc: netdev, serge, eric.dumazet
From: ebiederm@xmission.com (Eric W. Biederman)
Date: Thu, 06 Sep 2012 21:20:01 -0700
>
> Passing uids and gids on NETLINK_CB from a process in one user
> namespace to a process in another user namespace can result in the
> wrong uid or gid being presented to userspace. Avoid that problem by
> passing kuids and kgids instead.
>
> - define struct scm_creds for use in scm_cookie and netlink_skb_parms
> that holds uid and gid information in kuid_t and kgid_t.
>
> - Modify scm_set_cred to fill out scm_creds by heand instead of using
> cred_to_ucred to fill out struct ucred. This conversion ensures
> userspace does not get incorrect uid or gid values to look at.
>
> - Modify scm_recv to convert from struct scm_creds to struct ucred
> before copying credential values to userspace.
>
> - Modify __scm_send to populate struct scm_creds on in the scm_cookie,
> instead of just copying struct ucred from userspace.
>
> - Modify netlink_sendmsg to copy scm_creds instead of struct ucred
> into the NETLINK_CB.
>
> Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>
Applied, thanks Eric.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net-next] scm: Don't use struct ucred in NETLINK_CB and struct scm_cookie.
2012-09-07 18:41 ` David Miller
@ 2012-09-07 21:25 ` Eric W. Biederman
2012-09-07 21:32 ` David Miller
0 siblings, 1 reply; 6+ messages in thread
From: Eric W. Biederman @ 2012-09-07 21:25 UTC (permalink / raw)
To: David Miller; +Cc: eric.dumazet, netdev, serge
David Miller <davem@davemloft.net> writes:
> From: Eric Dumazet <eric.dumazet@gmail.com>
> Date: Fri, 07 Sep 2012 17:07:26 +0200
>
>> Seems fine to me, but I am not sure why you kept NETLINK_CREDS()
>> defined/used once.
>
> There are two remaining users, kernel/audit.c and scsi netlink.
Good point. I have a change to kernel/audit.c that removes the
usage. I believe the scsi netlink usage is just plain bogus,
and it wants the netlink port identifier instead.
Would anyone mind if we changed the name of the field in netlink from
pid to portid? It seems to cause a lot of confusion. We can't change
the user space fields of course but...
Right now because we can now use current using NETLINK_CREDS in the
kernel is unnecessary.
Eric
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net-next] scm: Don't use struct ucred in NETLINK_CB and struct scm_cookie.
2012-09-07 21:25 ` Eric W. Biederman
@ 2012-09-07 21:32 ` David Miller
0 siblings, 0 replies; 6+ messages in thread
From: David Miller @ 2012-09-07 21:32 UTC (permalink / raw)
To: ebiederm; +Cc: eric.dumazet, netdev, serge
From: ebiederm@xmission.com (Eric W. Biederman)
Date: Fri, 07 Sep 2012 14:25:09 -0700
> Would anyone mind if we changed the name of the field in netlink from
> pid to portid? It seems to cause a lot of confusion. We can't change
> the user space fields of course but...
No objections.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2012-09-07 21:32 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-09-07 4:20 [PATCH net-next] scm: Don't use struct ucred in NETLINK_CB and struct scm_cookie Eric W. Biederman
2012-09-07 15:07 ` Eric Dumazet
2012-09-07 18:41 ` David Miller
2012-09-07 21:25 ` Eric W. Biederman
2012-09-07 21:32 ` David Miller
2012-09-07 18:42 ` David Miller
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox