empty blocklist generated

[hgy59]

What is the issue?

In SynoCommunity, we are building transmission packages for Synology NAS.

There is an issue that the downloaded blocklist file is not correctly processed and results in an empty blocklist file.
see: SynoCommunity/spksrc#5685

The result of the analysis of the source code (in libtransmission/file-posix.cc) is:
The tr_sys_path_copy does not work here.

It is related to the fallback from sendfile64 to user-space copy.
When reaching the fallback code (after trying sendfile64), the errno_cpy variable is 22 (EINVAL), but only 18 (EXDEV) and 0 are accepted to continue with use-space copy.
This results in an empty blocklist file and an entry in the log file:

[2023-09-01 13:26:12.373] WRN blocklist.cc:505 Couldn't save '/volume1/@appdata/transmission/blocklists/blocklist': Unable to read/write: Invalid argument (22) (blocklist.cc:505)

For the failing targets, the configure of build logs

-- Performing Test NO_LFS_MACROS_REQUIRED
-- Performing Test NO_LFS_MACROS_REQUIRED - Failed
-- Performing Test FILE_OFFSET_BITS_LFS_MACRO_REQUIRED
-- Performing Test FILE_OFFSET_BITS_LFS_MACRO_REQUIRED - Success
...
-- Looking for sendfile64
-- Looking for sendfile64 - found

I have not a clue, whether this error is related to large file support or sendfile64 compatibility, but for SynoCommunity Transmission package it could be fixed with this patch:

# fix empty blocklist files is some situations
# ARMv7 archs (and may be other systems with older kernel) fail to fallback from sendfile64 usage to user-space copy
# The first try with sendfile64 might fail not only with EXDEV (18) but also with EINVAL (22)
# this must be considered by the fallback to user-space copy
--- libtransmission/file-posix.cc.orig	2023-08-23 22:56:00.000000000 +0000
+++ libtransmission/file-posix.cc	2023-09-02 16:18:59.497180915 +0000
@@ -460,7 +460,7 @@
     /* Fallback to user-space copy. */
     /* if file_size>0 and errno_cpy==0, we probably never entered any copy attempt, also: */
     /* if we (still) got something to copy and we encountered certain error in previous attempts */
