| Message ID | ZhkMcZDhJhsVjo52@vm1.arverb.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | mount: If a reserved ports is used, do so for the pings as well | expand |
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). 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);
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); > >
On 4/21/24 12:06 PM, Trond Myklebust wrote: > 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. Well the patch was sitting around for a while without any objection so I figured I would go with it since it would make mounts work on other OSs > > 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. The patch is on the top of stack... easy revert-able... Is that what you are suggesting? steved. > > >> >> 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); >> >> >
On Sun, 2024-04-21 at 17:38 -0400, Steve Dickson wrote: > > > On 4/21/24 12:06 PM, Trond Myklebust wrote: > > 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. > Well the patch was sitting around for a while without any objection > so I figured I would go with it since it would make mounts > work on other OSs > > > > > 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. > The patch is on the top of stack... easy revert-able... Is that what > you are suggesting? That is my suggestion for now, yes. I don't have any objection to a patch that adds opt-in functionality either to turn off the NULL ping, or to force that ping to use a privileged port. However we should not change the default behaviour to cause the existing paucity of privileged ports to be even more of a problem.
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);