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.