-    if (file_size > 0U && (errno_cpy == 0 || errno_cpy == EXDEV))
+    if (file_size > 0U && (errno_cpy == 0 || errno_cpy == EXDEV || errno_cpy == EINVAL))
     {
         static auto constexpr Buflen = size_t{ 1024U * 1024U }; /* 1024 KiB buffer */
         auto buf = std::vector<char>{};

I provide this information here, so this can be probably fixed upstream.

Which application of Transmission?

None

Which version of Transmission?

4.0.x

[ckerr]

@hgy59 looks reasonable to me. Would you like to open a PR that adds the new errno_cpy test to main?

[hgy59]

meanwhile I have modified the patch to disable the use of sendfile64 at all.
The first attempt worked, but the log entry has not gone away:

[2023-09-01 13:26:12.373] WRN blocklist.cc:505 Couldn't save '/volume1/@appdata/transmission/blocklists/blocklist': Unable to read/write: Invalid argument (22) (blocklist.cc:505)

The new patch avoids the definition of USE_SENDFILE64 to force the use of "user-space copy" and looks like this:

--- libtransmission/file-posix.cc.orig	2023-08-23 22:56:00.000000000 +0000
+++ libtransmission/file-posix.cc	2023-09-02 20:11:04.227014716 +0000
@@ -28,15 +28,12 @@
 #include <xfs/xfs.h>
 #endif
 
-/* OS-specific file copy (copy_file_range, sendfile64, or copyfile). */
+/* OS-specific file copy (copy_file_range or copyfile). */
 #if defined(__linux__)
 #include <linux/version.h>
 /* Linux's copy_file_range(2) is buggy prior to 5.3. */
 #if defined(HAVE_COPY_FILE_RANGE) && LINUX_VERSION_CODE >= KERNEL_VERSION(5, 3, 0)
 #define USE_COPY_FILE_RANGE
-#elif defined(HAVE_SENDFILE64)
-#include <sys/sendfile.h>
-#define USE_SENDFILE64
 #endif
 #elif defined(__APPLE__) && defined(HAVE_COPYFILE)
 #include <copyfile.h>

With this in mind, a general solution would require an adjustment of the build process. In a classical autoconf scenario I would say that configure has to detect an incompatible sendfile64 implementation (i.e. the call generates an EINVAL error) and USE_SENDFILE64 must not be defined in this situation. I don't have a clue how to fix this in cmake/CMakeLists.txt environments.

At least I would expect an option in CMakeLists.txt to manually disable the definition of HAVE_SENDFILE64.

---

And there is another "invalid argument" Warning in the log file on 32-bit arm.

[2023-09-02 22:51:34.061] WRN session-id.cc:74 Couldn't create '/volume1/@apptemp/transmission/tr_session_id_I2Njnr5HBQcxWyvLh5zWk4aRQmsv3hg2dA7IfI6auweescxd': Invalid argument (22) (session-id.cc:74)

As far as I can tell, this comes from the call of flock on the session file (this call is in tr_sys_file_lock in file-posix.cc too).
For this, I couldn't find a solution, as it is not clear, what the wrong argument is (tried to apply only the LOCK_EX flag and omit the LOCK_NB, but the error result was not gone).

With this warning in the logs, I don't know, whether the session-id file gets locked or not.
At least I have to know what side effects in transmission are expected, when the lock on the session-id file does not work.

Shall I create another issue for this?

[tearfur]

but the log entry has not gone away

@hgy59 Sounds to me all you need is avoid setting the tr_error struct instead of disabling sendfile64() altogether.

diff --git a/libtransmission/file-posix.cc b/libtransmission/file-posix.cc
index db40e36a1..d9daea5ab 100644
--- a/libtransmission/file-posix.cc
+++ b/libtransmission/file-posix.cc
@@ -439,7 +439,10 @@ bool tr_sys_path_copy(char const* src_path, char const* dst_path, tr_error** err
                 if (copied == -1)
                 {
                     errno_cpy = errno; /* remember me for later */
-                    tr_error_set_from_errno(error, errno);
+                    if (errno != EINVAL)
+                    {
+                        tr_error_set_from_errno(error, errno);
+                    }
                     if (file_size > 0U)
                     {
                         file_size = info->size; /* restore file_size for next fallback */

A similar thing is already in place for EXDEV generated by copy_file_range():

transmission/libtransmission/file-posix.cc

Lines 390 to 393 in de889c9

	if (errno != EXDEV) /* EXDEV is expected, don't log error */
	{
	tr_error_set_from_errno(error, errno);
	}

In a classical autoconf scenario I would say that configure has to detect an incompatible sendfile64 implementation

What variants of sendfile64() are there? As far as I know, Synology uses a custom Linux distribution, so sendfile64() should be the same there as any normal Linux systems.

[hgy59]

@hgy59 Sounds to me all you need is avoid setting the tr_error struct instead of disabling sendfile64() altogether.

@tearfur Thank you for this solution (I should have figured it out myself).

---

This are the declarations in sys/sendfile.h in the armv7 toolchain (DSM 7.1)

/* Send up to COUNT bytes from file associated with IN_FD starting at
   *OFFSET to descriptor OUT_FD.  Set *OFFSET to the IN_FD's file position
   following the read bytes.  If OFFSET is a null pointer, use the normal
   file position instead.  Return the number of written bytes, or -1 in
   case of error.  */
#ifndef __USE_FILE_OFFSET64
extern ssize_t sendfile (int __out_fd, int __in_fd, off_t *__offset,
			 size_t __count) __THROW;
#else
# ifdef __REDIRECT_NTH
extern ssize_t __REDIRECT_NTH (sendfile,
			       (int __out_fd, int __in_fd, __off64_t *__offset,
				size_t __count), sendfile64);
# else
#  define sendfile sendfile64
# endif
#endif
#ifdef __USE_LARGEFILE64
extern ssize_t sendfile64 (int __out_fd, int __in_fd, __off64_t *__offset,
			   size_t __count) __THROW;
#endif

I supposed an issue with USE_FILE_OFFSET64 but didn't get it working.

BTW the manpages document error EINVAL for sendfile64 like this:

ssize_t sendfile(int out_fd, int in_fd, off_t * offset ", size_t" " count" );

EINVAL   Descriptor is not valid or locked, or an mmap(2)-like operation is not available for in_fd.

could there be a permission issue with the blocklist file that applies to sendfile64 but not to user-space file copy implementation?

[hgy59]

And there is another "invalid argument" Warning in the log file on 32-bit arm.

[2023-09-02 22:51:34.061] WRN session-id.cc:74 Couldn't create '/volume1/@apptemp/transmission/tr_session_id_I2Njnr5HBQcxWyvLh5zWk4aRQmsv3hg2dA7IfI6auweescxd': Invalid argument (22) (session-id.cc:74)

As far as I can tell, this comes from the call of flock on the session file (this call is in tr_sys_file_lock in file-posix.cc too). For this, I couldn't find a solution, as it is not clear, what the wrong argument is (tried to apply only the LOCK_EX flag and omit the LOCK_NB, but the error result was not gone).

With this warning in the logs, I don't know, whether the session-id file gets locked or not. At least I have to know what side effects in transmission are expected, when the lock on the session-id file does not work.

Shall I create another issue for this?

I have found a solution for this:

The error above was not created by flock, but by fnctl with F_OFD_SETLK.
flock works sucessfully with 32-bit arm archs, but fnctl with F_OFD_SETLK results in "invalid argument (22)".

I fixed this in SynoCommunity transmission package by a patch for 32-bit arm archs, that enforces the use of flock in tr_sys_file_lock in file-posix.cc.

[hgy59]

@hgy59 looks reasonable to me. Would you like to open a PR that adds the new errno_cpy test to main?

PR created, is waiting for approval.

[tearfur]

@hgy59 F_OFD_SETLK has to be used with fcntl64() on 32-bit Linux. I think this needs to be addressed, so please create a new issue for it.

https://elixir.bootlin.com/linux/latest/source/fs/fcntl.c#L343