I didn't get any feedback on this patch. Maybe the problem was this:
On Mon, Sep 17, 2018 at 11:33:06AM -0400, J. Bruce Fields wrote:
> From: "J. Bruce Fields" <bfields@xxxxxxxxxx>
>
> Commits ffb6ca33b04b and e08ea3a96fc7 prevent setting xprt_min_resvport
> greater than xprt_max_resvport, but may also break simple code that sets
> one parameter then the other, if the new range does not overlap the old.
>
> Also it looks racy to me, unless there's some serialization I'm not
> seeing. Granted it would probably require malicious privileged processes
> (unless there's a chance these might eventually be settable in unprivileged
> containers), but still it seems better not to let userspace panic the
> kernel.
>
> Simpler seems to be to allow setting the parameters to whatever you want
> but interpret xprt_min_resvport > xprt_max_resvport as the empty range.
>
> Untested.
So, I've booted to it and verified that I can set min_resvport and
max_resvport, and that mount fails when min_resvport > max_resvport.
I'll resend.
--b.
>
> Fixes: ffb6ca33b04b "sunrpc: Prevent resvport min/max inversion..."
> Fixes: e08ea3a96fc7 "sunrpc: Prevent rexvport min/max inversion..."
> Signed-off-by: J. Bruce Fields <bfields@xxxxxxxxxx>
> ---
> net/sunrpc/xprtsock.c | 34 ++++++++++++++++++----------------
> 1 file changed, 18 insertions(+), 16 deletions(-)
>
> Whoops, I sent this before but just noticed that I failed to cc the
> list, then that I mispelled the address. Argh.
>
> diff --git a/net/sunrpc/xprtsock.c b/net/sunrpc/xprtsock.c
> index 9e1c5024aba9..5d59de92b5a1 100644
> --- a/net/sunrpc/xprtsock.c
> +++ b/net/sunrpc/xprtsock.c
> @@ -129,7 +129,7 @@ static struct ctl_table xs_tunables_table[] = {
> .mode = 0644,
> .proc_handler = proc_dointvec_minmax,
> .extra1 = &xprt_min_resvport_limit,
> - .extra2 = &xprt_max_resvport
> + .extra2 = &xprt_max_resvport_limit
> },
> {
> .procname = "max_resvport",
> @@ -137,7 +137,7 @@ static struct ctl_table xs_tunables_table[] = {
> .maxlen = sizeof(unsigned int),
> .mode = 0644,
> .proc_handler = proc_dointvec_minmax,
> - .extra1 = &xprt_min_resvport,
> + .extra1 = &xprt_min_resvport_limit,
> .extra2 = &xprt_max_resvport_limit
> },
> {
> @@ -1773,11 +1773,17 @@ static void xs_udp_timer(struct rpc_xprt *xprt, struct rpc_task *task)
> spin_unlock_bh(&xprt->transport_lock);
> }
>
> -static unsigned short xs_get_random_port(void)
> +static int xs_get_random_port(void)
> {
> - unsigned short range = xprt_max_resvport - xprt_min_resvport + 1;
> - unsigned short rand = (unsigned short) prandom_u32() % range;
> - return rand + xprt_min_resvport;
> + unsigned short min = xprt_min_resvport, max = xprt_max_resvport;
> + unsigned short range;
> + unsigned short rand;
> +
> + if (max < min)
> + return -EADDRINUSE;
> + range = max - min + 1;
> + rand = (unsigned short) prandom_u32() % range;
> + return rand + min;
> }
>
> /**
> @@ -1833,9 +1839,9 @@ static void xs_set_srcport(struct sock_xprt *transport, struct socket *sock)
> transport->srcport = xs_sock_getport(sock);
> }
>
> -static unsigned short xs_get_srcport(struct sock_xprt *transport)
> +static int xs_get_srcport(struct sock_xprt *transport)
> {
> - unsigned short port = transport->srcport;
> + int port = transport->srcport;
>
> if (port == 0 && transport->xprt.resvport)
> port = xs_get_random_port();
> @@ -1856,7 +1862,7 @@ static int xs_bind(struct sock_xprt *transport, struct socket *sock)
> {
> struct sockaddr_storage myaddr;
> int err, nloop = 0;
> - unsigned short port = xs_get_srcport(transport);
> + int port = xs_get_srcport(transport);
> unsigned short last;
>
> /*
> @@ -1874,8 +1880,8 @@ static int xs_bind(struct sock_xprt *transport, struct socket *sock)
> * transport->xprt.resvport == 1) xs_get_srcport above will
> * ensure that port is non-zero and we will bind as needed.
> */
> - if (port == 0)
> - return 0;
> + if (port <= 0)
> + return port;
>
> memcpy(&myaddr, &transport->srcaddr, transport->xprt.addrlen);
> do {
> @@ -3317,12 +3323,8 @@ static int param_set_uint_minmax(const char *val,
>
> static int param_set_portnr(const char *val, const struct kernel_param *kp)
> {
> - if (kp->arg == &xprt_min_resvport)
> - return param_set_uint_minmax(val, kp,
> - RPC_MIN_RESVPORT,
> - xprt_max_resvport);
> return param_set_uint_minmax(val, kp,
> - xprt_min_resvport,
> + RPC_MIN_RESVPORT,
> RPC_MAX_RESVPORT);
> }
>
> --
> 2.17.1
>
