Patchwork soreuseport: fix use of uid in tb->fastuid

login
register
mail settings
Submitter Tom Herbert
Date Jan. 26, 2013, 5:50 p.m.
Message ID <alpine.DEB.2.00.1301260944370.27231@pokey.mtv.corp.google.com>
Download mbox | patch
Permalink /patch/215920/
State Accepted
Delegated to: David Miller
Headers show

Comments

Tom Herbert - Jan. 26, 2013, 5:50 p.m.
Fix a reported compilation error where ia variable of type kuid_t
was being set to zero.

Eliminate two instances of setting tb->fastuid to zero.  tb->fastuid is 
only used if tb->fastreuseport is set, so there should be no problem if 
tb->fastuid is not initialized (when tb->fastreuesport is zero).

Signed-off-by: Tom Herbert <therbert@google.com>
---
 net/ipv4/inet_connection_sock.c | 11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)
David Miller - Jan. 27, 2013, 6:02 a.m.
From: Tom Herbert <therbert@google.com>
Date: Sat, 26 Jan 2013 09:50:54 -0800 (PST)

> Fix a reported compilation error where ia variable of type kuid_t
> was being set to zero.
> 
> Eliminate two instances of setting tb->fastuid to zero.  tb->fastuid is 
> only used if tb->fastreuseport is set, so there should be no problem if 
> tb->fastuid is not initialized (when tb->fastreuesport is zero).
> 
> Signed-off-by: Tom Herbert <therbert@google.com>

Applied.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Steffen Klassert - Jan. 28, 2013, 7:47 a.m.
On Sat, Jan 26, 2013 at 09:50:54AM -0800, Tom Herbert wrote:
> Fix a reported compilation error where ia variable of type kuid_t
> was being set to zero.
> 

Thanks for fixing this.
Unfortunately I get another compilation error now:

net/ipv6/inet6_connection_sock.c: In function ‘inet6_csk_bind_conflict’:
net/ipv6/inet6_connection_sock.c:37:12: error: incompatible types when initializing type ‘int’ using type ‘kuid_t’
net/ipv6/inet6_connection_sock.c:54:30: error: incompatible type for argument 1 of ‘uid_eq’
include/linux/uidgid.h:70:20: note: expected ‘kuid_t’ but argument is of type ‘int’
make[3]: *** [net/ipv6/inet6_connection_sock.o] Error 1

I wondered why noone else is seeing this, so I did some analysis.
I'm getting this because of the UIDGID_STRICT_TYPE_CHECKS config option.
This is selected by USER_NS which in turn depends on UIDGID_CONVERTED.
Most people can't enable USER_NS because they compile something like
nfs or anything else that is not 'UIDGID_CONVERTED'.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Eric W. Biederman - Jan. 29, 2013, 12:26 p.m.
Steffen Klassert <steffen.klassert@secunet.com> writes:

> On Sat, Jan 26, 2013 at 09:50:54AM -0800, Tom Herbert wrote:
>> Fix a reported compilation error where ia variable of type kuid_t
>> was being set to zero.
>> 
>
> Thanks for fixing this.
> Unfortunately I get another compilation error now:
>
> net/ipv6/inet6_connection_sock.c: In function ‘inet6_csk_bind_conflict’:
> net/ipv6/inet6_connection_sock.c:37:12: error: incompatible types when initializing type ‘int’ using type ‘kuid_t’
> net/ipv6/inet6_connection_sock.c:54:30: error: incompatible type for argument 1 of ‘uid_eq’
> include/linux/uidgid.h:70:20: note: expected ‘kuid_t’ but argument is of type ‘int’
> make[3]: *** [net/ipv6/inet6_connection_sock.o] Error 1
>
> I wondered why noone else is seeing this, so I did some analysis.
> I'm getting this because of the UIDGID_STRICT_TYPE_CHECKS config option.
> This is selected by USER_NS which in turn depends on UIDGID_CONVERTED.
> Most people can't enable USER_NS because they compile something like
> nfs or anything else that is not 'UIDGID_CONVERTED'.

Yes.  There are currently 11 filesystems whose uid_t to kuid_t
conversions have not yet been merged.  I am working on those
conversions now and with a little luck I will have everything
ready them all converted by 3.9.

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

Patch

diff --git a/net/ipv4/inet_connection_sock.c b/net/ipv4/inet_connection_sock.c
index 8bb623d..11cb497 100644
--- a/net/ipv4/inet_connection_sock.c
+++ b/net/ipv4/inet_connection_sock.c
@@ -204,7 +204,8 @@  tb_found:
 			ret = 1;
 			if (inet_csk(sk)->icsk_af_ops->bind_conflict(sk, tb, true)) {
 				if (((sk->sk_reuse && sk->sk_state != TCP_LISTEN) ||
-				     (sk->sk_reuseport && uid_eq(tb->fastuid, uid))) &&
+				     (tb->fastreuseport > 0 &&
+				      sk->sk_reuseport && uid_eq(tb->fastuid, uid))) &&
 				    smallest_size != -1 && --attempts >= 0) {
 					spin_unlock(&head->lock);
 					goto again;
@@ -227,19 +228,15 @@  tb_not_found:
 		if (sk->sk_reuseport) {
 			tb->fastreuseport = 1;
 			tb->fastuid = uid;
-		} else {
+		} else
 			tb->fastreuseport = 0;
-			tb->fastuid = 0;
-		}
 	} else {
 		if (tb->fastreuse &&
 		    (!sk->sk_reuse || sk->sk_state == TCP_LISTEN))
 			tb->fastreuse = 0;
 		if (tb->fastreuseport &&
-		    (!sk->sk_reuseport || !uid_eq(tb->fastuid, uid))) {
+		    (!sk->sk_reuseport || !uid_eq(tb->fastuid, uid)))
 			tb->fastreuseport = 0;
-			tb->fastuid = 0;
-		}
 	}
 success:
 	if (!inet_csk(sk)->icsk_bind_hash)