| Message ID | 20180410213043.5545-2-steved@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
> On Apr 10, 2018, at 3:30 PM, Steve Dickson <steved@redhat.com> wrote: > > Commit 46e04a73 changed svc_com_create() to avoid using > reserve ports but removed the listen() call which > could break, undocumented expectations, of legacy > server apps. > > Reserve ports are not needed for listening ports, so > the new __binddynport() is used to avoid squatting > on ports that are assigned to other network services, > but it is important to say backwards compatible > with the legacy glibc code so the the listen() > call is added back. svc_tli_create should already perform a call to __binddynport() and to listen(2). That's why those calls were removed from svc_com_create(). 222 if (madefd || !__rpc_sockisbound(fd)) { 223 if (bindaddr == NULL) { 224 if (__binddynport(fd) == -1) { 225 warnx( 226 "svc_tli_create: could not bind to anonymous port"); 227 goto freedata; 228 } 229 listen(fd, SOMAXCONN); 230 } else { 231 if (bind(fd, 232 (struct sockaddr *)bindaddr->addr.buf, 233 (socklen_t)si.si_alen) < 0) { 234 warnx( 235 "svc_tli_create: could not bind to requested address"); 236 goto freedata; 237 } 238 listen(fd, (int)bindaddr->qlen); 239 } 240 241 } I expect that __rpc_sockisbound(fd) should be false if svc_tli_create() is called from svc_com_create(). Can you tell why it isn't? > Signed-off-by: Steve Dickson <steved@redhat.com> > --- > src/rpc_soc.c | 7 +++++++ > 1 file changed, 7 insertions(+) > > diff --git a/src/rpc_soc.c b/src/rpc_soc.c > index f32a27c..cf15216 100644 > --- a/src/rpc_soc.c > +++ b/src/rpc_soc.c > @@ -331,6 +331,13 @@ svc_com_create(fd, sendsize, recvsize, netid) > madefd = TRUE; > } > > + if (__binddynport(fd) == -1) { > + if (madefd) > + (void)close(fd); > + (void) syslog(LOG_ERR, > + "svc%s_create: could not bind connection: %m", netid); > + } > + listen(fd, SOMAXCONN); > svc = svc_tli_create(fd, nconf, NULL, sendsize, recvsize); > (void) freenetconfigent(nconf); > if (svc == NULL) { > -- > 2.14.3 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-nfs" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- Chuck Lever -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hey, On 04/10/2018 06:29 PM, Chuck Lever wrote: > > >> On Apr 10, 2018, at 3:30 PM, Steve Dickson <steved@redhat.com> wrote: >> >> Commit 46e04a73 changed svc_com_create() to avoid using >> reserve ports but removed the listen() call which >> could break, undocumented expectations, of legacy >> server apps. >> >> Reserve ports are not needed for listening ports, so >> the new __binddynport() is used to avoid squatting >> on ports that are assigned to other network services, >> but it is important to say backwards compatible >> with the legacy glibc code so the the listen() >> call is added back. > > svc_tli_create should already perform a call to __binddynport() > and to listen(2). That's why those calls were removed from > svc_com_create(). > Fair enough... I did miss that call to svc_tli_create(). It appears the callers of svc_com_create() are no longer used at least in nfs-utils. It appears to be the old SUN RPC API and we using the TIRPC API. steved. -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/src/rpc_soc.c b/src/rpc_soc.c index f32a27c..cf15216 100644 --- a/src/rpc_soc.c +++ b/src/rpc_soc.c @@ -331,6 +331,13 @@ svc_com_create(fd, sendsize, recvsize, netid) madefd = TRUE; } + if (__binddynport(fd) == -1) { + if (madefd) + (void)close(fd); + (void) syslog(LOG_ERR, + "svc%s_create: could not bind connection: %m", netid); + } + listen(fd, SOMAXCONN); svc = svc_tli_create(fd, nconf, NULL, sendsize, recvsize); (void) freenetconfigent(nconf); if (svc == NULL) {
Commit 46e04a73 changed svc_com_create() to avoid using reserve ports but removed the listen() call which could break, undocumented expectations, of legacy server apps. Reserve ports are not needed for listening ports, so the new __binddynport() is used to avoid squatting on ports that are assigned to other network services, but it is important to say backwards compatible with the legacy glibc code so the the listen() call is added back. Signed-off-by: Steve Dickson <steved@redhat.com> --- src/rpc_soc.c | 7 +++++++ 1 file changed, 7 insertions(+)