On Sun, 2024-04-21 at 07:09 -0400, Steve Dickson wrote:
>
>
> On 4/12/24 6:26 AM, Alexandre Ratchov wrote:
> > Hi,
> >
> > mount.nfs always uses a high port to probe the server's ports
> > (regardless of
> > the "-o resvport" option). Certain NFS servers (ex. OpenBSD -
> > current) will
> > drop the connection, the probe will fail, and mount.nfs will exit
> > before any
> > attempt to mount the file-system. If mount.nfs doesn't ping the
> > server from
> > a high port, mounting the file system will just work.
> >
> > Note that the same will happen if the server is behind a firewall
> > that
> > blocks connections to the NFS service that originates from a high
> > port.
> Committed... (tag: nfs-utils-2-7-1-rc7)
>
> I just hope we don't run out of privilege ports during
> a mount storm (aka when a server reboots).
Agreed, and that is why this change was entirely the wrong thing to do.
The point of the ping is to allow for fast failover in the case where
the portmap/rpcbind server returns incorrect or stale information.
If there are servers out there that deliberately break the convention
for NULL ping, as described in RFC5531, then we might allow optional
use of the privileged port for those servers, but please don't force
this on everyone else.
>
> steved.
>
> >
> > ---
> > support/include/nfsrpc.h | 3 ++-
> > support/nfs/getport.c | 10 +++++++---
> > utils/mount/network.c | 36 ++++++++++++++++++++---------------
> > -
> > utils/mount/network.h | 4 ++--
> > utils/mount/nfsmount.c | 16 +++++++++++-----
> > utils/mount/stropts.c | 24 +++++++++++++++++++++---
> > 6 files changed, 63 insertions(+), 30 deletions(-)
> >
> > diff --git a/support/include/nfsrpc.h b/support/include/nfsrpc.h
> > index fbbdb6a9..9106c195 100644
> > --- a/support/include/nfsrpc.h
> > +++ b/support/include/nfsrpc.h
> > @@ -170,7 +170,8 @@ extern int nfs_rpc_ping(const struct
> > sockaddr *sap,
> > const rpcprog_t program,
> > const rpcvers_t version,
> > const unsigned short protocol,
> > - const struct timeval *timeout);
> > + const struct timeval *timeout,
> > + const int resvport);
> >
> > /* create AUTH_SYS handle with no supplemental groups */
> > extern AUTH * nfs_authsys_create(void);
> > diff --git a/support/nfs/getport.c b/support/nfs/getport.c
> > index 813f7bf9..ff7e9991 100644
> > --- a/support/nfs/getport.c
> > +++ b/support/nfs/getport.c
> > @@ -730,7 +730,8 @@ static unsigned short nfs_gp_getport(CLIENT
> > *client,
> > */
> > int nfs_rpc_ping(const struct sockaddr *sap, const socklen_t
> > salen,
> > const rpcprog_t program, const rpcvers_t version,
> > - const unsigned short protocol, const struct
> > timeval *timeout)
> > + const unsigned short protocol, const struct
> > timeval *timeout,
> > + const int resvport)
> > {
> > union nfs_sockaddr address;
> > struct sockaddr *saddr = &address.sa;
> > @@ -744,8 +745,11 @@ int nfs_rpc_ping(const struct sockaddr *sap,
> > const socklen_t salen,
> > nfs_clear_rpc_createerr();
> >
> > memcpy(saddr, sap, (size_t)salen);
> > - client = nfs_get_rpcclient(saddr, salen, protocol,
> > - program, version,
> > &tout);
> > + client = resvport ?
> > + nfs_get_priv_rpcclient(saddr, salen, protocol,
> > + program, version, &tout) :
> > + nfs_get_rpcclient(saddr, salen, protocol,
> > + program, version, &tout);
> > if (client != NULL) {
> > result = nfs_gp_ping(client, tout);
> > nfs_gp_map_tcp_errorcodes(protocol);
> > diff --git a/utils/mount/network.c b/utils/mount/network.c
> > index 01ead49f..d9221567 100644
> > --- a/utils/mount/network.c
> > +++ b/utils/mount/network.c
> > @@ -525,7 +525,7 @@ static void nfs_pp_debug2(const char *str)
> > */
> > static int nfs_probe_port(const struct sockaddr *sap, const
> > socklen_t salen,
> > struct pmap *pmap, const unsigned long
> > *versions,
> > - const unsigned int *protos)
> > + const unsigned int *protos, const int
> > resvp)
> > {
> > union nfs_sockaddr address;
> > struct sockaddr *saddr = &address.sa;
> > @@ -550,7 +550,7 @@ static int nfs_probe_port(const struct sockaddr
> > *sap, const socklen_t salen,
> > nfs_pp_debug(saddr, salen, prog,
> > *p_vers,
> > *p_prot, p_port);
> > if (nfs_rpc_ping(saddr, salen,
> > prog,
> > - *p_vers,
> > *p_prot, NULL))
> > + *p_vers, *p_prot,
> > NULL, resvp))
> > goto out_ok;
> > } else
> > rpc_createerr.cf_stat =
> > RPC_PROGNOTREGISTERED;
> > @@ -603,7 +603,7 @@ out_ok:
> > * returned; rpccreateerr.cf_stat is set to reflect the nature of
> > the error.
> > */
> > static int nfs_probe_nfsport(const struct sockaddr *sap, const
> > socklen_t salen,
> > - struct pmap *pmap, int checkv4)
> > + struct pmap *pmap, int checkv4, int
> > resvp)
> > {
> > if (pmap->pm_vers && pmap->pm_prot && pmap->pm_port)
> > return 1;
> > @@ -617,13 +617,13 @@ static int nfs_probe_nfsport(const struct
> > sockaddr *sap, const socklen_t salen,
> > memcpy(&save_sa, sap, salen);
> >
> > ret = nfs_probe_port(sap, salen, pmap,
> > - probe_nfs3_only,
> > probe_proto);
> > + probe_nfs3_only, probe_proto,
> > resvp);
> > if (!ret || !checkv4 || probe_proto !=
> > probe_tcp_first)
> > return ret;
> >
> > nfs_set_port((struct sockaddr *)&save_sa,
> > NFS_PORT);
> > ret = nfs_rpc_ping((struct sockaddr *)&save_sa,
> > salen,
> > - NFS_PROGRAM, 4, IPPROTO_TCP, NULL);
> > + NFS_PROGRAM, 4, IPPROTO_TCP,
> > NULL, resvp);
> > if (ret) {
> > rpc_createerr.cf_stat = RPC_FAILED;
> > rpc_createerr.cf_error.re_errno = EAGAIN;
> > @@ -632,7 +632,7 @@ static int nfs_probe_nfsport(const struct
> > sockaddr *sap, const socklen_t salen,
> > return 1;
> > } else
> > return nfs_probe_port(sap, salen, pmap,
> > - probe_nfs2_only,
> > probe_udp_only);
> > + probe_nfs2_only,
> > probe_udp_only, resvp);
> > }
> >
> > /*
> > @@ -649,17 +649,17 @@ static int nfs_probe_nfsport(const struct
> > sockaddr *sap, const socklen_t salen,
> > * returned; rpccreateerr.cf_stat is set to reflect the nature of
> > the error.
> > */
> > static int nfs_probe_mntport(const struct sockaddr *sap, const
> > socklen_t salen,
> > - struct pmap *pmap)
> > + struct pmap *pmap)
> > {
> > if (pmap->pm_vers && pmap->pm_prot && pmap->pm_port)
> > return 1;
> >
> > if (nfs_mount_data_version >= 4)
> > return nfs_probe_port(sap, salen, pmap,
> > - probe_mnt3_only,
> > probe_udp_first);
> > + probe_mnt3_only,
> > probe_udp_first, 0);
> > else
> > return nfs_probe_port(sap, salen, pmap,
> > - probe_mnt1_first,
> > probe_udp_only);
> > + probe_mnt1_first,
> > probe_udp_only, 0);
> > }
> >
> > /*
> > @@ -676,9 +676,10 @@ static int nfs_probe_version_fixed(const
> > struct sockaddr *mnt_saddr,
> > struct pmap *mnt_pmap,
> > const struct sockaddr *nfs_saddr,
> > const socklen_t nfs_salen,
> > - struct pmap *nfs_pmap)
> > + struct pmap *nfs_pmap,
> > + const int resvp)
> > {
> > - if (!nfs_probe_nfsport(nfs_saddr, nfs_salen, nfs_pmap, 0))
> > + if (!nfs_probe_nfsport(nfs_saddr, nfs_salen, nfs_pmap, 0,
> > resvp))
> > return 0;
> > return nfs_probe_mntport(mnt_saddr, mnt_salen, mnt_pmap);
> > }
> > @@ -706,7 +707,8 @@ int nfs_probe_bothports(const struct sockaddr
> > *mnt_saddr,
> > const struct sockaddr *nfs_saddr,
> > const socklen_t nfs_salen,
> > struct pmap *nfs_pmap,
> > - int checkv4)
> > + int checkv4,
> > + int resvp)
> > {
> > struct pmap save_nfs, save_mnt;
> > const unsigned long *probe_vers;
> > @@ -718,7 +720,8 @@ int nfs_probe_bothports(const struct sockaddr
> > *mnt_saddr,
> >
> > if (nfs_pmap->pm_vers)
> > return nfs_probe_version_fixed(mnt_saddr,
> > mnt_salen, mnt_pmap,
> > - nfs_saddr,
> > nfs_salen, nfs_pmap);
> > + nfs_saddr,
> > nfs_salen, nfs_pmap,
> > + resvp);
> >
> > memcpy(&save_nfs, nfs_pmap, sizeof(save_nfs));
> > memcpy(&save_mnt, mnt_pmap, sizeof(save_mnt));
> > @@ -727,7 +730,8 @@ int nfs_probe_bothports(const struct sockaddr
> > *mnt_saddr,
> >
> > for (; *probe_vers; probe_vers++) {
> > nfs_pmap->pm_vers = mntvers_to_nfs(*probe_vers);
> > - if (nfs_probe_nfsport(nfs_saddr, nfs_salen,
> > nfs_pmap, checkv4) != 0) {
> > + if (nfs_probe_nfsport(nfs_saddr, nfs_salen,
> > nfs_pmap, checkv4,
> > + resvp) != 0) {
> > mnt_pmap->pm_vers = *probe_vers;
> > if (nfs_probe_mntport(mnt_saddr,
> > mnt_salen, mnt_pmap) != 0)
> > return 1;
> > @@ -759,7 +763,7 @@ int nfs_probe_bothports(const struct sockaddr
> > *mnt_saddr,
> > * Otherwise zero is returned; rpccreateerr.cf_stat is set to
> > reflect
> > * the nature of the error.
> > */
> > -int probe_bothports(clnt_addr_t *mnt_server, clnt_addr_t
> > *nfs_server)
> > +int probe_bothports(clnt_addr_t *mnt_server, clnt_addr_t
> > *nfs_server, int resvp)
> > {
> > struct sockaddr *mnt_addr = SAFE_SOCKADDR(&mnt_server-
> > >saddr);
> > struct sockaddr *nfs_addr = SAFE_SOCKADDR(&nfs_server-
> > >saddr);
> > @@ -767,7 +771,7 @@ int probe_bothports(clnt_addr_t *mnt_server,
> > clnt_addr_t *nfs_server)
> > return nfs_probe_bothports(mnt_addr, sizeof(mnt_server-
> > >saddr),
> > &mnt_server->pmap,
> > nfs_addr,
> > sizeof(nfs_server->saddr),
> > - &nfs_server->pmap, 0);
> > + &nfs_server->pmap, 0,
> > resvp);
> > }
> >
> > /**
> > diff --git a/utils/mount/network.h b/utils/mount/network.h
> > index 0fc98acd..8bcc7ace 100644
> > --- a/utils/mount/network.h
> > +++ b/utils/mount/network.h
> > @@ -39,10 +39,10 @@ typedef struct {
> > static const struct timeval TIMEOUT = { 20, 0 };
> > static const struct timeval RETRY_TIMEOUT = { 3, 0 };
> >
> > -int probe_bothports(clnt_addr_t *, clnt_addr_t *);
> > +int probe_bothports(clnt_addr_t *, clnt_addr_t *, int);
> > int nfs_probe_bothports(const struct sockaddr *, const socklen_t,
> > struct pmap *, const struct sockaddr *,
> > - const socklen_t, struct pmap *, int);
> > + const socklen_t, struct pmap *, int, int);
> > int nfs_gethostbyname(const char *, struct sockaddr_in *);
> > int nfs_lookup(const char *hostname, const sa_family_t family,
> > struct sockaddr *sap, socklen_t *salen);
> > diff --git a/utils/mount/nfsmount.c b/utils/mount/nfsmount.c
> > index 3d95da94..a792c6e7 100644
> > --- a/utils/mount/nfsmount.c
> > +++ b/utils/mount/nfsmount.c
> > @@ -123,13 +123,13 @@ nfs2_mount(CLIENT *clnt, mnt2arg_t *mnt2arg,
> > mnt2res_t *mnt2res)
> >
> > static int
> > nfs_call_mount(clnt_addr_t *mnt_server, clnt_addr_t *nfs_server,
> > - mntarg_t *mntarg, mntres_t *mntres)
> > + mntarg_t *mntarg, mntres_t *mntres, int resvp)
> > {
> > CLIENT *clnt;
> > enum clnt_stat stat;
> > int msock;
> >
> > - if (!probe_bothports(mnt_server, nfs_server))
> > + if (!probe_bothports(mnt_server, nfs_server, resvp))
> > goto out_bad;
> >
> > clnt = mnt_openclnt(mnt_server, &msock);
> > @@ -164,7 +164,8 @@ nfs_call_mount(clnt_addr_t *mnt_server,
> > clnt_addr_t *nfs_server,
> > static int
> > parse_options(char *old_opts, struct nfs_mount_data *data,
> > int *bg, int *retry, clnt_addr_t *mnt_server,
> > - clnt_addr_t *nfs_server, char *new_opts, const int
> > opt_size)
> > + clnt_addr_t *nfs_server, char *new_opts, const int
> > opt_size,
> > + int *resvp)
> > {
> > struct sockaddr_in *mnt_saddr = &mnt_server->saddr;
> > struct pmap *mnt_pmap = &mnt_server->pmap;
> > @@ -177,6 +178,7 @@ parse_options(char *old_opts, struct
> > nfs_mount_data *data,
> >
> > data->flags = 0;
> > *bg = 0;
> > + *resvp = 1;
> >
> > len = strlen(new_opts);
> > tmp_opts = xstrdup(old_opts);
> > @@ -365,6 +367,8 @@ parse_options(char *old_opts, struct
> > nfs_mount_data *data,
> > data->flags &= ~NFS_MOUNT_NOAC;
> > if (!val)
> > data->flags |=
> > NFS_MOUNT_NOAC;
> > + } else if (!strcmp(opt, "resvport")) {
> > + *resvp = val;
> > #if NFS_MOUNT_VERSION >= 2
> > } else if (!strcmp(opt, "tcp")) {
> > data->flags &= ~NFS_MOUNT_TCP;
> > @@ -498,6 +502,7 @@ nfsmount(const char *spec, const char *node,
> > int flags,
> > static struct nfs_mount_data data;
> > int val;
> > static int doonce = 0;
> > + int resvp;
> >
> > clnt_addr_t mnt_server = {
> > .hostname = &mounthost
> > @@ -582,7 +587,7 @@ nfsmount(const char *spec, const char *node,
> > int flags,
> > /* parse options */
> > new_opts[0] = 0;
> > if (!parse_options(old_opts, &data, &bg, &retry,
> > &mnt_server, &nfs_server,
> > - new_opts, sizeof(new_opts)))
> > + new_opts, sizeof(new_opts), &resvp))
> > goto fail;
> > if (!nfsmnt_check_compat(nfs_pmap, mnt_pmap))
> > goto fail;
> > @@ -620,6 +625,7 @@ nfsmount(const char *spec, const char *node,
> > int flags,
> > #if NFS_MOUNT_VERSION >= 5
> > printf(_(", sec = %u"), data.pseudoflavor);
> > printf(_(", readdirplus = %d"), (data.flags &
> > NFS_MOUNT_NORDIRPLUS) != 0);
> > + printf(_(", resvp = %u"), resvp);
> > #endif
> > printf("\n");
> > #endif
> > @@ -670,7 +676,7 @@ nfsmount(const char *spec, const char *node,
> > int flags,
> > sleep(30);
> >
> > stat = nfs_call_mount(&mnt_server,
> > &nfs_server,
> > - &dirname, &mntres);
> > + &dirname, &mntres,
> > resvp);
> > if (stat)
> > break;
> > memcpy(nfs_pmap, &save_nfs,
> > sizeof(*nfs_pmap));
> > diff --git a/utils/mount/stropts.c b/utils/mount/stropts.c
> > index a92c4200..85b8ca5a 100644
> > --- a/utils/mount/stropts.c
> > +++ b/utils/mount/stropts.c
> > @@ -337,6 +337,20 @@ static int nfs_verify_lock_option(struct
> > mount_options *options)
> > return 1;
> > }
> >
> > +static const char *nfs_resvport_opttbl[] = {
> > + "noresvport",
> > + "resvport",
> > + NULL,
> > +};
> > +
> > +/*
> > + * Returns true unless "noresvport" is set
> > + */
> > +static int nfs_resvport_option(struct mount_options *options)
> > +{
> > + return po_rightmost(options, nfs_resvport_opttbl) != 0;
> > +}
> > +
> > static int nfs_insert_sloppy_option(struct mount_options
> > *options)
> > {
> > if (linux_version_code() < MAKE_VERSION(2, 6, 27))
> > @@ -550,7 +564,7 @@ static int nfs_construct_new_options(struct
> > mount_options *options,
> > * FALSE is returned if some failure occurred.
> > */
> > static int
> > -nfs_rewrite_pmap_mount_options(struct mount_options *options, int
> > checkv4)
> > +nfs_rewrite_pmap_mount_options(struct mount_options *options, int
> > checkv4, int resvp)
> > {
> > union nfs_sockaddr nfs_address;
> > struct sockaddr *nfs_saddr = &nfs_address.sa;
> > @@ -604,7 +618,8 @@ nfs_rewrite_pmap_mount_options(struct
> > mount_options *options, int checkv4)
> > * negotiate. Bail now if we can't contact it.
> > */
> > if (!nfs_probe_bothports(mnt_saddr, mnt_salen, &mnt_pmap,
> > - nfs_saddr, nfs_salen, &nfs_pmap,
> > checkv4)) {
> > + nfs_saddr, nfs_salen, &nfs_pmap,
> > + checkv4, resvp)) {
> > errno = ESPIPE;
> > if (rpc_createerr.cf_stat ==
> > RPC_PROGNOTREGISTERED)
> > errno = EOPNOTSUPP;
> > @@ -670,6 +685,7 @@ static int nfs_do_mount_v3v2(struct
> > nfsmount_info *mi,
> > {
> > struct mount_options *options = po_dup(mi->options);
> > int result = 0;
> > + int resvp;
> >
> > if (!options) {
> > errno = ENOMEM;
> > @@ -704,11 +720,13 @@ static int nfs_do_mount_v3v2(struct
> > nfsmount_info *mi,
> > goto out_fail;
> > }
> >
> > + resvp = nfs_resvport_option(options);
> > +
> > if (verbose)
> > printf(_("%s: trying text-based options '%s'\n"),
> > progname, *mi->extra_opts);
> >
> > - if (!nfs_rewrite_pmap_mount_options(options, checkv4))
> > + if (!nfs_rewrite_pmap_mount_options(options, checkv4,
> > resvp))
> > goto out_fail;
> >
> > result = nfs_sys_mount(mi, options);
>
>
--
Trond Myklebust
CTO, Hammerspace Inc
1900 S Norfolk St, Suite 350 - #45
San Mateo, CA 94403
www.hammerspace.com
