copy() fails on cifs mounts because of incorrect length (cfr_max) specified in streams.c:1584 copy_file_range()

[knowsshit]

Description

I noticed that Wordpress on Azure Web App with PHP 8.2 stack on Linux fails to install and upgrade anything because copy() fails when trying to copy files from /home/site/wwwroot/wp-content/upgrade/ to /home/site/wwwroot/wp-content/plugins/.

/home/ is a cifs mount.

The following code:

if ( ! copy( '/home/site/wwwroot/sourcefile', '/home/site/wwwroot/destfile' ) )
  echo "copy failed\n";

Resulted in this output:

copy failed

But I expected this output instead: (no output and successful file copy)

I did a strace:

copy_file_range(5, NULL, 6, NULL, 9223372036854775807, 0) = -1 EIO (Input/output error)

Here we can see that PHP specifies a very large length value for the fifth argument, and that copy_file_range() results in an I/O error. So I guess the large length value works fine on most filesystems, but not on cifs.

PHP source from php-8.2.2/main/streams/streams.c lines 1576,1577,1584:

                        /* clamp to INT_MAX to avoid EOVERFLOW */
                        const size_t cfr_max = MIN(maxlen, (size_t)SSIZE_MAX);
                        ssize_t result = copy_file_range(src_fd, NULL, dest_fd, NULL, cfr_max, 0);

I took the copy_file_range() example from https://man7.org/linux/man-pages/man2/copy_file_range.2.html that specifies the correct length by getting the file size with fstat(). Compiling it and running it with strace shows successful copy of the same files within the same /home cifs mount:

copy_file_range(3, NULL, 4, NULL, 11, 0) = 11

The solution would be to do the same as the example on https://man7.org/linux/man-pages/man2/copy_file_range.2.html and get the correct file size of the source file and specify it as the fifth argument to copy_file_range().

Additional info (some parts replaced with ... for privacy):

# php -v
PHP 8.2.0 (cli) (built: Dec 27 2022 20:38:32) (NTS)
Copyright (c) The PHP Group
Zend Engine v4.2.0, Copyright (c) Zend Technologies
    with Zend OPcache v8.2.0, Copyright (c), by Zend Technologies

# egrep '(Version|Features)' /proc/fs/cifs/DebugData
/proc/fs/cifs/DebugData:CIFS Version 2.10
/proc/fs/cifs/DebugData:Features: DFS,FSCACHE,STATS,DEBUG,ALLOW_INSECURE_LEGACY,WEAK_PW_HASH,CIFS_POSIX,UPCALL(SPNEGO),XATTR,ACL

# mount | grep home
//10.0.0.1/volume-...-default/... on /home type cifs (rw,relatime,vers=default,cache=strict,username=dummyadmin_...,domain=,uid=0,noforceuid,gid=0,noforcegid,addr=10.0.0.1,file_mode=0777,dir_mode=0777,soft,nounix,mapposix,mfsymlinks,noperm,rsize=1048576,wsize=1048576,echo_interval=60,actimeo=1)

# uname -a
Linux 3f6c4cc7fa2c 4.15.0-191-generic #202-Ubuntu SMP Thu Aug 4 01:49:29 UTC 2022 x86_64 GNU/Linux
# cat /etc/debian_version 
11.6

# cat /etc/motd 
   _____                               
  /  _  \ __________ _________   ____  
 /  /_\  \\___   /  |  \_  __ \_/ __ \ 
/    |    \/    /|  |  /|  | \/\  ___/ 
\____|__  /_____ \____/ |__|    \___  >
        \/      \/                  \/ 
A P P   S E R V I C E   O N   L I N U X

Documentation: http://aka.ms/webapp-linux
PHP quickstart: https://aka.ms/php-qs
PHP version : 8.2.0
Note: Any data outside '/home' is not persisted

PHP Version

PHP 8.2

Operating System

Debian 11.6

[nielsdos]

Could be fixed by #10440.
EDIT: nevermind I misunderstood the issue, sorry (when I looked at the length of the strace I immediately thought about the corruption case but this one is different, I should've looked more carefully).

[nielsdos]

Your methodology to fix this issue sounds correct. You can probably use php_stream_state instead of fstat to support all kinds of streams of PHP (and if that call fails by returning -1, then you can skip the file_copy_range I guess). If you have a patch feel free to PR it :)

[knowsshit]

I would love to write a patch, but I am not sure if I am able to do it properly. The main problem here is that PHP uses a length argument with copy_file_range() that is larger than the source file size.

...so as long as PHP can ensure to reduce the length to the source size it would be good, and I guess combining it with looping through copy_file_range() until every byte is copied as proposed in #10440 would be the correct solution here.

Can anyone help out to make this happen?

[nielsdos]

I can put it on my todo list for this week, unless someone beats me to it

[knowsshit]

I can put it on my todo list for this week, unless someone beats me to it

Fantastic!