netdev - Re: [PATCH] netns: correctly use per-netns ipv4 sysctl_tcp

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: <20120709.152100.571089964662155300.davem@davemloft.net>
Date:	Mon, 09 Jul 2012 15:21:00 -0700 (PDT)
From:	David Miller <davem@...emloft.net>
To:	h.huangqiang@...wei.com
Cc:	glommer@...allels.com, netdev@...r.kernel.org,
	containers@...ts.linux-foundation.org, yangzhenzhang@...wei.com
Subject: Re: [PATCH] netns: correctly use per-netns ipv4 sysctl_tcp_mem

From: Huang Qiang <h.huangqiang@...wei.com>
Date: Mon, 9 Jul 2012 14:05:09 +0800

> From: Yang Zhenzhang <yangzhenzhang@...wei.com>
> 
> Now, kernel allows each net namespace to independently set up its levels
> for tcp memory pressure thresholds.
> 
> But it seems there is a bug, as using the following steps:
> 
> [root@...t socket]# lxc-start -n test -f config /bin/bash
> [root@...-test socket]# ip route add default via 192.168.58.2
> [root@...-test socket]# echo 0 0 0 > /proc/sys/net/ipv4/tcp_mem
> [root@...-test socket]# scp root@....168.58.174:/home/tcp_mem_test .
> 
> and it still can transport the "tcp_mem_test" file which we hope it
> would not.
> 
> It's because inet_init() (net/ipv4/af_inet.c)initialize the
> tcp_prot.sysctl_mem:
> tcp_prot.sysctl_mem = init_net.ipv4.sysctl_tcp_mem;
> 
> So when the protocal is TCP, sk->sk_prot->sysctl_mem(following code)
> always use the ipv4 sysctl_tcp_mem of init_net namespace rather than
> it's own net namespace.
> This patch simply set "prot" equal to net->ipv4.sysctl_tcp_mem when
> the protocol type is TCP.
> 
> Signed-off-by: Yang Zhenzhang <yangzhenzhang@...wei.com>

Another regression added by the socket memory cgroup code, BIG
SURPRISE.

But this patch is even worse than the bug.

>  	long *prot = sk->sk_prot->sysctl_mem;
> +	
   ^^^^^^^^

Trailing whitespace please remove.

Also, this patch causes build failures:

In file included from include/linux/tcp.h:227:0,
                 from include/linux/ipv6.h:221,
                 from include/net/ipv6.h:16,
                 from include/linux/sunrpc/clnt.h:26,
                 from include/linux/nfs_fs.h:57,
                 from init/do_mounts.c:30:
include/net/sock.h: In function ‘sk_prot_mem_limits’:
include/net/sock.h:1066:9: error: implicit declaration of function ‘sock_net’ [-Werror=implicit-function-declaration]
include/net/sock.h:1066:20: warning: initialization makes pointer from integer without a cast [enabled by default]
include/net/sock.h:1069:8: error: ‘const struct sock’ has no member named ‘protocol’
include/net/sock.h: At top level:
include/net/sock.h:2136:13: error: conflicting types for ‘sock_net’
include/net/sock.h:1066:20: note: previous implicit declaration of ‘sock_net’ was here
cc1: some warnings being treated as errors

It is basically impossible that you even compile tested this patch
because you're using an interface from the same header file before
it's even defined.

This is an incredibly poor patch submission, I hate to tell you.