From: Alan T. DeKok Date: Sat, 2 Aug 2025 10:48:31 +0000 (-0400) Subject: remove ACCEPTED state. X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=fb4d3ae313bc92cf5c6a2d8df2a5c02943d59eb2;p=thirdparty%2Ffreeradius-server.git remove ACCEPTED state. and more cleanups for accept. We can't re-open an accepted socket --- diff --git a/src/lib/bio/fd.c b/src/lib/bio/fd.c index c7f6b4fd8a..9afc424440 100644 --- a/src/lib/bio/fd.c +++ b/src/lib/bio/fd.c @@ -729,8 +729,6 @@ static ssize_t fr_bio_fd_try_connect(fr_bio_fd_t *my) retry: if (connect(my->info.socket.fd, (struct sockaddr *) &my->remote_sockaddr, my->remote_sockaddr_len) == 0) { - fr_bio_fd_set_open(my); - /* * The source IP may have changed, so get the new one. */ @@ -738,6 +736,7 @@ retry: if (fr_bio_fd_init_common(my) < 0) goto fail; + fr_bio_fd_set_open(my); return 0; } @@ -917,12 +916,17 @@ int fr_bio_fd_init_common(fr_bio_fd_t *my) int fr_bio_fd_init_listen(fr_bio_fd_t *my) { + fr_assert(my->info.socket.type == SOCK_STREAM); + my->bio.read = fr_bio_null_read; my->bio.write = fr_bio_null_write; + /* + * @todo - make the backlog configurable. + */ if (listen(my->info.socket.fd, 8) < 0) { - fr_strerror_printf("Failed opening setting FD_CLOEXE: %s", fr_syserror(errno)); - return -1; + fr_strerror_printf("Failed calling listen() %s", fr_syserror(errno)); + return fr_bio_error(IO); } fr_bio_fd_set_open(my); @@ -1044,7 +1048,7 @@ retry: case EIO: tries++; if (tries < my->max_tries) goto retry; - return -1; + return fr_bio_error(IO); default: /* @@ -1345,7 +1349,6 @@ int fr_bio_fd_write_only(fr_bio_t *bio) goto set_recv_buff_zero; case FR_BIO_FD_CONNECTED: - case FR_BIO_FD_ACCEPTED: /* * Further reads are disallowed. However, this likely has no effect for UDP sockets. */ @@ -1386,104 +1389,144 @@ int fr_bio_fd_write_only(fr_bio_t *bio) return 0; } -/** Alternative to calling fr_bio_read() on new socket. +#ifndef __linux__ +static int inline accept4(int fd, struct sockaddr *sockaddr, socklen_t *salen, UNUSED int flags) +{ + fd = accept(fd, sockaddr, salen); + if (fd >= 0) { +#ifdef FD_CLOEXEC + int rcode; + + rcode = fcntl(fd, F_GETFD); + if (rcode >= 0) { + if (fcntl(fd, F_SETFD, rcode | FD_CLOEXEC) < 0) { + close(fd); + return -1; + } + } +#endif + + if (fr_nonblock(fd) < 0) { + close(fd); + } + } + + return fd; +} +#ifndef SOCK_NONBLOCK +#define SOCK_NONBLOCK 0 +#endif + +#ifndef SOCK_CLOEXEC +#define SOCK_CLOEXEC 0 +#endif + +#endif + +/** Accept a stream socket and initialize its flags. * */ -int fr_bio_fd_accept(TALLOC_CTX *ctx, fr_bio_t **out_p, fr_bio_t *bio) +static int fr_bio_fd_accept_stream(fr_bio_fd_t *my, fr_bio_fd_t const *parent) { - int fd, tries = 0; int rcode; - fr_bio_fd_t *my = talloc_get_type_abort(bio, fr_bio_fd_t); - socklen_t salen; - struct sockaddr_storage sockaddr; - fr_bio_fd_t *out; - fr_bio_fd_config_t *cfg; - - salen = sizeof(sockaddr); - *out_p = NULL; + int fd, tries = 0; - fr_assert(my->info.type == FR_BIO_FD_LISTEN); - fr_assert(my->info.socket.type == SOCK_STREAM); + my->remote_sockaddr_len = sizeof(my->remote_sockaddr); retry: -#ifdef __linux__ /* * Set these flags immediately on the new socket. */ - fd = accept4(my->info.socket.fd, (struct sockaddr *) &sockaddr, &salen, SOCK_NONBLOCK | SOCK_CLOEXEC); -#else - fd = accept(my->info.socket.fd, (struct sockaddr *) &sockaddr, &salen); -#endif + fd = accept4(parent->info.socket.fd, (struct sockaddr *) &my->remote_sockaddr, &my->remote_sockaddr_len, + SOCK_NONBLOCK | SOCK_CLOEXEC); if (fd < 0) { - switch (errno) { - case EINTR: - /* - * Try a few times before giving up. - */ + /* + * Try a few times before giving up. + */ + if (errno == EINTR) { tries++; if (tries <= my->max_tries) goto retry; - return 0; - - /* - * We can ignore these errors. - */ - case ECONNABORTED: -#if defined(EWOULDBLOCK) && (EWOULDBLOCK != EAGAIN) - case EWOULDBLOCK: -#endif - case EAGAIN: -#ifdef EPERM - case EPERM: -#endif -#ifdef ETIMEDOUT - case ETIMEDOUT: -#endif - return 0; - - default: - /* - * Some other error, it's fatal. - */ - fr_bio_shutdown(&my->bio); - break; } return fr_bio_error(IO); } /* - * Allocate the base BIO and set it up. + * Get IP addresses from the sockaddr. */ - out = (fr_bio_fd_t *) fr_bio_fd_alloc(ctx, NULL, my->offset); - if (!out) { - close(fd); - return fr_bio_error(GENERIC); + if ((my->info.socket.af == AF_INET) || (my->info.socket.af == AF_INET6)) { + fr_ipaddr_from_sockaddr(&my->info.socket.inet.dst_ipaddr, &my->info.socket.inet.dst_port, + &my->remote_sockaddr, my->remote_sockaddr_len); } /* - * We have a file descriptor. Initialize the configuration with the new information. + * The socket is now open. Save the new state. */ - cfg = talloc_memdup(out, my->info.cfg, sizeof(*my->info.cfg)); - if (!cfg) { - fr_strerror_const("Out of memory"); + my->info.socket.fd = fd; + my->info.type = FR_BIO_FD_CONNECTED; + + rcode = fr_bio_fd_init_common(my); + if (rcode < 0) { close(fd); - talloc_free(out); - return fr_bio_error(GENERIC); + return rcode; + } + + fr_bio_fd_name(my); + if (!my->info.name) { + close(fd); + return fr_bio_error(OOM); } + return 0; +} + + +/** Accept a new connection on a socket. + * + */ +int fr_bio_fd_accept(TALLOC_CTX *ctx, fr_bio_t **out_p, fr_bio_t *bio) +{ + int rcode; + fr_bio_fd_t *my = talloc_get_type_abort(bio, fr_bio_fd_t); + fr_bio_fd_t *out; + + *out_p = NULL; + + fr_assert(my->info.type == FR_BIO_FD_LISTEN); + /* - * Set the type to ACCEPTED, and set up the rest of the callbacks to match. + * Allocate the base BIO and set it up. */ - cfg->type = FR_BIO_FD_ACCEPTED; - out->info.socket.fd = fd; + out = (fr_bio_fd_t *) fr_bio_fd_alloc(ctx, NULL, my->offset); + if (!out) return fr_bio_error(GENERIC); - rcode = fr_bio_fd_open(bio, cfg); + /* + * Initialize the new BIO information. + */ + out->info.cfg = my->info.cfg; + + /* + * Copy all socket fields, including ones which will be over-written later. + */ + out->info.socket = my->info.socket; + + if (my->info.socket.type == SOCK_STREAM) { + rcode = fr_bio_fd_accept_stream(out, my); + + } else { + fr_assert(my->info.socket.type == SOCK_DGRAM); + + // rcode = fr_bio_fd_accept_datagram(out, my); + rcode = -1; + } if (rcode < 0) { talloc_free(out); return rcode; } - fr_assert(out->info.type == FR_BIO_FD_CONNECTED); + /* + * The socket is now + */ *out_p = (fr_bio_t *) out; return 1; diff --git a/src/lib/bio/fd.h b/src/lib/bio/fd.h index 81ed87de92..64d256f6bc 100644 --- a/src/lib/bio/fd.h +++ b/src/lib/bio/fd.h @@ -68,7 +68,6 @@ typedef enum { FR_BIO_FD_CONNECTED, //!< connected client sockets (UDP or TCP) FR_BIO_FD_LISTEN, //!< returns new fd in buffer on fr_bio_read() or fr_bio_fd_accept() // updates #fr_bio_fd_packet_ctx_t on successful FD read. - FR_BIO_FD_ACCEPTED, //!< temporarily until it's connected. } fr_bio_fd_type_t; /** Configuration for sockets diff --git a/src/lib/bio/fd_open.c b/src/lib/bio/fd_open.c index 71c409ecf7..a55c1470ad 100644 --- a/src/lib/bio/fd_open.c +++ b/src/lib/bio/fd_open.c @@ -815,7 +815,10 @@ done: return fr_bio_fd_socket_name(my); } -static void fr_bio_fd_name(fr_bio_fd_t *my) +/** Set the name of an FD BIO + * + */ +void fr_bio_fd_name(fr_bio_fd_t *my) { fr_bio_fd_config_t const *cfg = my->info.cfg; @@ -847,7 +850,6 @@ static void fr_bio_fd_name(fr_bio_fd_t *my) break; case FR_BIO_FD_CONNECTED: - case FR_BIO_FD_ACCEPTED: switch (my->info.socket.af) { case AF_INET: case AF_INET6: @@ -889,8 +891,6 @@ static void fr_bio_fd_name(fr_bio_fd_t *my) break; case FR_BIO_FD_LISTEN: - fr_assert(cfg->socket_type == SOCK_STREAM); - switch (my->info.socket.af) { case AF_INET: case AF_INET6: @@ -977,11 +977,6 @@ int fr_bio_fd_check_config(fr_bio_fd_config_t const *cfg) return -1; } break; - - case FR_BIO_FD_ACCEPTED: - fr_assert(cfg->src_ipaddr.af != AF_UNSPEC); - fr_assert(cfg->dst_ipaddr.af != AF_UNSPEC); - break; } return 0; @@ -999,11 +994,6 @@ int fr_bio_fd_open(fr_bio_t *bio, fr_bio_fd_config_t const *cfg) fr_strerror_clear(); - if (cfg->type == FR_BIO_FD_ACCEPTED) { - fr_strerror_const("Connection is already open"); - return fr_bio_error(GENERIC); - } - my->info = (fr_bio_fd_info_t) { .socket = { .type = cfg->socket_type, @@ -1079,10 +1069,6 @@ int fr_bio_fd_open(fr_bio_t *bio, fr_bio_fd_config_t const *cfg) case FR_BIO_FD_LISTEN: fr_assert(my->info.socket.inet.src_ipaddr.af != AF_UNSPEC); break; - - case FR_BIO_FD_ACCEPTED: - fr_assert(0); - break; } if (cfg->socket_type == SOCK_STREAM) { @@ -1103,7 +1089,7 @@ int fr_bio_fd_open(fr_bio_t *bio, fr_bio_fd_config_t const *cfg) fd = socket(my->info.socket.af, my->info.socket.type, protocol); if (fd < 0) { fr_strerror_printf("Failed opening socket: %s", fr_syserror(errno)); - return -1; + return fr_bio_error(GENERIC); } } else if (cfg->path) { @@ -1114,7 +1100,7 @@ int fr_bio_fd_open(fr_bio_t *bio, fr_bio_fd_config_t const *cfg) fd = socket(my->info.socket.af, my->info.socket.type, 0); if (fd < 0) { fr_strerror_printf("Failed opening domain socket %s: %s", cfg->path, fr_syserror(errno)); - return -1; + return fr_bio_error(GENERIC); } } else { @@ -1165,7 +1151,7 @@ int fr_bio_fd_open(fr_bio_t *bio, fr_bio_fd_config_t const *cfg) } if (fd < 0) { fr_strerror_printf("Failed opening file %s: %s", cfg->filename, fr_syserror(errno)); - return -1; + return fr_bio_error(GENERIC); } } @@ -1173,7 +1159,8 @@ int fr_bio_fd_open(fr_bio_t *bio, fr_bio_fd_config_t const *cfg) * Set it to be non-blocking if required. */ if (cfg->async && (fr_nonblock(fd) < 0)) { - fr_strerror_printf("Failed opening setting O_NONBLOCK: %s", fr_syserror(errno)); + fr_strerror_printf("Failed setting O_NONBLOCK: %s", fr_syserror(errno)); + rcode = fr_bio_error(GENERIC); fail: my->info.socket = (fr_socket_t) { @@ -1182,7 +1169,7 @@ int fr_bio_fd_open(fr_bio_t *bio, fr_bio_fd_config_t const *cfg) my->info.state = FR_BIO_FD_STATE_CLOSED; my->info.cfg = NULL; close(fd); - return -1; + return rcode; } #ifdef FD_CLOEXEC @@ -1192,7 +1179,7 @@ int fr_bio_fd_open(fr_bio_t *bio, fr_bio_fd_config_t const *cfg) rcode = fcntl(fd, F_GETFD); if (rcode >= 0) { if (fcntl(fd, F_SETFD, rcode | FD_CLOEXEC) < 0) { - fr_strerror_printf("Failed opening setting FD_CLOEXE: %s", fr_syserror(errno)); + fr_strerror_printf("Failed setting FD_CLOEXEC: %s", fr_syserror(errno)); goto fail; } } @@ -1287,15 +1274,14 @@ int fr_bio_fd_open(fr_bio_t *bio, fr_bio_fd_config_t const *cfg) if (fr_bio_fd_init_connected(my) < 0) goto fail; break; - case FR_BIO_FD_ACCEPTED: - fr_assert(0); - break; - /* * Server socket which listens for new stream connections */ case FR_BIO_FD_LISTEN: - fr_assert(my->info.socket.type == SOCK_STREAM); + if ((my->info.socket.type == SOCK_DGRAM) && !cfg->reuse_port) { + fr_strerror_const("reuseport must be set for datagram sockets"); + goto fail; + } switch (my->info.socket.af) { case AF_INET: diff --git a/src/lib/bio/fd_priv.h b/src/lib/bio/fd_priv.h index a338e53218..98fb7b914e 100644 --- a/src/lib/bio/fd_priv.h +++ b/src/lib/bio/fd_priv.h @@ -70,3 +70,5 @@ int fr_bio_fd_init_connected(fr_bio_fd_t *my); int fr_bio_fd_init_listen(fr_bio_fd_t *my); int fr_bio_fd_socket_name(fr_bio_fd_t *my); + +void fr_bio_fd_name(fr_bio_fd_t *my); diff --git a/src/lib/bio/network.c b/src/lib/bio/network.c index 50c1a9020f..397897c6fa 100644 --- a/src/lib/bio/network.c +++ b/src/lib/bio/network.c @@ -134,7 +134,6 @@ fr_bio_t *fr_bio_network_alloc(TALLOC_CTX *ctx, fr_ipaddr_t const *allow, fr_ipa case FR_BIO_FD_INVALID: case FR_BIO_FD_CONNECTED: - case FR_BIO_FD_ACCEPTED: return NULL; case FR_BIO_FD_LISTEN: