netdev - Re: [PATCH net-next] netlink: fix compilation after memory mapped patches

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>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <877gjtuqy2.fsf@xmission.com>
Date:	Tue, 23 Apr 2013 09:55:49 -0700
From:	ebiederm@...ssion.com (Eric W. Biederman)
To:	Ben Hutchings <bhutchings@...arflare.com>
Cc:	Nicolas Dichtel <nicolas.dichtel@...nd.com>, <davem@...emloft.net>,
	<netdev@...r.kernel.org>, <kaber@...sh.net>
Subject: Re: [PATCH net-next] netlink: fix compilation after memory mapped patches

Ben Hutchings <bhutchings@...arflare.com> writes:

> On Tue, 2013-04-23 at 10:55 +0200, Nicolas Dichtel wrote:
>> Depending of the kernel configuration (CONFIG_UIDGID_STRICT_TYPE_CHECKS), we can
>> get the following errors:
>> 
>> net/netlink/af_netlink.c: In function ‘netlink_queue_mmaped_skb’:
>> net/netlink/af_netlink.c:663:14: error: incompatible types when assigning to type ‘__u32’ from type ‘kuid_t’
>> net/netlink/af_netlink.c:664:14: error: incompatible types when assigning to type ‘__u32’ from type ‘kgid_t’
>> net/netlink/af_netlink.c: In function ‘netlink_ring_set_copied’:
>> net/netlink/af_netlink.c:693:14: error: incompatible types when assigning to type ‘__u32’ from type ‘kuid_t’
>> net/netlink/af_netlink.c:694:14: error: incompatible types when assigning to type ‘__u32’ from type ‘kgid_t’
>> 
>> We must use the helpers to get the uid and gid.
>>
>> Signed-off-by: Nicolas Dichtel <nicolas.dichtel@...nd.com>
>> ---
>>  net/netlink/af_netlink.c | 8 ++++----
>>  1 file changed, 4 insertions(+), 4 deletions(-)
>> 
>> diff --git a/net/netlink/af_netlink.c b/net/netlink/af_netlink.c
>> index 2a3e9ba..0593a9a 100644
>> --- a/net/netlink/af_netlink.c
>> +++ b/net/netlink/af_netlink.c
>> @@ -660,8 +660,8 @@ static void netlink_queue_mmaped_skb(struct sock *sk, struct sk_buff *skb)
>>  	hdr->nm_len	= skb->len;
>>  	hdr->nm_group	= NETLINK_CB(skb).dst_group;
>>  	hdr->nm_pid	= NETLINK_CB(skb).creds.pid;
>> -	hdr->nm_uid	= NETLINK_CB(skb).creds.uid;
>> -	hdr->nm_gid	= NETLINK_CB(skb).creds.gid;
>> +	hdr->nm_uid	= __kuid_val(NETLINK_CB(skb).creds.uid);
>> +	hdr->nm_gid	= __kgid_val(NETLINK_CB(skb).creds.gid);
>
> Shouldn't these use from_k{u,g}id_munged() like scm_recv() does?

Most definitely.

__kuid_val and __kgid_val are implementation details you probably don't
want to be messing with.

So I think what the code wants to say is:
hdr->nm_uid = from_kuid(sk_user_ns(sk), NETLINK_CB(skb).creds.uid);
hdr->nm_gid = from_kgid(sk_user_ns(sk), NETLINK_CB(skb).creds.gid);

A rather frustrated part of me wonders if anyone uses netlink sockets
between userspace processes.  If not we could just make all of this
credential passing go away.

And there is something more that should be done with pids but netlink
is so twisty I haven't figured out how to make netlink sockets properly
pass pids between pid namespaces.

Eric
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html