The branch, master has been updated
via cb5d579 gitlab-ci: Add a 32bit build
via df91870 Add fix for incorrect mapping of fcntl64() -> fcntl(),
causing locking failures
via ceb139d Add test for F_SETLK as this is needs to be 64-bit aware on
32-bit userspace
from b15c02f tests: New test with poll
https://git.samba.org/?p=socket_wrapper.git;a=shortlog;h=master
- Log -----------------------------------------------------------------
commit cb5d5790fff30e3be5a9465a85b1ac0aaaebfed2
Author: Andreas Schneider <a...@samba.org>
Date: Fri May 5 07:14:26 2023 +0200
gitlab-ci: Add a 32bit build
BUG: https://bugzilla.samba.org/show_bug.cgi?id=15367
Signed-off-by: Andreas Schneider <a...@samba.org>
Reviewed-by: Andrew Bartlett <abart...@samba.org>
commit df918708717c084ec9048be2864edcde81816108
Author: Andrew Bartlett <abart...@samba.org>
Date: Fri May 5 13:34:00 2023 +1200
Add fix for incorrect mapping of fcntl64() -> fcntl(), causing locking
failures
We need to call fcntl64() if possible for 32-bit hosts
This is a strange case of socket_wrapper breaking normal file operation.
Newer glibc has introduced fcntl64 and symbol renaming but
the end function call was not caught by the automatic rename.
This means socket_wrapper would call fcntl(), not fcntl64 in libc
and this would do a "struct flock" -> "struct flock64" translation on the
supplied argument, despite this being already a flock64 from
the caller.
This in turn changed the lock offset values (eg to 0, 0).
BUG: https://bugzilla.samba.org/show_bug.cgi?id=15367
Signed-off-by: Andrew Bartlett <abart...@samba.org>
Reviewed-by: Andreas Schneider <a...@samba.org>
commit ceb139dc42c50275a11ca974ef8800032cf24b6f
Author: Andrew Bartlett <abart...@samba.org>
Date: Fri May 5 13:15:51 2023 +1200
Add test for F_SETLK as this is needs to be 64-bit aware on 32-bit userspace
If this is not correctly routed to fcntl64 (where that exists) then an
incorrect thunking could be applied breaking the functionality.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=15367
Signed-off-by: Andrew Bartlett <abart...@samba.org>
Reviewed-by: Andreas Schneider <a...@samba.org>
-----------------------------------------------------------------------
Summary of changes:
.gitlab-ci.yml | 20 ++++++++++
ConfigureChecks.cmake | 1 +
cmake/Toolchain-cross-m32.cmake | 23 +++++++++++
src/socket_wrapper.c | 25 ++++++++++++
tests/CMakeLists.txt | 1 +
tests/test_fcntl_lock.c | 86 +++++++++++++++++++++++++++++++++++++++++
6 files changed, 156 insertions(+)
create mode 100644 cmake/Toolchain-cross-m32.cmake
create mode 100644 tests/test_fcntl_lock.c
Changeset truncated at 500 lines:
diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index d5dc461..ef98aeb 100644
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@@ -224,6 +224,26 @@ tumbleweed/x86_64/clang:
paths:
- obj/
+tumbleweed/x86/gcc:
+ stage: test
+ image: $CI_REGISTRY/$BUILD_IMAGES_PROJECT:$TUMBLEWEED_BUILD
+ script:
+ - mkdir -p obj && cd obj && cmake
+ -DCMAKE_TOOLCHAIN_FILE=../cmake/Toolchain-cross-m32.cmake
+ -DCMAKE_BUILD_TYPE=RelWithDebInfo
+ -DPICKY_DEVELOPER=ON
+ -DUNIT_TESTING=ON .. &&
+ make -j$(nproc) && ctest --output-on-failure
+ tags:
+ - shared
+ except:
+ - tags
+ artifacts:
+ expire_in: 1 week
+ when: on_failure
+ paths:
+ - obj/
+
tumbleweed/static-analysis:
stage: analysis
image: $CI_REGISTRY/$BUILD_IMAGES_PROJECT:$TUMBLEWEED_BUILD
diff --git a/ConfigureChecks.cmake b/ConfigureChecks.cmake
index b820a65..c99e2ae 100644
--- a/ConfigureChecks.cmake
+++ b/ConfigureChecks.cmake
@@ -80,6 +80,7 @@ check_function_exists(__close_nocancel HAVE___CLOSE_NOCANCEL)
check_function_exists(recvmmsg HAVE_RECVMMSG)
check_function_exists(sendmmsg HAVE_SENDMMSG)
check_function_exists(syscall HAVE_SYSCALL)
+check_function_exists(fcntl64 HAVE_FCNTL64)
if (UNIX)
find_library(DLFCN_LIBRARY dl)
diff --git a/cmake/Toolchain-cross-m32.cmake b/cmake/Toolchain-cross-m32.cmake
new file mode 100644
index 0000000..7918c60
--- /dev/null
+++ b/cmake/Toolchain-cross-m32.cmake
@@ -0,0 +1,23 @@
+set(CMAKE_C_FLAGS "-m32" CACHE STRING "C compiler flags" FORCE)
+set(CMAKE_CXX_FLAGS "-m32" CACHE STRING "C++ compiler flags" FORCE)
+
+set(LIB32 /usr/lib) # Fedora
+
+if(EXISTS /usr/lib32)
+ set(LIB32 /usr/lib32) # Arch, Solus
+endif()
+
+set(CMAKE_SYSTEM_LIBRARY_PATH ${LIB32} CACHE STRING "system library search
path" FORCE)
+set(CMAKE_LIBRARY_PATH ${LIB32} CACHE STRING "library search path"
FORCE)
+
+# this is probably unlikely to be needed, but just in case
+set(CMAKE_EXE_LINKER_FLAGS "-m32 -L${LIB32}" CACHE STRING "executable
linker flags" FORCE)
+set(CMAKE_SHARED_LINKER_FLAGS "-m32 -L${LIB32}" CACHE STRING "shared library
linker flags" FORCE)
+set(CMAKE_MODULE_LINKER_FLAGS "-m32 -L${LIB32}" CACHE STRING "module linker
flags" FORCE)
+
+# on Fedora and Arch and similar, point pkgconfig at 32 bit .pc files. We have
+# to include the regular system .pc files as well (at the end), because some
+# are not always present in the 32 bit directory
+if(EXISTS ${LIB32}/pkgconfig)
+ set(ENV{PKG_CONFIG_LIBDIR}
${LIB32}/pkgconfig:/usr/share/pkgconfig:/usr/lib/pkgconfig:/usr/lib64/pkgconfig)
+endiF()
diff --git a/src/socket_wrapper.c b/src/socket_wrapper.c
index bf4a976..de2f732 100644
--- a/src/socket_wrapper.c
+++ b/src/socket_wrapper.c
@@ -611,7 +611,11 @@ struct swrap_libc_symbols {
SWRAP_SYMBOL_ENTRY(connect);
SWRAP_SYMBOL_ENTRY(dup);
SWRAP_SYMBOL_ENTRY(dup2);
+#ifdef HAVE_FCNTL64
+ SWRAP_SYMBOL_ENTRY(fcntl64);
+#else
SWRAP_SYMBOL_ENTRY(fcntl);
+#endif
SWRAP_SYMBOL_ENTRY(fopen);
#ifdef HAVE_FOPEN64
SWRAP_SYMBOL_ENTRY(fopen64);
@@ -978,7 +982,24 @@ static int libc_vfcntl(int fd, int cmd, va_list ap)
arg = va_arg(ap, void *);
+ /*
+ * If fcntl64 exists then this is a system were fcntl is
+ * renamed (including when building this file), and so we must
+ * assume that the binary under test was built with
+ * -D_FILE_OFFSET_BITS=64 and pass on to fcntl64.
+ *
+ * If we are wrong, then fcntl is unwrapped, but while that is
+ * not ideal, is is also unlikely.
+ *
+ * In any case, it is always wrong to map fcntl64() to fcntl()
+ * as this will cause a thunk from struct flock -> flock64
+ * that the caller had already prepared for.
+ */
+#ifdef HAVE_FCNTL64
+ rc = swrap.libc.symbols._libc_fcntl64.f(fd, cmd, arg);
+#else
rc = swrap.libc.symbols._libc_fcntl.f(fd, cmd, arg);
+#endif
return rc;
}
@@ -1400,7 +1421,11 @@ static void __swrap_bind_symbol_all_once(void)
swrap_bind_symbol_libsocket(connect);
swrap_bind_symbol_libc(dup);
swrap_bind_symbol_libc(dup2);
+#ifdef HAVE_FCNTL64
+ swrap_bind_symbol_libc(fcntl64);
+#else
swrap_bind_symbol_libc(fcntl);
+#endif
swrap_bind_symbol_libc(fopen);
#ifdef HAVE_FOPEN64
swrap_bind_symbol_libc(fopen64);
diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt
index b1a3c6c..e35c258 100644
--- a/tests/CMakeLists.txt
+++ b/tests/CMakeLists.txt
@@ -58,6 +58,7 @@ set(SWRAP_TESTS
test_tcp_listen
test_tcp_dup2
test_fcntl
+ test_fcntl_lock
test_echo_tcp_connect
test_echo_tcp_bind
test_echo_tcp_socket_options
diff --git a/tests/test_fcntl_lock.c b/tests/test_fcntl_lock.c
new file mode 100644
index 0000000..98cf7c0
--- /dev/null
+++ b/tests/test_fcntl_lock.c
@@ -0,0 +1,86 @@
+#include "torture.h"
+
+#include <cmocka.h>
+#include <stdlib.h>
+#include <stdio.h>
+#include <string.h>
+#include <limits.h>
+#include <unistd.h>
+#include <fcntl.h>
+#include <errno.h>
+
+static int setup(void **state)
+{
+ char test_tmpdir[256];
+ const char *p;
+
+ (void) state; /* unused */
+
+ snprintf(test_tmpdir, sizeof(test_tmpdir),
"/tmp/test_socket_wrapper_XXXXXX");
+
+ p = mkdtemp(test_tmpdir);
+ assert_non_null(p);
+
+ *state = strdup(p);
+ return 0;
+}
+
+static int teardown(void **state)
+{
+ char remove_cmd[PATH_MAX] = {0};
+ char *s = (char *)*state;
+ int rc;
+
+ if (s == NULL) {
+ return -1;
+ }
+
+ snprintf(remove_cmd, sizeof(remove_cmd), "rm -rf %s", s);
+ free(s);
+
+ rc = system(remove_cmd);
+ if (rc < 0) {
+ fprintf(stderr, "%s failed: %s", remove_cmd, strerror(errno));
+ }
+
+ return rc;
+}
+
+static void test_fcntl_lock(void **state)
+{
+ char file[PATH_MAX];
+ int fd, rc;
+ struct flock lock;
+ char *s = (char *)*state;
+
+ rc = snprintf(file, sizeof(file), "%s/file", s);
+ assert_in_range(rc, 0, PATH_MAX);
+
+ fd = open(file, O_CREAT, 0600);
+ assert_return_code(fd, errno);
+
+ lock.l_type = F_RDLCK;
+ lock.l_whence = SEEK_SET;
+ lock.l_start = 0;
+ lock.l_len = 4;
+ lock.l_pid = 0;
+
+ rc = fcntl(fd, F_SETLK, &lock);
+ assert_return_code(rc, errno);
+
+ rc = unlink(file);
+ assert_return_code(rc, errno);
+}
+
+
+int main(void) {
+ int rc;
+
+ const struct CMUnitTest tcp_fcntl_lock_tests[] = {
+ cmocka_unit_test(test_fcntl_lock),
+ };
+
+ rc = cmocka_run_group_tests(tcp_fcntl_lock_tests, setup, teardown);
+
+ return rc;
+}
--
Socket Wrapper Repository
