[SCM] Socket Wrapper Repository - branch master updated

Andreas Schneider asn at samba.org
Mon May 8 10:55:57 UTC 2023


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 <asn at 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 <asn at samba.org>
    Reviewed-by: Andrew Bartlett <abartlet at samba.org>

commit df918708717c084ec9048be2864edcde81816108
Author: Andrew Bartlett <abartlet at 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 <abartlet at samba.org>
    Reviewed-by: Andreas Schneider <asn at samba.org>

commit ceb139dc42c50275a11ca974ef8800032cf24b6f
Author: Andrew Bartlett <abartlet at 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 <abartlet at samba.org>
    Reviewed-by: Andreas Schneider <asn at 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



More information about the samba-cvs mailing list