2024-02-25 23:44:25

by NeilBrown

[permalink] [raw]
Subject: [PATCH 2/3] Change local_rpcb() to take a targaddr pointer.

Two callers of local_rpcb() want the target-addr, and local_rcpb() has
easy access to it. So accept a pointer and fill it in if not NULL.

Signed-off-by: NeilBrown <[email protected]>
---
src/rpcb_clnt.c | 35 +++++++++++------------------------
1 file changed, 11 insertions(+), 24 deletions(-)

diff --git a/src/rpcb_clnt.c b/src/rpcb_clnt.c
index 68fe69a320ff..f587580228ab 100644
--- a/src/rpcb_clnt.c
+++ b/src/rpcb_clnt.c
@@ -89,7 +89,7 @@ static struct address_cache *copy_of_cached(const char *, char *);
static void delete_cache(struct netbuf *);
static void add_cache(const char *, const char *, struct netbuf *, char *);
static CLIENT *getclnthandle(const char *, const struct netconfig *, char **);
-static CLIENT *local_rpcb(void);
+static CLIENT *local_rpcb(char **targaddr);
#ifdef NOTUSED
static struct netbuf *got_entry(rpcb_entry_list_ptr, const struct netconfig *);
#endif
@@ -430,19 +430,12 @@ getclnthandle(host, nconf, targaddr)
nconf->nc_netid, si.si_af, si.si_proto, si.si_socktype));

if (nconf->nc_protofmly != NULL && strcmp(nconf->nc_protofmly, NC_LOOPBACK) == 0) {
- client = local_rpcb();
+ client = local_rpcb(targaddr);
if (! client) {
LIBTIRPC_DEBUG(1, ("getclnthandle: %s",
clnt_spcreateerror("local_rpcb failed")));
goto out_err;
} else {
- struct sockaddr_un sun;
-
- if (targaddr) {
- *targaddr = malloc(sizeof(sun.sun_path));
- strncpy(*targaddr, _PATH_RPCBINDSOCK,
- sizeof(sun.sun_path));
- }
return (client);
}
} else {
@@ -541,7 +534,8 @@ getpmaphandle(nconf, hostname, tgtaddr)
* rpcbind. Returns NULL on error and free's everything.
*/
static CLIENT *
-local_rpcb()
+local_rpcb(targaddr)
+ char **targaddr;
{
CLIENT *client;
static struct netconfig *loopnconf;
@@ -574,6 +568,8 @@ local_rpcb()
if (client != NULL) {
/* Mark the socket to be closed in destructor */
(void) CLNT_CONTROL(client, CLSET_FD_CLOSE, NULL);
+ if (targaddr)
+ *targaddr = strdup(sun.sun_path);
return client;
}

@@ -632,7 +628,7 @@ try_nconf:
endnetconfig(nc_handle);
}
mutex_unlock(&loopnconf_lock);
- client = getclnthandle(hostname, loopnconf, NULL);
+ client = getclnthandle(hostname, loopnconf, targaddr);
return (client);
}

@@ -661,20 +657,11 @@ rpcb_set(program, version, nconf, address)
rpc_createerr.cf_stat = RPC_UNKNOWNADDR;
return (FALSE);
}
- client = local_rpcb();
+ client = local_rpcb(&parms.r_addr);
if (! client) {
return (FALSE);
}

- /* convert to universal */
- /*LINTED const castaway*/
- parms.r_addr = taddr2uaddr((struct netconfig *) nconf,
- (struct netbuf *)address);
- if (!parms.r_addr) {
- CLNT_DESTROY(client);
- rpc_createerr.cf_stat = RPC_N2AXLATEFAILURE;
- return (FALSE); /* no universal address */
- }
parms.r_prog = program;
parms.r_vers = version;
parms.r_netid = nconf->nc_netid;
@@ -712,7 +699,7 @@ rpcb_unset(program, version, nconf)
RPCB parms;
char uidbuf[32];

- client = local_rpcb();
+ client = local_rpcb(NULL);
if (! client) {
return (FALSE);
}
@@ -1342,7 +1329,7 @@ rpcb_taddr2uaddr(nconf, taddr)
rpc_createerr.cf_stat = RPC_UNKNOWNADDR;
return (NULL);
}
- client = local_rpcb();
+ client = local_rpcb(NULL);
if (! client) {
return (NULL);
}
@@ -1376,7 +1363,7 @@ rpcb_uaddr2taddr(nconf, uaddr)
rpc_createerr.cf_stat = RPC_UNKNOWNADDR;
return (NULL);
}
- client = local_rpcb();
+ client = local_rpcb(NULL);
if (! client) {
return (NULL);
}
--
2.43.0



