Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fs.copyFile fails with permission denied when source is read-only #37284

Closed
kakaroto opened this issue Feb 9, 2021 · 10 comments
Closed

fs.copyFile fails with permission denied when source is read-only #37284

kakaroto opened this issue Feb 9, 2021 · 10 comments
Labels
fs Issues and PRs related to the fs subsystem / file system.

Comments

@kakaroto
Copy link

kakaroto commented Feb 9, 2021

What steps will reproduce the bug?

if I have an external mount (in my case a cephfs cluster mounted using ceph-fuse), then copying a file into it will fail with permission denied if the source file is read-only.

Simple code to reproduce the issue :

const fs = require("fs");

fs.copyFileSync("/tmp/foo", "/ceph-mount/tmp/bar");

Doing a touch /tmp/foo && chmod -w /tmp/foo before running the script will allow you to reproduce the error. The above code will fail as long as the source file is read-only, even if the file can be copied in the shell.

(note, it might work with an NFS mount or some other type of mount, but have only tested with my cephfs mount. A local ext4fs mount does not seem to be affected by the problem).

Also note that this works with Node 12.18.4. I actually noticed this issue a little while ago when I reinstalled one of my systems, and ended up with node 12.19.0 and my app started failing to copy files from a read-only file system and I was forced to downgrade to 12.18.3 where it was known to be working as expected. Today, I confirmed the bug is still there in node 14.15.4.

Using this command docker run --rm -it -v /ceph-mount/:/ceph-mount node:12.19.0-alpine /bin/sh to run node then pasting these commands in it will show the problem :

su - node
touch /tmp/foo
chmod -w /tmp/foo
cat <<EOF > test.js
const fs = require("fs");
fs.copyFileSync("/tmp/foo", "/ceph-mount/tmp/bar");
EOF
rm -f /ceph-mount/tmp/bar
node test.js
exit
exit

How often does it reproduce? Is there a required condition?

100% reproducible on node 12.19.0 up to 14.15.4, 0% reproducible on node 12.18.4

What is the expected behavior?

the fs.copyFile should be able to succeed in copying the file as long as the destination folder is writable.

What do you see instead?

The output is :

internal/fs/utils.js:269
    throw err;
    ^

Error: EACCES: permission denied, copyfile '/tmp/foo' -> '/ceph-mount/tmp/bar'
    at Object.copyFileSync (fs.js:1904:3)
    at Object.<anonymous> (/home/node/test.js:2:4)
    at Module._compile (internal/modules/cjs/loader.js:1015:30)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:1035:10)
    at Module.load (internal/modules/cjs/loader.js:879:32)
    at Function.Module._load (internal/modules/cjs/loader.js:724:14)
    at Function.executeUserEntryPoint [as runMain] (internal/modules/run_main.js:60:12)
    at internal/main/run_main_module.js:17:47 {
  errno: -13,
  syscall: 'copyfile',
  code: 'EACCES',
  path: '/tmp/foo',
  dest: '/ceph-mount/tmp/bar'
}

Here are screenshots showing the above running on node 12.19.0 as well as 12.18.4 to prove that the bug was introduced in the 12.19.x branch :
image

And a screenshot showing it with node 14.15.4 proving that a cp in the shell works as well (this is on another system, the /forge-vtt-dev/ folder is the cephfs mount):
image

This also shows a second bug where it's unable to overwrite the file if it already exists, and I'm about to file another issue for it.

@PoojaDurgad PoojaDurgad added the fs Issues and PRs related to the fs subsystem / file system. label Feb 9, 2021
@RaisinTen
Copy link
Contributor

It seems to be working for v15.2.0:

❯ cat test.js
const fs = require('fs');

fs.copyFileSync('a/file', 'b/file');
❯ mkdir a b
❯ touch a/file
❯ ls -l a/file
-rw-r--r-- 1 raisinten raisinten 0 Feb  9 19:35 a/file
❯ chmod -w a/file
❯ node test.js
❯ ls -l a/file b/file
-r--r--r-- 1 raisinten raisinten 0 Feb  9 19:35 a/file
-r--r--r-- 1 raisinten raisinten 0 Feb  9 19:35 b/file

@kakaroto
Copy link
Author

kakaroto commented Feb 9, 2021

It seems to be working for v15.2.0:

I just tried it on 15.2.0 and I still get the same error.
In the output you posted, I can see you're trying to copy from folder a to folder b, but you seem to have missed in the original report the requirement on the fact that it's only reproducible when the destination folder is on a remote mount (in my case cephfs).

I understand that it unfortunately makes this much harder to reproduce on a test environment.
I've just tried it now with an NFS mount (which should be easier to setup than cephfs) and I get the same error though it's a slightly different behavior. I get the permission error on both 12.18.4 and 15.2.0, but also in both cases, a file actually gets created, but remains empty (I changed the touch /tmp/foo into an echo foo > /tmp/foo to verify this).

