lkml.org 
[lkml]   [2019]   [Nov]   [27]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[PATCH 4.14 069/211] sunrpc: safely reallow resvport min/max inversion
    Date
    From: J. Bruce Fields <bfields@redhat.com>

    [ Upstream commit 826799e66e8683e5698e140bb9ef69afc8c0014e ]

    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.

    Fixes: ffb6ca33b04b "sunrpc: Prevent resvport min/max inversion..."
    Fixes: e08ea3a96fc7 "sunrpc: Prevent rexvport min/max inversion..."
    Signed-off-by: J. Bruce Fields <bfields@redhat.com>
    Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com>
    Signed-off-by: Sasha Levin <sashal@kernel.org>
    ---
    net/sunrpc/xprtsock.c | 34 ++++++++++++++++++----------------
    1 file changed, 18 insertions(+), 16 deletions(-)

    diff --git a/net/sunrpc/xprtsock.c b/net/sunrpc/xprtsock.c
    index a42871a59f3b9..f75b5b7c1fc2a 100644
    --- a/net/sunrpc/xprtsock.c
    +++ b/net/sunrpc/xprtsock.c
    @@ -127,7 +127,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",
    @@ -135,7 +135,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
    },
    {
    @@ -1754,11 +1754,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;
    }

    /**
    @@ -1815,9 +1821,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();
    @@ -1838,7 +1844,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;

    /*
    @@ -1856,8 +1862,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 {
    @@ -3286,12 +3292,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.20.1


    \
     
     \ /
      Last update: 2019-11-27 22:36    [W:4.036 / U:0.132 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site