diff --git a/SOURCES/0001-Move-closing-standard-FDs-after-listen.patch b/SOURCES/0001-Move-closing-standard-FDs-after-listen.patch new file mode 100644 index 0000000..12511e7 --- /dev/null +++ b/SOURCES/0001-Move-closing-standard-FDs-after-listen.patch @@ -0,0 +1,46 @@ +From 40fea4552377504ce69935149e64e39a595f4600 Mon Sep 17 00:00:00 2001 +From: =?UTF-8?q?Ond=C5=99ej=20Lyson=C4=9Bk?= +Date: Sat, 3 Aug 2019 17:50:14 +0200 +Subject: [PATCH 1/2] Move closing standard FDs after listen() + +The vsf_sysutil_close() calls need to be moved a bit further so that +die() works properly in case listen() fails. + +I see no reason the calls should be placed before listen() +specifically, as they are now. My guess is that the author who added +the calls thought that listen() is a blocking call, which is not the +case. The only thing we need to satisfy is that close() is called +before accept, because that is a blocking call. That's all that is +needed to fix the bug that was fixed by adding the close() calls. + +Resolves: rhbz#1666380 +--- + standalone.c | 6 +++--- + 1 file changed, 3 insertions(+), 3 deletions(-) + +diff --git a/standalone.c b/standalone.c +index 3f35e9e..b358ca1 100644 +--- a/standalone.c ++++ b/standalone.c +@@ -152,15 +152,15 @@ vsf_standalone_main(void) + vsf_sysutil_kill(vsf_sysutil_getppid(), kVSFSysUtilSigUSR1); + } + } +- vsf_sysutil_close(0); +- vsf_sysutil_close(1); +- vsf_sysutil_close(2); + retval = vsf_sysutil_listen(listen_sock, VSFTP_LISTEN_BACKLOG); + if (vsf_sysutil_retval_is_error(retval)) + { + die("could not listen"); + } + vsf_sysutil_sockaddr_alloc(&p_accept_addr); ++ vsf_sysutil_close(0); ++ vsf_sysutil_close(1); ++ vsf_sysutil_close(2); + while (1) + { + struct vsf_client_launch child_info; +-- +2.20.1 + diff --git a/SOURCES/0001-Set-s_uwtmp_inserted-only-after-record-insertion-rem.patch b/SOURCES/0001-Set-s_uwtmp_inserted-only-after-record-insertion-rem.patch new file mode 100644 index 0000000..a2be65a --- /dev/null +++ b/SOURCES/0001-Set-s_uwtmp_inserted-only-after-record-insertion-rem.patch @@ -0,0 +1,53 @@ +From 96698a525784ad91cb27b572dd5f871c183fdfa5 Mon Sep 17 00:00:00 2001 +From: =?UTF-8?q?Ond=C5=99ej=20Lyson=C4=9Bk?= +Date: Sun, 28 Jul 2019 12:25:35 +0200 +Subject: [PATCH 1/3] Set s_uwtmp_inserted only after record insertion/removal + +pututxline() is the function that actually inserts the new record, so +setting 's_uwtmp_inserted' before calling pututxline() doesn't make +sense. + +We'll need this change for other fixes. +--- + sysdeputil.c | 4 ++-- + 1 file changed, 2 insertions(+), 2 deletions(-) + +diff --git a/sysdeputil.c b/sysdeputil.c +index 4fe56c2..bd1e8c9 100644 +--- a/sysdeputil.c ++++ b/sysdeputil.c +@@ -1224,7 +1224,6 @@ vsf_insert_uwtmp(const struct mystr* p_user_str, + sizeof(s_utent.ut_line)); + str_free(&line_str); + } +- s_uwtmp_inserted = 1; + s_utent.ut_type = USER_PROCESS; + s_utent.ut_pid = vsf_sysutil_getpid(); + vsf_sysutil_strcpy(s_utent.ut_user, str_getbuf(p_user_str), +@@ -1235,6 +1234,7 @@ vsf_insert_uwtmp(const struct mystr* p_user_str, + setutxent(); + (void) pututxline(&s_utent); + endutxent(); ++ s_uwtmp_inserted = 1; + updwtmpx(WTMPX_FILE, &s_utent); + } + +@@ -1245,7 +1245,6 @@ vsf_remove_uwtmp(void) + { + return; + } +- s_uwtmp_inserted = 0; + s_utent.ut_type = DEAD_PROCESS; + vsf_sysutil_memclr(s_utent.ut_user, sizeof(s_utent.ut_user)); + vsf_sysutil_memclr(s_utent.ut_host, sizeof(s_utent.ut_host)); +@@ -1253,6 +1252,7 @@ vsf_remove_uwtmp(void) + setutxent(); + (void) pututxline(&s_utent); + endutxent(); ++ s_uwtmp_inserted = 0; + s_utent.ut_tv.tv_sec = vsf_sysutil_get_time_sec(); + updwtmpx(WTMPX_FILE, &s_utent); + } +-- +2.20.1 + diff --git a/SOURCES/0002-Prevent-recursion-in-bug.patch b/SOURCES/0002-Prevent-recursion-in-bug.patch new file mode 100644 index 0000000..061fd1e --- /dev/null +++ b/SOURCES/0002-Prevent-recursion-in-bug.patch @@ -0,0 +1,107 @@ +From e679a3ce0f2cf1558da31e0bccd9e2398b89c7e9 Mon Sep 17 00:00:00 2001 +From: =?UTF-8?q?Ond=C5=99ej=20Lyson=C4=9Bk?= +Date: Tue, 30 Jul 2019 16:07:01 +0200 +Subject: [PATCH 2/2] Prevent recursion in bug() + +Resolves: rhbz#1666380 +--- + sysutil.c | 35 +++++++++++++++++++++++++++++++---- + sysutil.h | 1 + + utility.c | 12 +++++++----- + 3 files changed, 39 insertions(+), 9 deletions(-) + +diff --git a/sysutil.c b/sysutil.c +index fd07d99..e2df671 100644 +--- a/sysutil.c ++++ b/sysutil.c +@@ -774,21 +774,48 @@ vsf_sysutil_deactivate_linger_failok(int fd) + (void) setsockopt(fd, SOL_SOCKET, SO_LINGER, &the_linger, sizeof(the_linger)); + } + +-void +-vsf_sysutil_activate_noblock(int fd) ++static int ++vsf_sysutil_activate_noblock_internal(int fd, int return_err) + { + int retval; + int curr_flags = fcntl(fd, F_GETFL); + if (vsf_sysutil_retval_is_error(curr_flags)) + { +- die("fcntl"); ++ if (return_err) ++ { ++ return -1; ++ } ++ else ++ { ++ die("fcntl"); ++ } + } + curr_flags |= O_NONBLOCK; + retval = fcntl(fd, F_SETFL, curr_flags); + if (retval != 0) + { +- die("fcntl"); ++ if (return_err) ++ { ++ return -1; ++ } ++ else ++ { ++ die("fcntl"); ++ } + } ++ return 0; ++} ++ ++void ++vsf_sysutil_activate_noblock(int fd) ++{ ++ (void) vsf_sysutil_activate_noblock_internal(fd, 0); ++} ++ ++int ++vsf_sysutil_activate_noblock_no_die(int fd) ++{ ++ return vsf_sysutil_activate_noblock_internal(fd, 1); + } + + void +diff --git a/sysutil.h b/sysutil.h +index 2df14ed..0772423 100644 +--- a/sysutil.h ++++ b/sysutil.h +@@ -281,6 +281,7 @@ void vsf_sysutil_activate_oobinline(int fd); + void vsf_sysutil_activate_linger(int fd); + void vsf_sysutil_deactivate_linger_failok(int fd); + void vsf_sysutil_activate_noblock(int fd); ++int vsf_sysutil_activate_noblock_no_die(int fd); + void vsf_sysutil_deactivate_noblock(int fd); + /* This does SHUT_RDWR */ + void vsf_sysutil_shutdown_failok(int fd); +diff --git a/utility.c b/utility.c +index 75e5bdd..5619a04 100644 +--- a/utility.c ++++ b/utility.c +@@ -47,11 +47,13 @@ bug(const char* p_text) + { + vsf_log_die(p_text); + } +- vsf_sysutil_activate_noblock(VSFTP_COMMAND_FD); +- (void) vsf_sysutil_write_loop(VSFTP_COMMAND_FD, "500 OOPS: ", 10); +- (void) vsf_sysutil_write_loop(VSFTP_COMMAND_FD, p_text, +- vsf_sysutil_strlen(p_text)); +- (void) vsf_sysutil_write_loop(VSFTP_COMMAND_FD, "\r\n", 2); ++ if (vsf_sysutil_activate_noblock_no_die(VSFTP_COMMAND_FD) == 0) ++ { ++ (void) vsf_sysutil_write_loop(VSFTP_COMMAND_FD, "500 OOPS: ", 10); ++ (void) vsf_sysutil_write_loop(VSFTP_COMMAND_FD, p_text, ++ vsf_sysutil_strlen(p_text)); ++ (void) vsf_sysutil_write_loop(VSFTP_COMMAND_FD, "\r\n", 2); ++ } + if (tunable_log_die) + { + /* Workaround for https://github.com/systemd/systemd/issues/2913 */ +-- +2.20.1 + diff --git a/SOURCES/0002-Repeat-pututxline-if-it-fails-with-EINTR.patch b/SOURCES/0002-Repeat-pututxline-if-it-fails-with-EINTR.patch new file mode 100644 index 0000000..fcbc728 --- /dev/null +++ b/SOURCES/0002-Repeat-pututxline-if-it-fails-with-EINTR.patch @@ -0,0 +1,105 @@ +From 896b3694ca062d747cd67e9e9ba246adb3fc706b Mon Sep 17 00:00:00 2001 +From: =?UTF-8?q?Ond=C5=99ej=20Lyson=C4=9Bk?= +Date: Mon, 5 Aug 2019 13:55:37 +0200 +Subject: [PATCH 2/3] Repeat pututxline() if it fails with EINTR + +This is a partial fix for rhbz#1688848. We cannot resolve it +completely until glibc bug rhbz#1734791 is fixed. See +https://bugzilla.redhat.com/show_bug.cgi?id=1688848#c13. + +The maximum number of attempts is currently 2, which might seem +low. However setting it to 2 was a decision based on data - see +https://bugzilla.redhat.com/show_bug.cgi?id=1688848#c16. + +Resolves: rhbz#1688848 +--- + sysdeputil.c | 53 +++++++++++++++++++++++++++++++++++++++++++++------- + 1 file changed, 46 insertions(+), 7 deletions(-) + +diff --git a/sysdeputil.c b/sysdeputil.c +index bd1e8c9..4fbcca7 100644 +--- a/sysdeputil.c ++++ b/sysdeputil.c +@@ -1203,6 +1203,8 @@ void + vsf_insert_uwtmp(const struct mystr* p_user_str, + const struct mystr* p_host_str) + { ++ int attempts; ++ + if (sizeof(s_utent.ut_line) < 16) + { + return; +@@ -1231,16 +1233,35 @@ vsf_insert_uwtmp(const struct mystr* p_user_str, + vsf_sysutil_strcpy(s_utent.ut_host, str_getbuf(p_host_str), + sizeof(s_utent.ut_host)); + s_utent.ut_tv.tv_sec = vsf_sysutil_get_time_sec(); +- setutxent(); +- (void) pututxline(&s_utent); +- endutxent(); +- s_uwtmp_inserted = 1; ++ for (attempts = 2; attempts > 0; --attempts) ++ { ++ struct utmpx* p_res; ++ setutxent(); ++ p_res = pututxline(&s_utent); ++ /* For now we'll ignore errors other than EINTR and EAGAIN */ ++ if (p_res != NULL || (errno != EINTR && errno != EAGAIN)) ++ { ++ break; ++ } ++ } ++ if (attempts == 0) ++ { ++ /* This makes us skip pututxline() in vsf_remove_uwtmp() */ ++ s_uwtmp_inserted = -1; ++ } ++ else ++ { ++ s_uwtmp_inserted = 1; ++ endutxent(); ++ } + updwtmpx(WTMPX_FILE, &s_utent); + } + + void + vsf_remove_uwtmp(void) + { ++ int attempts; ++ + if (!s_uwtmp_inserted) + { + return; +@@ -1249,9 +1270,27 @@ vsf_remove_uwtmp(void) + vsf_sysutil_memclr(s_utent.ut_user, sizeof(s_utent.ut_user)); + vsf_sysutil_memclr(s_utent.ut_host, sizeof(s_utent.ut_host)); + s_utent.ut_tv.tv_sec = 0; +- setutxent(); +- (void) pututxline(&s_utent); +- endutxent(); ++ if (s_uwtmp_inserted == 1) ++ { ++ for (attempts = 2; attempts > 0; --attempts) ++ { ++ struct utmpx* p_res; ++ setutxent(); ++ p_res = pututxline(&s_utent); ++ /* For now we'll ignore errors other than EINTR and EAGAIN */ ++ if (p_res != NULL || (errno != EINTR && errno != EAGAIN)) ++ { ++ break; ++ } ++ } ++ if (attempts != 0) ++ { ++ endutxent(); ++ } ++ } ++ /* Set s_uwtmp_inserted to 0 regardless of the result of ++ * pututxline() to make sure we won't run this function twice. ++ */ + s_uwtmp_inserted = 0; + s_utent.ut_tv.tv_sec = vsf_sysutil_get_time_sec(); + updwtmpx(WTMPX_FILE, &s_utent); +-- +2.20.1 + diff --git a/SOURCES/0003-Repeat-pututxline-until-it-succeeds-if-it-fails-with.patch b/SOURCES/0003-Repeat-pututxline-until-it-succeeds-if-it-fails-with.patch new file mode 100644 index 0000000..0e3bcd4 --- /dev/null +++ b/SOURCES/0003-Repeat-pututxline-until-it-succeeds-if-it-fails-with.patch @@ -0,0 +1,109 @@ +From 7957425ef5ab365fc96ea0615f99705581c6dbd8 Mon Sep 17 00:00:00 2001 +From: =?UTF-8?q?Ond=C5=99ej=20Lyson=C4=9Bk?= +Date: Mon, 12 Aug 2019 18:15:36 +0200 +Subject: [PATCH 3/3] Repeat pututxline() until it succeeds if it fails with + EINTR + +Since the pututxline() bug rhbz#1749439 is now fixed in glibc in +Fedora and RHEL-8, we can implement a complete solution for the stale +utmp entries issue originally reported as rhbz#1688848. + +This patch is a followup to commit 896b3694ca062d7. + +Resolves: rhbz#1688852 +Resolves: rhbz#1737433 +--- + sysdeputil.c | 53 +++++++++++++--------------------------------------- + 1 file changed, 13 insertions(+), 40 deletions(-) + +diff --git a/sysdeputil.c b/sysdeputil.c +index 4fbcca7..75be680 100644 +--- a/sysdeputil.c ++++ b/sysdeputil.c +@@ -1203,7 +1203,7 @@ void + vsf_insert_uwtmp(const struct mystr* p_user_str, + const struct mystr* p_host_str) + { +- int attempts; ++ struct utmpx* p_res; + + if (sizeof(s_utent.ut_line) < 16) + { +@@ -1233,34 +1233,21 @@ vsf_insert_uwtmp(const struct mystr* p_user_str, + vsf_sysutil_strcpy(s_utent.ut_host, str_getbuf(p_host_str), + sizeof(s_utent.ut_host)); + s_utent.ut_tv.tv_sec = vsf_sysutil_get_time_sec(); +- for (attempts = 2; attempts > 0; --attempts) ++ setutxent(); ++ do + { +- struct utmpx* p_res; +- setutxent(); + p_res = pututxline(&s_utent); + /* For now we'll ignore errors other than EINTR and EAGAIN */ +- if (p_res != NULL || (errno != EINTR && errno != EAGAIN)) +- { +- break; +- } +- } +- if (attempts == 0) +- { +- /* This makes us skip pututxline() in vsf_remove_uwtmp() */ +- s_uwtmp_inserted = -1; +- } +- else +- { +- s_uwtmp_inserted = 1; +- endutxent(); +- } ++ } while (p_res == NULL && (errno == EINTR || errno == EAGAIN)); ++ s_uwtmp_inserted = 1; ++ endutxent(); + updwtmpx(WTMPX_FILE, &s_utent); + } + + void + vsf_remove_uwtmp(void) + { +- int attempts; ++ struct utmpx* p_res; + + if (!s_uwtmp_inserted) + { +@@ -1270,27 +1257,13 @@ vsf_remove_uwtmp(void) + vsf_sysutil_memclr(s_utent.ut_user, sizeof(s_utent.ut_user)); + vsf_sysutil_memclr(s_utent.ut_host, sizeof(s_utent.ut_host)); + s_utent.ut_tv.tv_sec = 0; +- if (s_uwtmp_inserted == 1) ++ setutxent(); ++ do + { +- for (attempts = 2; attempts > 0; --attempts) +- { +- struct utmpx* p_res; +- setutxent(); +- p_res = pututxline(&s_utent); +- /* For now we'll ignore errors other than EINTR and EAGAIN */ +- if (p_res != NULL || (errno != EINTR && errno != EAGAIN)) +- { +- break; +- } +- } +- if (attempts != 0) +- { +- endutxent(); +- } +- } +- /* Set s_uwtmp_inserted to 0 regardless of the result of +- * pututxline() to make sure we won't run this function twice. +- */ ++ p_res = pututxline(&s_utent); ++ /* For now we'll ignore errors other than EINTR and EAGAIN */ ++ } while (p_res == NULL && (errno == EINTR || errno == EAGAIN)); ++ endutxent(); + s_uwtmp_inserted = 0; + s_utent.ut_tv.tv_sec = vsf_sysutil_get_time_sec(); + updwtmpx(WTMPX_FILE, &s_utent); +-- +2.20.1 + diff --git a/SPECS/vsftpd.spec b/SPECS/vsftpd.spec index da2ee12..560520e 100644 --- a/SPECS/vsftpd.spec +++ b/SPECS/vsftpd.spec @@ -2,7 +2,7 @@ Name: vsftpd Version: 3.0.3 -Release: 28%{?dist} +Release: 30%{?dist} Summary: Very Secure Ftp Daemon Group: System Environment/Daemons @@ -88,6 +88,11 @@ Patch56: 0056-Log-die-calls-to-syslog.patch Patch57: 0057-Improve-error-message-when-max-number-of-bind-attemp.patch Patch58: 0058-Make-the-max-number-of-bind-retries-tunable.patch Patch59: 0059-Fix-SEGFAULT-when-running-in-a-container-as-PID-1.patch +Patch60: 0001-Move-closing-standard-FDs-after-listen.patch +Patch61: 0002-Prevent-recursion-in-bug.patch +Patch62: 0001-Set-s_uwtmp_inserted-only-after-record-insertion-rem.patch +Patch63: 0002-Repeat-pututxline-if-it-fails-with-EINTR.patch +Patch64: 0003-Repeat-pututxline-until-it-succeeds-if-it-fails-with.patch %description vsftpd is a Very Secure FTP daemon. It was written completely from @@ -156,6 +161,14 @@ mkdir -p $RPM_BUILD_ROOT/%{_var}/ftp/pub %{_var}/ftp %changelog +* Thu Nov 28 2019 Ondřej Lysoněk - 3.0.3-30 +- Fix a problem with bad utmp entries when pututxline() fails +- Resolves: rhbz#1688852 + +* Thu Nov 28 2019 Ondřej Lysoněk - 3.0.3-29 +- Fix segfault when listen() returns an error +- Resolves: rhbz#1734340 + * Wed Jul 25 2018 Ondřej Lysoněk - 3.0.3-28 - Rebuilt, switched to SHA512 source tarball hash