On Mon, May 17, 2021 at 09:53:30AM -0400, Olga Kornievskaia wrote: > On Sat, May 15, 2021 at 8:42 AM Dan Aloni <dan@xxxxxxxxxxxx> wrote: > > > > On Fri, May 14, 2021 at 10:13:21AM -0400, Olga Kornievskaia wrote: > > > From: Olga Kornievskaia <kolga@xxxxxxxxxx> > > > > > > Allow to query and set the destination's address of a transport. > > > Setting of the destination address is allowed only for TCP or RDMA > > > based connections. > > > > > > Signed-off-by: Olga Kornievskaia <kolga@xxxxxxxxxx> > > .. > > > + saddr = (struct sockaddr *)&xprt->addr; > > > + port = rpc_get_port(saddr); > > > + > > > + dst_addr = kstrndup(buf, count - 1, GFP_KERNEL); > > > + if (!dst_addr) > > > + goto out_err; > > > + saved_addr = kzalloc(sizeof(*saved_addr), GFP_KERNEL); > > > + if (!saved_addr) > > > + goto out_err_free; > > > + saved_addr->addr = > > > + rcu_dereference_raw(xprt->address_strings[RPC_DISPLAY_ADDR]); > > > + rcu_assign_pointer(xprt->address_strings[RPC_DISPLAY_ADDR], dst_addr); > > > + call_rcu(&saved_addr->rcu, free_xprt_addr); > > > + xprt->addrlen = rpc_pton(xprt->xprt_net, buf, count - 1, saddr, > > > + sizeof(*saddr)); > > > > Hi Olga, > > > > How does this behave if rpc_pton fails? Perhaps this conversion being > > also a validation check on input given from user-space should be done > > before the xprt is being modified? > > It's assumed that an administrator is providing a valid (and correct) > address value. Transport would continue to be disconnected until a > proper value is supplied. We can't validate for instance that the > supplied value is a "correct" value for the v4.1 server (which would > require sending an EXCHANGE_ID and checking that it's the same server > as before). I'm not sure it would work that way - looks like `rpc_pton` zeroes `saddr` before trying to fill it (or leaves it as it in other cases such as the INET_ADDRSTRLEN bound check). I think `0.0.0.0` routes to localhost by default? That might cause unexpected behavior from admin PoV. More to consider is that `xprt->address_strings[RPC_DISPLAY_ADDR]` should be consistent with the value of `saddr` in the error cases, otherwise debugging xprt state may be give confusing output. -- Dan Aloni