2024-03-09 14:27:22

by Steve Dickson

[permalink] [raw]
Subject: Re: [PATCH 2/3] Change local_rpcb() to take a targaddr pointer.



On 2/25/24 6:40 PM, NeilBrown wrote:
> Two callers of local_rpcb() want the target-addr, and local_rcpb() has
> easy access to it. So accept a pointer and fill it in if not NULL.
>
> Signed-off-by: NeilBrown <[email protected]>
> ---
> src/rpcb_clnt.c | 35 +++++++++++------------------------
> 1 file changed, 11 insertions(+), 24 deletions(-)
>
> diff --git a/src/rpcb_clnt.c b/src/rpcb_clnt.c
> index 68fe69a320ff..f587580228ab 100644
> --- a/src/rpcb_clnt.c
> +++ b/src/rpcb_clnt.c
> @@ -89,7 +89,7 @@ static struct address_cache *copy_of_cached(const char *, char *);
> static void delete_cache(struct netbuf *);
> static void add_cache(const char *, const char *, struct netbuf *, char *);
> static CLIENT *getclnthandle(const char *, const struct netconfig *, char **);
> -static CLIENT *local_rpcb(void);
> +static CLIENT *local_rpcb(char **targaddr);
> #ifdef NOTUSED
> static struct netbuf *got_entry(rpcb_entry_list_ptr, const struct netconfig *);
> #endif
> @@ -430,19 +430,12 @@ getclnthandle(host, nconf, targaddr)
> nconf->nc_netid, si.si_af, si.si_proto, si.si_socktype));
>
> if (nconf->nc_protofmly != NULL && strcmp(nconf->nc_protofmly, NC_LOOPBACK) == 0) {
> - client = local_rpcb();
> + client = local_rpcb(targaddr);
> if (! client) {
> LIBTIRPC_DEBUG(1, ("getclnthandle: %s",
> clnt_spcreateerror("local_rpcb failed")));
> goto out_err;
> } else {
> - struct sockaddr_un sun;
> -
> - if (targaddr) {
> - *targaddr = malloc(sizeof(sun.sun_path));
> - strncpy(*targaddr, _PATH_RPCBINDSOCK,
> - sizeof(sun.sun_path));
> - }
> return (client);
> }
> } else {
> @@ -541,7 +534,8 @@ getpmaphandle(nconf, hostname, tgtaddr)
> * rpcbind. Returns NULL on error and free's everything.
> */
> static CLIENT *
> -local_rpcb()
> +local_rpcb(targaddr)
> + char **targaddr;
> {
> CLIENT *client;
> static struct netconfig *loopnconf;
> @@ -574,6 +568,8 @@ local_rpcb()
> if (client != NULL) {
> /* Mark the socket to be closed in destructor */
> (void) CLNT_CONTROL(client, CLSET_FD_CLOSE, NULL);
> + if (targaddr)
> + *targaddr = strdup(sun.sun_path);
> return client;
> }
>
> @@ -632,7 +628,7 @@ try_nconf:
> endnetconfig(nc_handle);
> }
> mutex_unlock(&loopnconf_lock);
> - client = getclnthandle(hostname, loopnconf, NULL);
> + client = getclnthandle(hostname, loopnconf, targaddr);
> return (client);
> }
>
> @@ -661,20 +657,11 @@ rpcb_set(program, version, nconf, address)
> rpc_createerr.cf_stat = RPC_UNKNOWNADDR;
> return (FALSE);
> }
> - client = local_rpcb();
> + client = local_rpcb(&parms.r_addr);
> if (! client) {
> return (FALSE);
> }
>
> - /* convert to universal */
> - /*LINTED const castaway*/
> - parms.r_addr = taddr2uaddr((struct netconfig *) nconf,
> - (struct netbuf *)address);
> - if (!parms.r_addr) {
> - CLNT_DESTROY(client);
> - rpc_createerr.cf_stat = RPC_N2AXLATEFAILURE;
> - return (FALSE); /* no universal address */
> - }
> parms.r_prog = program;
> parms.r_vers = version;
> parms.r_netid = nconf->nc_netid;
> @@ -712,7 +699,7 @@ rpcb_unset(program, version, nconf)
> RPCB parms;
> char uidbuf[32];
>
> - client = local_rpcb();
> + client = local_rpcb(NULL);
> if (! client) {
> return (FALSE);
> }
> @@ -1342,7 +1329,7 @@ rpcb_taddr2uaddr(nconf, taddr)
> rpc_createerr.cf_stat = RPC_UNKNOWNADDR;
> return (NULL);
> }
> - client = local_rpcb();
> + client = local_rpcb(NULL);
> if (! client) {
> return (NULL);
> }
> @@ -1376,7 +1363,7 @@ rpcb_uaddr2taddr(nconf, uaddr)
> rpc_createerr.cf_stat = RPC_UNKNOWNADDR;
> return (NULL);
> }
> - client = local_rpcb();
> + client = local_rpcb(NULL);
> if (! client) {
> return (NULL);
> }
It is not clear why... but this patch stop mountd from
registering with rpcbind (both the old and changed via
the latest patches), which means v3 mounts break.
Not good :-)

Turning debugging on... rpcbind is receiving the set prog
but not recording it, since port 0 is returned when
the client tries to do a v3 mount.

Are you guys seeing this??

I remove this patch and everything works!

steved.


2024-03-11 01:17:57

by NeilBrown

[permalink] [raw]
Subject: Re: [PATCH 2/3] Change local_rpcb() to take a targaddr pointer.

On Sun, 10 Mar 2024, Steve Dickson wrote:
>
> On 2/25/24 6:40 PM, NeilBrown wrote:
> > Two callers of local_rpcb() want the target-addr, and local_rcpb() has
> > easy access to it. So accept a pointer and fill it in if not NULL.
> >
> > Signed-off-by: NeilBrown <[email protected]>
> > ---
> > src/rpcb_clnt.c | 35 +++++++++++------------------------
> > 1 file changed, 11 insertions(+), 24 deletions(-)
> >
> > diff --git a/src/rpcb_clnt.c b/src/rpcb_clnt.c
> > index 68fe69a320ff..f587580228ab 100644
> > --- a/src/rpcb_clnt.c
> > +++ b/src/rpcb_clnt.c
> > @@ -89,7 +89,7 @@ static struct address_cache *copy_of_cached(const char *, char *);
> > static void delete_cache(struct netbuf *);
> > static void add_cache(const char *, const char *, struct netbuf *, char *);
> > static CLIENT *getclnthandle(const char *, const struct netconfig *, char **);
> > -static CLIENT *local_rpcb(void);
> > +static CLIENT *local_rpcb(char **targaddr);
> > #ifdef NOTUSED
> > static struct netbuf *got_entry(rpcb_entry_list_ptr, const struct netconfig *);
> > #endif
> > @@ -430,19 +430,12 @@ getclnthandle(host, nconf, targaddr)
> > nconf->nc_netid, si.si_af, si.si_proto, si.si_socktype));
> >
> > if (nconf->nc_protofmly != NULL && strcmp(nconf->nc_protofmly, NC_LOOPBACK) == 0) {
> > - client = local_rpcb();
> > + client = local_rpcb(targaddr);
> > if (! client) {
> > LIBTIRPC_DEBUG(1, ("getclnthandle: %s",
> > clnt_spcreateerror("local_rpcb failed")));
> > goto out_err;
> > } else {
> > - struct sockaddr_un sun;
> > -
> > - if (targaddr) {
> > - *targaddr = malloc(sizeof(sun.sun_path));
> > - strncpy(*targaddr, _PATH_RPCBINDSOCK,
> > - sizeof(sun.sun_path));
> > - }
> > return (client);
> > }
> > } else {
> > @@ -541,7 +534,8 @@ getpmaphandle(nconf, hostname, tgtaddr)
> > * rpcbind. Returns NULL on error and free's everything.
> > */
> > static CLIENT *
> > -local_rpcb()
> > +local_rpcb(targaddr)
> > + char **targaddr;
> > {
> > CLIENT *client;
> > static struct netconfig *loopnconf;
> > @@ -574,6 +568,8 @@ local_rpcb()
> > if (client != NULL) {
> > /* Mark the socket to be closed in destructor */
> > (void) CLNT_CONTROL(client, CLSET_FD_CLOSE, NULL);
> > + if (targaddr)
> > + *targaddr = strdup(sun.sun_path);
> > return client;
> > }
> >
> > @@ -632,7 +628,7 @@ try_nconf:
> > endnetconfig(nc_handle);
> > }
> > mutex_unlock(&loopnconf_lock);
> > - client = getclnthandle(hostname, loopnconf, NULL);
> > + client = getclnthandle(hostname, loopnconf, targaddr);
> > return (client);
> > }
> >
> > @@ -661,20 +657,11 @@ rpcb_set(program, version, nconf, address)
> > rpc_createerr.cf_stat = RPC_UNKNOWNADDR;
> > return (FALSE);
> > }
> > - client = local_rpcb();
> > + client = local_rpcb(&parms.r_addr);
> > if (! client) {
> > return (FALSE);
> > }
> >
> > - /* convert to universal */
> > - /*LINTED const castaway*/
> > - parms.r_addr = taddr2uaddr((struct netconfig *) nconf,
> > - (struct netbuf *)address);
> > - if (!parms.r_addr) {
> > - CLNT_DESTROY(client);
> > - rpc_createerr.cf_stat = RPC_N2AXLATEFAILURE;
> > - return (FALSE); /* no universal address */
> > - }
> > parms.r_prog = program;
> > parms.r_vers = version;
> > parms.r_netid = nconf->nc_netid;
> > @@ -712,7 +699,7 @@ rpcb_unset(program, version, nconf)
> > RPCB parms;
> > char uidbuf[32];
> >
> > - client = local_rpcb();
> > + client = local_rpcb(NULL);
> > if (! client) {
> > return (FALSE);
> > }
> > @@ -1342,7 +1329,7 @@ rpcb_taddr2uaddr(nconf, taddr)
> > rpc_createerr.cf_stat = RPC_UNKNOWNADDR;
> > return (NULL);
> > }
> > - client = local_rpcb();
> > + client = local_rpcb(NULL);
> > if (! client) {
> > return (NULL);
> > }
> > @@ -1376,7 +1363,7 @@ rpcb_uaddr2taddr(nconf, uaddr)
> > rpc_createerr.cf_stat = RPC_UNKNOWNADDR;
> > return (NULL);
> > }
> > - client = local_rpcb();
> > + client = local_rpcb(NULL);
> > if (! client) {
> > return (NULL);
> > }
> It is not clear why... but this patch stop mountd from
> registering with rpcbind (both the old and changed via
> the latest patches), which means v3 mounts break.
> Not good :-)
>
> Turning debugging on... rpcbind is receiving the set prog
> but not recording it, since port 0 is returned when
> the client tries to do a v3 mount.
>
> Are you guys seeing this??

I am now. Wonder why I didn't before..

The problem is the removal of that /* convert to universal */ section.

r_addr is meant to be based on 'address', not whatever local_rpcb()
finds.

I'll fix that up as well as a bug I found in patch 1 (a '\0' should have
been '@') and test and resend.

Thanks,
NeilBrown


>
> I remove this patch and everything works!
>
> steved.
>
>


2024-03-11 09:46:46

by Petr Vorel

[permalink] [raw]
Subject: Re: [PATCH 2/3] Change local_rpcb() to take a targaddr pointer.



> On 2/25/24 6:40 PM, NeilBrown wrote:
> > Two callers of local_rpcb() want the target-addr, and local_rcpb() has
> > easy access to it. So accept a pointer and fill it in if not NULL.

> > Signed-off-by: NeilBrown <[email protected]>
> > ---
> > src/rpcb_clnt.c | 35 +++++++++++------------------------
> > 1 file changed, 11 insertions(+), 24 deletions(-)

> > diff --git a/src/rpcb_clnt.c b/src/rpcb_clnt.c
> > index 68fe69a320ff..f587580228ab 100644
> > --- a/src/rpcb_clnt.c
> > +++ b/src/rpcb_clnt.c
> > @@ -89,7 +89,7 @@ static struct address_cache *copy_of_cached(const char *, char *);
> > static void delete_cache(struct netbuf *);
> > static void add_cache(const char *, const char *, struct netbuf *, char *);
> > static CLIENT *getclnthandle(const char *, const struct netconfig *, char **);
> > -static CLIENT *local_rpcb(void);
> > +static CLIENT *local_rpcb(char **targaddr);
> > #ifdef NOTUSED
> > static struct netbuf *got_entry(rpcb_entry_list_ptr, const struct netconfig *);
> > #endif
> > @@ -430,19 +430,12 @@ getclnthandle(host, nconf, targaddr)
> > nconf->nc_netid, si.si_af, si.si_proto, si.si_socktype));
> > if (nconf->nc_protofmly != NULL && strcmp(nconf->nc_protofmly, NC_LOOPBACK) == 0) {
> > - client = local_rpcb();
> > + client = local_rpcb(targaddr);
> > if (! client) {
> > LIBTIRPC_DEBUG(1, ("getclnthandle: %s",
> > clnt_spcreateerror("local_rpcb failed")));
> > goto out_err;
> > } else {
> > - struct sockaddr_un sun;
> > -
> > - if (targaddr) {
> > - *targaddr = malloc(sizeof(sun.sun_path));
> > - strncpy(*targaddr, _PATH_RPCBINDSOCK,
> > - sizeof(sun.sun_path));
> > - }
> > return (client);
> > }
> > } else {
> > @@ -541,7 +534,8 @@ getpmaphandle(nconf, hostname, tgtaddr)
> > * rpcbind. Returns NULL on error and free's everything.
> > */
> > static CLIENT *
> > -local_rpcb()
> > +local_rpcb(targaddr)
> > + char **targaddr;
> > {
> > CLIENT *client;
> > static struct netconfig *loopnconf;
> > @@ -574,6 +568,8 @@ local_rpcb()
> > if (client != NULL) {
> > /* Mark the socket to be closed in destructor */
> > (void) CLNT_CONTROL(client, CLSET_FD_CLOSE, NULL);
> > + if (targaddr)
> > + *targaddr = strdup(sun.sun_path);
> > return client;
> > }
> > @@ -632,7 +628,7 @@ try_nconf:
> > endnetconfig(nc_handle);
> > }
> > mutex_unlock(&loopnconf_lock);
> > - client = getclnthandle(hostname, loopnconf, NULL);
> > + client = getclnthandle(hostname, loopnconf, targaddr);
> > return (client);
> > }
> > @@ -661,20 +657,11 @@ rpcb_set(program, version, nconf, address)
> > rpc_createerr.cf_stat = RPC_UNKNOWNADDR;
> > return (FALSE);
> > }
> > - client = local_rpcb();
> > + client = local_rpcb(&parms.r_addr);
> > if (! client) {
> > return (FALSE);
> > }
> > - /* convert to universal */
> > - /*LINTED const castaway*/
> > - parms.r_addr = taddr2uaddr((struct netconfig *) nconf,
> > - (struct netbuf *)address);
> > - if (!parms.r_addr) {
> > - CLNT_DESTROY(client);
> > - rpc_createerr.cf_stat = RPC_N2AXLATEFAILURE;
> > - return (FALSE); /* no universal address */
> > - }
> > parms.r_prog = program;
> > parms.r_vers = version;
> > parms.r_netid = nconf->nc_netid;
> > @@ -712,7 +699,7 @@ rpcb_unset(program, version, nconf)
> > RPCB parms;
> > char uidbuf[32];
> > - client = local_rpcb();
> > + client = local_rpcb(NULL);
> > if (! client) {
> > return (FALSE);
> > }
> > @@ -1342,7 +1329,7 @@ rpcb_taddr2uaddr(nconf, taddr)
> > rpc_createerr.cf_stat = RPC_UNKNOWNADDR;
> > return (NULL);
> > }
> > - client = local_rpcb();
> > + client = local_rpcb(NULL);
> > if (! client) {
> > return (NULL);
> > }
> > @@ -1376,7 +1363,7 @@ rpcb_uaddr2taddr(nconf, uaddr)
> > rpc_createerr.cf_stat = RPC_UNKNOWNADDR;
> > return (NULL);
> > }
> > - client = local_rpcb();
> > + client = local_rpcb(NULL);
> > if (! client) {
> > return (NULL);
> > }
> It is not clear why... but this patch stop mountd from
> registering with rpcbind (both the old and changed via
> the latest patches), which means v3 mounts break.
> Not good :-)

> Turning debugging on... rpcbind is receiving the set prog
> but not recording it, since port 0 is returned when
> the client tries to do a v3 mount.

> Are you guys seeing this??

Hi Steve,

I'm sorry I haven't had time to test. Strange, I don't see any bug in that
commit.

Kind regards,
Petr

> I remove this patch and everything works!

> steved.