Hi-
On Apr 23, 2012, at 11:24 AM, Niels de Vos wrote:
> There is a check for NC_TPI_CLTS connections which correctly binds only
> to the hostnames/addresses given on the commandline (-h option). This
> implementation does not take non NC_TPI_CLTS like NC_TPI_COTS and
> NC_TPI_COTS_ORD into account. This causes non NC_TPI_CLTS protocols to
> listen on the "0.0.0.0" and "::" wildcard addresses.
>
> This patch simplifies the code, and moves the check for NC_TPI_CLTS into
> the loop when a bind() for each requested address is done. All
> connections, including NC_TPI_COTS and NC_TPI_COTS_ORD can now benefit
> from resticted bind()'ing on addresses.
Please check the mailing list archives for linux-nfs and/or libtirpc-devel. There may be a good reason for this asymmetrical behavior, or at least some enlightening discussion of this idea.
Also, can you tell us why you need this change in the patch description?
> Signed-off-by: Niels de Vos <ndevos@xxxxxxxxxx>
> ---
> src/rpcbind.c | 230 ++++++++++++++++++++------------------------------------
> 1 files changed, 82 insertions(+), 148 deletions(-)
>
> diff --git a/src/rpcbind.c b/src/rpcbind.c
> index 9a0504d..3f6c2a9 100644
> --- a/src/rpcbind.c
> +++ b/src/rpcbind.c
> @@ -339,172 +339,103 @@ init_transport(struct netconfig *nconf)
> hints.ai_socktype = si.si_socktype;
> hints.ai_protocol = si.si_proto;
> }
> - if (nconf->nc_semantics == NC_TPI_CLTS) {
> +
> + /*
> + * If no hosts were specified, just bind to INADDR_ANY. Otherwise
> + * make sure 127.0.0.1 is added to the list.
> + */
> + nhostsbak = nhosts;
> + nhostsbak++;
> + hosts = realloc(hosts, nhostsbak * sizeof(char *));
> + if (nhostsbak == 1)
> + hosts[0] = "*";
> + else {
> + if (hints.ai_family == AF_INET) {
> + hosts[nhostsbak - 1] = "127.0.0.1";
> + } else if (hints.ai_family == AF_INET6) {
> + hosts[nhostsbak - 1] = "::1";
> + } else
> + return 1;
> + }
> +
> + /*
> + * Bind to specific IPs if asked to
> + */
> + checkbind = 0;
> + while (nhostsbak > 0) {
> + --nhostsbak;
> /*
> - * If no hosts were specified, just bind to INADDR_ANY. Otherwise
> - * make sure 127.0.0.1 is added to the list.
> + * XXX - using RPC library internal functions.
> */
> - nhostsbak = nhosts;
> - nhostsbak++;
> - hosts = realloc(hosts, nhostsbak * sizeof(char *));
> - if (nhostsbak == 1)
> - hosts[0] = "*";
> - else {
> - if (hints.ai_family == AF_INET) {
> - hosts[nhostsbak - 1] = "127.0.0.1";
> - } else if (hints.ai_family == AF_INET6) {
> - hosts[nhostsbak - 1] = "::1";
> - } else
> - return 1;
> + if ((fd = __rpc_nconf2fd(nconf)) < 0) {
> + syslog(LOG_ERR, "cannot create socket for %s",
> + nconf->nc_netid);
> + return (1);
> }
> -
> - /*
> - * Bind to specific IPs if asked to
> - */
> - checkbind = 0;
> - while (nhostsbak > 0) {
> - --nhostsbak;
> - /*
> - * XXX - using RPC library internal functions.
> - */
> - if ((fd = __rpc_nconf2fd(nconf)) < 0) {
> - syslog(LOG_ERR, "cannot create socket for %s",
> - nconf->nc_netid);
> - return (1);
> + switch (hints.ai_family) {
> + case AF_INET:
> + if (inet_pton(AF_INET, hosts[nhostsbak],
> + host_addr) == 1) {
> + hints.ai_flags &= AI_NUMERICHOST;
> + } else {
> + /*
> + * Skip if we have an AF_INET6 adress.
> + */
> + if (inet_pton(AF_INET6,
> + hosts[nhostsbak], host_addr) == 1)
> + continue;
> }
> - switch (hints.ai_family) {
> - case AF_INET:
> + break;
> + case AF_INET6:
> + if (inet_pton(AF_INET6, hosts[nhostsbak],
> + host_addr) == 1) {
> + hints.ai_flags &= AI_NUMERICHOST;
> + } else {
> + /*
> + * Skip if we have an AF_INET adress.
> + */
> if (inet_pton(AF_INET, hosts[nhostsbak],
> - host_addr) == 1) {
> - hints.ai_flags &= AI_NUMERICHOST;
> - } else {
> - /*
> - * Skip if we have an AF_INET6 adress.
> - */
> - if (inet_pton(AF_INET6,
> - hosts[nhostsbak], host_addr) == 1)
> - continue;
> - }
> - break;
> - case AF_INET6:
> - if (inet_pton(AF_INET6, hosts[nhostsbak],
> - host_addr) == 1) {
> - hints.ai_flags &= AI_NUMERICHOST;
> - } else {
> - /*
> - * Skip if we have an AF_INET adress.
> - */
> - if (inet_pton(AF_INET, hosts[nhostsbak],
> - host_addr) == 1)
> - continue;
> - }
> - break;
> - default:
> - break;
> + host_addr) == 1)
> + continue;
> }
> + break;
> + default:
> + break;
> + }
>
> - /*
> - * If no hosts were specified, just bind to INADDR_ANY
> - */
> - if (strcmp("*", hosts[nhostsbak]) == 0)
> - hosts[nhostsbak] = NULL;
> + /*
> + * If no hosts were specified, just bind to INADDR_ANY
> + */
> + if (strcmp("*", hosts[nhostsbak]) == 0)
> + hosts[nhostsbak] = NULL;
>
> + if ((strcmp(nconf->nc_netid, "local") != 0) &&
> + (strcmp(nconf->nc_netid, "unix") != 0)) {
> if ((aicode = getaddrinfo(hosts[nhostsbak],
> - servname, &hints, &res)) != 0) {
> + servname, &hints, &res))!= 0) {
> if ((aicode = getaddrinfo(hosts[nhostsbak],
> - "portmapper", &hints, &res)) != 0) {
> - syslog(LOG_ERR,
> + "portmapper", &hints, &res))!= 0) {
> + syslog(LOG_ERR,
> "cannot get local address for %s: %s",
> nconf->nc_netid, gai_strerror(aicode));
> - continue;
> + continue;
> }
> }
> addrlen = res->ai_addrlen;
> sa = (struct sockaddr *)res->ai_addr;
> - oldmask = umask(S_IXUSR|S_IXGRP|S_IXOTH);
> - if (bind(fd, sa, addrlen) != 0) {
> - syslog(LOG_ERR, "cannot bind %s on %s: %m",
> - (hosts[nhostsbak] == NULL) ? "*" :
> - hosts[nhostsbak], nconf->nc_netid);
> - if (res != NULL)
> - freeaddrinfo(res);
> - continue;
> - } else
> - checkbind++;
> - (void) umask(oldmask);
> -
> - /* Copy the address */
> - taddr.addr.maxlen = taddr.addr.len = addrlen;
> - taddr.addr.buf = malloc(addrlen);
> - if (taddr.addr.buf == NULL) {
> - syslog(LOG_ERR,
> - "cannot allocate memory for %s address",
> - nconf->nc_netid);
> + }
> + oldmask = umask(S_IXUSR|S_IXGRP|S_IXOTH);
> + if (nconf->nc_semantics != NC_TPI_CLTS) {
> + /* NC_TPI_COTS and NC_TPI_COTS_ORD etc */
> + __rpc_fd2sockinfo(fd, &si);
> + if (setsockopt(fd, SOL_SOCKET, SO_REUSEADDR, &on,
> + sizeof(on)) != 0) {
> + syslog(LOG_ERR, "cannot set SO_REUSEADDR on %s",
> + nconf->nc_netid);
> if (res != NULL)
> freeaddrinfo(res);
> return 1;
> }
> - memcpy(taddr.addr.buf, sa, addrlen);
> -#ifdef RPCBIND_DEBUG
> - if (debugging) {
> - /*
> - * for debugging print out our universal
> - * address
> - */
> - char *uaddr;
> - struct netbuf nb;
> - int sa_size = 0;
> -
> - nb.buf = sa;
> - switch( sa->sa_family){
> - case AF_INET:
> - sa_size = sizeof (struct sockaddr_in);
> - break;
> - case AF_INET6:
> - sa_size = sizeof (struct sockaddr_in6);
> - break;
> - }
> - nb.len = nb.maxlen = sa_size;
> - uaddr = taddr2uaddr(nconf, &nb);
> - (void) fprintf(stderr,
> - "rpcbind : my address is %s\n", uaddr);
> - (void) free(uaddr);
> - }
> -#endif
> - my_xprt = (SVCXPRT *)svc_tli_create(fd, nconf, &taddr,
> - RPC_MAXDATASIZE, RPC_MAXDATASIZE);
> - if (my_xprt == (SVCXPRT *)NULL) {
> - syslog(LOG_ERR, "%s: could not create service",
> - nconf->nc_netid);
> - goto error;
> - }
> - }
> - if (!checkbind)
> - return 1;
> - } else { /* NC_TPI_COTS */
> - if ((strcmp(nconf->nc_netid, "local") != 0) &&
> - (strcmp(nconf->nc_netid, "unix") != 0)) {
> - if ((aicode = getaddrinfo(NULL, servname, &hints, &res))!= 0) {
> - if ((aicode = getaddrinfo(NULL, "portmapper", &hints, &res))!= 0) {
> - printf("cannot get local address for %s: %s", nconf->nc_netid, gai_strerror(aicode));
> - syslog(LOG_ERR,
> - "cannot get local address for %s: %s",
> - nconf->nc_netid, gai_strerror(aicode));
> - return 1;
> - }
> - }
> - addrlen = res->ai_addrlen;
> - sa = (struct sockaddr *)res->ai_addr;
> - }
> - oldmask = umask(S_IXUSR|S_IXGRP|S_IXOTH);
> - __rpc_fd2sockinfo(fd, &si);
> - if (setsockopt(fd, SOL_SOCKET, SO_REUSEADDR, &on,
> - sizeof(on)) != 0) {
> - syslog(LOG_ERR, "cannot set SO_REUSEADDR on %s",
> - nconf->nc_netid);
> - if (res != NULL)
> - freeaddrinfo(res);
> - return 1;
> }
> if (bind(fd, sa, addrlen) < 0) {
> syslog(LOG_ERR, "cannot bind %s: %m", nconf->nc_netid);
> @@ -530,7 +461,7 @@ init_transport(struct netconfig *nconf)
> /* for debugging print out our universal address */
> char *uaddr;
> struct netbuf nb;
> - int sa_size2 = 0;
> + int sa_size2 = 0;
>
> nb.buf = sa;
> switch( sa->sa_family){
> @@ -551,13 +482,16 @@ init_transport(struct netconfig *nconf)
>
> listen(fd, SOMAXCONN);
>
> - my_xprt = (SVCXPRT *)svc_tli_create(fd, nconf, &taddr, RPC_MAXDATASIZE, RPC_MAXDATASIZE);
> + my_xprt = (SVCXPRT *)svc_tli_create(fd, nconf, &taddr,
> + RPC_MAXDATASIZE, RPC_MAXDATASIZE);
> if (my_xprt == (SVCXPRT *)NULL) {
> syslog(LOG_ERR, "%s: could not create service",
> nconf->nc_netid);
> goto error;
> }
> }
> + if (!checkbind)
> + return 1;
>
> #ifdef PORTMAP
> /*
> --
> 1.7.7.6
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
Chuck Lever
chuck[dot]lever[at]oracle[dot]com
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
