netdev - Re: [PATCH v2 net-next] rds-tcp: Add sysctl tunables for sndbuf/rcvbuf on rds-tcp socket

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: <1458062294.31401.37.camel@edumazet-glaptop3.roam.corp.google.com>
Date:	Tue, 15 Mar 2016 10:18:14 -0700
From:	Eric Dumazet <eric.dumazet@...il.com>
To:	Sowmini Varadhan <sowmini.varadhan@...cle.com>
Cc:	santosh shilimkar <santosh.shilimkar@...cle.com>,
	netdev@...r.kernel.org, davem@...emloft.net
Subject: Re: [PATCH v2 net-next] rds-tcp: Add sysctl tunables for
 sndbuf/rcvbuf on rds-tcp socket

On Tue, 2016-03-15 at 12:58 -0400, Sowmini Varadhan wrote:
> On (03/15/16 09:38), santosh shilimkar wrote:
> > >+	if (rtn->sndbuf_size > 0) {
> > So value of 1 is allowed as well. There should be some
> > minimum default or multiple of it. Of course above check
> > can remain as is as long as you validate the user input
> > in handlers.
> 
> yes, just as user-space SO_SNDBUF allows ridiculous values
> for buffer size..

Well... Not really.

> 
> > >@@ -309,6 +352,7 @@ static void __net_exit rds_tcp_exit_net(struct net *net)
> > >  {
> > >  	struct rds_tcp_net *rtn = net_generic(net, rds_tcp_netid);
> > >
> > >+	unregister_net_sysctl_table(rtn->rds_tcp_sysctl);
> > >  	/* If rds_tcp_exit_net() is called as a result of netns deletion,
> > >  	 * the rds_tcp_kill_sock() device notifier would already have cleaned
> > >  	 * up the listen socket, thus there is no work to do in this function.
> > You need to unregister it on rds_tcp_listen_init() failure.
> 
> Ok, good point.
> 
> > >+	rtn->sndbuf_size = user_atoi(buffer, *lenp);
> > As mentioned above, you should make sure the buffer lengths are
> > legitimate.
> 
> As above. We allow any values that the TCP socket itself allows.
> If the user wants to shoot themself in the foot, we dont stop them.

Look at SO_SNDBUF and SO_RCVBUF implementation.

sk->sk_sndbuf = max_t(u32, val * 2, SOCK_MIN_SNDBUF);

sk->sk_rcvbuf = max_t(u32, val * 2, SOCK_MIN_RCVBUF);

kernel definitely has some logic here.

If you believe SOCK_MIN_SNDBUF and/or SOCK_MIN_RCVBUF are wrong, please
elaborate.