EDIT: My bad, it does work on 12.18.4 but fails with 15.2.0 with an NFS mount. A file does get created despite the thrown exception, but it has no data in it (on a cephfs mount, no file is created).

@RaisinTen
Copy link
Contributor

I believe this should be reported to libuv too because internally, this calls
CopyFile which in turn, calls uv_fs_copyfile:

node/src/node_file.cc

Lines 1802 to 1803 in ad3ebed

SyncCall(env, args[4], &req_wrap_sync, "copyfile",
uv_fs_copyfile, *src, *dest, flags);

EDIT: My bad, it does work on 12.18.4 but fails with 15.2.0 with an NFS mount. A file does get created despite the thrown exception, but it has no data in it (on a cephfs mount, no file is created).

According to the warning in the docs for the function, the destination path
should be removed instead of being just an empty file when there is an error,
so that looks unexpected too.

cc @nodejs/fs

@kakaroto
Copy link
Author

I believe this should be reported to libuv too because internally, this calls CopyFile which in turn, calls uv_fs_copyfile

That's what I originally thought as I dug a bit into the internals, but I'm not sure then why the behavior changed between 12.18.4 and 12.19.0, unless libuv is statically linked to nodejs.
If it is, then you'd be more likely to know what changed and trace back the cause in order to send a proper report to libuv. (I just checked and the 12.19.0 changelog mentions upgrading libuv to 1.38.1 (#34187) and 1.39.0 (#34915)).
Thanks.

@RaisinTen
Copy link
Contributor

RaisinTen commented Feb 13, 2021

I think, I understand what's going on. libuv/libuv#2352 introduced
the usage of copy_file_range for uv_fs_copyfile when possible.
However, as the man page for copy_file_range mentions, it can
throw this error for pre Linux 5.3:

EXDEV
The files referred to by fd_in and fd_out are not on the same mounted filesystem (pre Linux 5.3).

This particular check was ignored.

RaisinTen added a commit to RaisinTen/libuv that referenced this issue Feb 13, 2021
copy_file_range will not work when in_fd and out_fd
are not on the same mounted filesystem (pre Linux 5.3).

Fixes: nodejs/node#37284
Refs: https://www.man7.org/linux/man-pages/man2/copy_file_range.2.html#ERRORS
@RaisinTen
Copy link
Contributor

Potential fix: libuv/libuv#3108

cjihrig pushed a commit to libuv/libuv that referenced this issue Feb 13, 2021
copy_file_range will not work when in_fd and out_fd
are not on the same mounted filesystem (pre Linux 5.3).

Refs: nodejs/node#37284
PR-URL: #3108
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Richard Lau <rlau@redhat.com>
@RaisinTen
Copy link
Contributor

@kakaroto does this issue still exist in v15.9.0?

@kakaroto
Copy link
Author

Sorry for my lack of availability. Managing a startup is exhausting!

I'm not sure that this would be the cause, since EXDEV seems to be about "a different mount" but not be about whether or not the source file is read-only or not. The error doesn't happen if the source file is read-write, only if it's read-only, so I think EXDEV is unrelated. The error shown is also with errno: -13, code: EACCESS, so it's more of a permission problem.

Regardless, thank you for looking into that and providing that EXDEV fix so quickly. Unfortunately, I just tested with v15.9.0 and the issue is still happening.

@RaisinTen
Copy link
Contributor

@kakaroto I have reported your issue at libuv: libuv/libuv#3117

@kakaroto
Copy link
Author

Thank you. I appreciate that.

JeffroMF pushed a commit to JeffroMF/libuv that referenced this issue May 16, 2022
copy_file_range will not work when in_fd and out_fd
are not on the same mounted filesystem (pre Linux 5.3).

Refs: nodejs/node#37284
PR-URL: libuv#3108
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Richard Lau <rlau@redhat.com>
Varstahl added a commit to Varstahl/libuv that referenced this issue Mar 10, 2023
Trying to copy a read-only file onto a ceph-fuse filesystem fails,
returning an `EACCES` error. This happens when the destination
doesn't exist yet, and a new file is created.

By checking that the error matches, and that the destination file
is empty, we can fix this issue while waiting for a proper Ceph
fix to be upstreamed.

Fixes: libuv#3919
Refs: nodejs/node#37284
Refs: libuv#3117
Refs: libuv#3322
bnoordhuis pushed a commit to libuv/libuv that referenced this issue Mar 12, 2023
Trying to copy a read-only file onto a ceph-fuse filesystem fails,
returning an `EACCES` error. This happens when the destination
doesn't exist yet, and a new file is created.

By checking that the error matches, and that the destination file
is empty, we can fix this issue while waiting for a proper Ceph
fix to be upstreamed.

Fixes: #3919
Refs: nodejs/node#37284
Refs: #3117
Refs: #3322
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fs Issues and PRs related to the fs subsystem / file system.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants