sunrpc: Turn bindresvport into a compatibility symbol
Commit Message
(This is a bit of a controversial change, but I'm not sure what our
alternatives are, given the conflict with libtirpc.)
bindresvport is mostly used along with Sun RPC functions. libtirpc
has its own implementation of it, and this implementation is
is used when linking against libtirpc (under a separate symbol
version). The libtirpc implementation is superior because it
provides a filter for well-known ports, addressing historic bug 456.
Exceptions that use bindresvport without linking against libtirpc
are torque (the resource manager) and the sign command in OBS
(the build service).
This change turns bindresvport into a compatibility symbol because
it is currently declared in a non-RPC header (<netinet/in.h>),
and remvoes it from the public header. In theory, it would be
possible to make the declaration conditional on
--enable-obsolete-rpc, but that would need quite a bit of shuffling
for a declaration that would be removed soon anyway.
Despite the declaration of bindresvport6 in a public header file,
no implemention was provided by glibc, so this commit removes this
declaration without further changes.
---
NEWS | 3 +++
include/netinet/in.h | 4 +++-
inet/netinet/in.h | 10 ----------
sunrpc/bindrsvprt.c | 9 +++++++--
sunrpc/clnt_tcp.c | 2 +-
sunrpc/clnt_udp.c | 2 +-
sunrpc/svc_tcp.c | 2 +-
sunrpc/svc_udp.c | 2 +-
8 files changed, 17 insertions(+), 17 deletions(-)
Comments
On Tue, Jul 7, 2020 at 9:13 AM Florian Weimer via Libc-alpha
<libc-alpha@sourceware.org> wrote:
>
> (This is a bit of a controversial change, but I'm not sure what our
> alternatives are, given the conflict with libtirpc.)
>
> bindresvport is mostly used along with Sun RPC functions. libtirpc
> has its own implementation of it, and this implementation is
> is used when linking against libtirpc (under a separate symbol
> version). The libtirpc implementation is superior because it
> provides a filter for well-known ports, addressing historic bug 456.
>
> Exceptions that use bindresvport without linking against libtirpc
> are torque (the resource manager) and the sign command in OBS
> (the build service).
Have they been fixed? Otherwise, they won't compile/link with glibc
2.32.
> This change turns bindresvport into a compatibility symbol because
> it is currently declared in a non-RPC header (<netinet/in.h>),
> and remvoes it from the public header. In theory, it would be
> possible to make the declaration conditional on
> --enable-obsolete-rpc, but that would need quite a bit of shuffling
> for a declaration that would be removed soon anyway.
>
> Despite the declaration of bindresvport6 in a public header file,
> no implemention was provided by glibc, so this commit removes this
> declaration without further changes.
I think that bindresvport6 can be removed with a separate patch.
* H. J. Lu:
> On Tue, Jul 7, 2020 at 9:13 AM Florian Weimer via Libc-alpha
> <libc-alpha@sourceware.org> wrote:
>>
>> (This is a bit of a controversial change, but I'm not sure what our
>> alternatives are, given the conflict with libtirpc.)
>>
>> bindresvport is mostly used along with Sun RPC functions. libtirpc
>> has its own implementation of it, and this implementation is
>> is used when linking against libtirpc (under a separate symbol
>> version). The libtirpc implementation is superior because it
>> provides a filter for well-known ports, addressing historic bug 456.
>>
>> Exceptions that use bindresvport without linking against libtirpc
>> are torque (the resource manager) and the sign command in OBS
>> (the build service).
>
> Have they been fixed? Otherwise, they won't compile/link with glibc
> 2.32.
I suppose I can try to submit patches for them if this indeed the
direction we want to move in.
The symbol clash with libtirpc is rather unfortunate.
>> This change turns bindresvport into a compatibility symbol because
>> it is currently declared in a non-RPC header (<netinet/in.h>),
>> and remvoes it from the public header. In theory, it would be
>> possible to make the declaration conditional on
>> --enable-obsolete-rpc, but that would need quite a bit of shuffling
>> for a declaration that would be removed soon anyway.
>>
>> Despite the declaration of bindresvport6 in a public header file,
>> no implemention was provided by glibc, so this commit removes this
>> declaration without further changes.
>
> I think that bindresvport6 can be removed with a separate patch.
Sure, if we don't want to follow this approach.
Thanks,
Florian
On Tue, Jul 7, 2020 at 10:04 AM Florian Weimer <fweimer@redhat.com> wrote:
>
> * H. J. Lu:
>
> > On Tue, Jul 7, 2020 at 9:13 AM Florian Weimer via Libc-alpha
> > <libc-alpha@sourceware.org> wrote:
> >>
> >> (This is a bit of a controversial change, but I'm not sure what our
> >> alternatives are, given the conflict with libtirpc.)
> >>
> >> bindresvport is mostly used along with Sun RPC functions. libtirpc
> >> has its own implementation of it, and this implementation is
> >> is used when linking against libtirpc (under a separate symbol
> >> version). The libtirpc implementation is superior because it
> >> provides a filter for well-known ports, addressing historic bug 456.
> >>
> >> Exceptions that use bindresvport without linking against libtirpc
> >> are torque (the resource manager) and the sign command in OBS
> >> (the build service).
> >
> > Have they been fixed? Otherwise, they won't compile/link with glibc
> > 2.32.
>
> I suppose I can try to submit patches for them if this indeed the
> direction we want to move in.
I think it is a good thing when using with glibc, regardless of what
glibc does with
bindresvport. We need to provide a solution before it is removed from
glibc API.
> The symbol clash with libtirpc is rather unfortunate.
True.
> >> This change turns bindresvport into a compatibility symbol because
> >> it is currently declared in a non-RPC header (<netinet/in.h>),
> >> and remvoes it from the public header. In theory, it would be
> >> possible to make the declaration conditional on
> >> --enable-obsolete-rpc, but that would need quite a bit of shuffling
> >> for a declaration that would be removed soon anyway.
> >>
> >> Despite the declaration of bindresvport6 in a public header file,
> >> no implemention was provided by glibc, so this commit removes this
> >> declaration without further changes.
> >
> > I think that bindresvport6 can be removed with a separate patch.
>
> Sure, if we don't want to follow this approach.
>
I think 2 things are orthogonal.
* H. J. Lu via Libc-alpha:
> On Tue, Jul 7, 2020 at 10:04 AM Florian Weimer <fweimer@redhat.com> wrote:
>>
>> * H. J. Lu:
>>
>> > On Tue, Jul 7, 2020 at 9:13 AM Florian Weimer via Libc-alpha
>> > <libc-alpha@sourceware.org> wrote:
>> >>
>> >> (This is a bit of a controversial change, but I'm not sure what our
>> >> alternatives are, given the conflict with libtirpc.)
>> >>
>> >> bindresvport is mostly used along with Sun RPC functions. libtirpc
>> >> has its own implementation of it, and this implementation is
>> >> is used when linking against libtirpc (under a separate symbol
>> >> version). The libtirpc implementation is superior because it
>> >> provides a filter for well-known ports, addressing historic bug 456.
>> >>
>> >> Exceptions that use bindresvport without linking against libtirpc
>> >> are torque (the resource manager) and the sign command in OBS
>> >> (the build service).
>> >
>> > Have they been fixed? Otherwise, they won't compile/link with glibc
>> > 2.32.
>>
>> I suppose I can try to submit patches for them if this indeed the
>> direction we want to move in.
>
> I think it is a good thing when using with glibc, regardless of what
> glibc does with
> bindresvport. We need to provide a solution before it is removed from
> glibc API.
I've posted a patch that disentangles bindresvport from sunrpc, so that
we can revisit this issue at a later point:
<https://sourceware.org/pipermail/libc-alpha/2020-July/115979.html>
Thanks,
Florian
@@ -67,6 +67,9 @@ Deprecated and removed features, and other changes affecting compatibility:
* ldconfig now defaults to the new format for ld.so.cache. glibc has
already supported this format for almost 20 years.
+* The bindresvport function is no longer supported for new programs.
+ Applications that need this functions should link against libtirpc.
+
Changes to build and runtime requirements:
* powerpc64le requires GCC 7.4 or newer. This is required for supporting
@@ -3,7 +3,9 @@
#include <inet/netinet/in.h>
#ifndef _ISOMAC
-libc_hidden_proto (bindresvport)
+int __bindresvport (int __sockfd, struct sockaddr_in *__sock_in) __THROW;
+libc_hidden_proto (__bindresvport)
+
libc_hidden_proto (in6addr_loopback)
extern __typeof (in6addr_loopback) __in6addr_loopback;
libc_hidden_proto (__in6addr_loopback)
@@ -502,16 +502,6 @@ extern uint16_t htons (uint16_t __hostshort)
#define IN6_IS_ADDR_MULTICAST(a) (((const uint8_t *) (a))[0] == 0xff)
-#ifdef __USE_MISC
-/* Bind socket to a privileged IP port. */
-extern int bindresvport (int __sockfd, struct sockaddr_in *__sock_in) __THROW;
-
-/* The IPv6 version of this function. */
-extern int bindresvport6 (int __sockfd, struct sockaddr_in6 *__sock_in)
- __THROW;
-#endif
-
-
#define IN6_IS_ADDR_MC_NODELOCAL(a) \
(IN6_IS_ADDR_MULTICAST(a) \
&& ((((const uint8_t *) (a))[1] & 0xf) == 0x1))
@@ -36,6 +36,7 @@
#include <sys/socket.h>
#include <netinet/in.h>
#include <libc-lock.h>
+#include <shlib-compat.h>
/*
* Locks the static variables in this file.
@@ -46,7 +47,7 @@ __libc_lock_define_initialized (static, lock);
* Bind a socket to a privileged IP port
*/
int
-bindresvport (int sd, struct sockaddr_in *sin)
+__bindresvport (int sd, struct sockaddr_in *sin)
{
static short port;
struct sockaddr_in myaddr;
@@ -107,4 +108,8 @@ bindresvport (int sd, struct sockaddr_in *sin)
return res;
}
-libc_hidden_def (bindresvport)
+libc_hidden_def (__bindresvport)
+
+#if SHLIB_COMPAT (libc, GLIBC_2_0, GLIBC_2_32)
+compat_symbol (libc, __bindresvport, bindresvport, GLIBC_2_0);
+#endif
@@ -148,7 +148,7 @@ clnttcp_create (struct sockaddr_in *raddr, u_long prog, u_long vers,
if (*sockp < 0)
{
*sockp = __socket (AF_INET, SOCK_STREAM, IPPROTO_TCP);
- (void) bindresvport (*sockp, (struct sockaddr_in *) 0);
+ (void) __bindresvport (*sockp, (struct sockaddr_in *) 0);
if ((*sockp < 0)
|| (__connect (*sockp, (struct sockaddr *) raddr,
sizeof (*raddr)) < 0))
@@ -185,7 +185,7 @@ __libc_clntudp_bufcreate (struct sockaddr_in *raddr, u_long program,
goto fooy;
}
/* attempt to bind to prov port */
- (void) bindresvport (*sockp, (struct sockaddr_in *) 0);
+ (void) __bindresvport (*sockp, (struct sockaddr_in *) 0);
#ifdef IP_RECVERR
{
int on = 1;
@@ -169,7 +169,7 @@ svctcp_create (int sock, u_int sendsize, u_int recvsize)
}
memset ((char *) &addr, 0, sizeof (addr));
addr.sin_family = AF_INET;
- if (bindresvport (sock, &addr))
+ if (__bindresvport (sock, &addr))
{
addr.sin_port = 0;
(void) __bind (sock, (struct sockaddr *) &addr, len);
@@ -140,7 +140,7 @@ svcudp_bufcreate (int sock, u_int sendsz, u_int recvsz)
}
memset ((char *) &addr, 0, sizeof (addr));
addr.sin_family = AF_INET;
- if (bindresvport (sock, &addr))
+ if (__bindresvport (sock, &addr))
{
addr.sin_port = 0;
(void) __bind (sock, (struct sockaddr *) &addr, len);