Commit 826799e6 authored by J. Bruce Fields's avatar J. Bruce Fields Committed by Trond Myklebust
Browse files

sunrpc: safely reallow resvport min/max inversion 



Commits ffb6ca33 and e08ea3a9 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.

Fixes: ffb6ca33 "sunrpc: Prevent resvport min/max inversion..."
Fixes: e08ea3a9 "sunrpc: Prevent rexvport min/max inversion..."
Signed-off-by: default avatarJ. Bruce Fields <bfields@redhat.com>
Signed-off-by: default avatarTrond Myklebust <trond.myklebust@hammerspace.com>
parent fc187514

net/sunrpc/xprtsock.c

+18 −16
Original line number Diff line number Diff line
@@ -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

	},

	{
@@ -1615,11 +1615,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;

}



/**
@@ -1675,9 +1681,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();
@@ -1698,7 +1704,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;



	/*
@@ -1716,8 +1722,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 {
@@ -3154,12 +3160,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_MAX_RESVPORT);

}

CRA Git | Maintained and supported by SUSTech CRA and CCSE