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

A file larger than 4 GB is copied to an interface CopyFileRange. The chunk index is incorrectly calculated, resulting in inconsistent data replication #4122

Closed
fengleng opened this issue Oct 26, 2023 · 7 comments
Labels
kind/bug Something isn't working

Comments

@fengleng
Copy link
Contributor

What happened: Copy files larger than 4G

What you expected to happen:The data of the copied file is the same as that of the source file

How to reproduce it (as minimally and precisely as possible): Deploy juicefs on ubuntu20.04LTS, set the mount point to /mnt/juicefs, share /mnt/juicefs with samba, add a mapped network drive on windows11, and generate a 6G file file1. On windows11, add file1 to /mnt/juicefs/file1, and then copy file1 in the /mnt/juicefs/ directory to generate a file with 'file1 - copy'. The data of file-copy and file1 are inconsistent

Anything else we need to know?
Code location:https://github.com/juicedata/juicefs/blob/86e69e48d06ac1b59c56ebda66de5df0727d7f4d/pkg/meta/tkv.go#L2026C7-L2026C7

Environment:
OS of the storage node: ubuntu20.04LTS
samba client::windows11
Any version of jiucefs

@fengleng fengleng added the kind/bug Something isn't working label Oct 26, 2023
@fengleng
Copy link
Contributor Author

kvMeta.CopyFileRange The chunk index of the source file is incorrectly calculated, causing a chunk slice error

https://github.com/juicedata/juicefs/blob/86e69e48d06ac1b59c56ebda66de5df0727d7f4d/pkg/meta/tkv.go#L2026C7-L2026C7

Correct code
tx.scan(m.chunkKey(fin, uint32(offIn/ChunkSize)), m.chunkKey(fin, uint32((offIn+size)/ChunkSize)+1),
false, func(k, v []byte) bool {
vals[string(k)] = v
return true
})

@SandyXSD
Copy link
Contributor

Yes, you're right. Could you submit a PR for this fix?

@SandyXSD
Copy link
Contributor

However, the original issue may not be related to this code snippet, because the loop here uses correct index. Since offin >= offin/ChunkSize, the wrong index may lead to over scan, but will not affect the final result.

@SandyXSD
Copy link
Contributor

SandyXSD commented Oct 26, 2023

The syscall function is defined as:

ssize_t copy_file_range(int fd_in, off64_t *_Nullable off_in,
                               int fd_out, off64_t *_Nullable off_out,
                               size_t len, unsigned int flags)

Note the type of len here is size_t, which may be a uint32 on some platform and thus overflowed by value >= 4G.
To confirm could you gather the access log when you are reproducing this issue?

@fengleng
Copy link
Contributor Author

vals := make(map[string][]byte) tx.scan(m.chunkKey(fin, uint32(offIn/ChunkSize)), m.chunkKey(fin, uint32(offIn+size/ChunkSize)+1), false, func(k, v []byte) bool { vals[string(k)] = v return true })

here, samba gives len 1M and offin is 4G. Therefore, uint32(offIn+size/ChunkSize) will be equal to 0, because the maximum uint32 is equal to 4g-1. Then, when the offset of a file is 4G, chunk metadata with 4G offset cannot be obtained

@fengleng
Copy link
Contributor Author

Yes, you're right. Could you submit a PR for this fix?

my great pleasure

fengleng added a commit to fengleng/juicefs that referenced this issue Oct 26, 2023
@SandyXSD
Copy link
Contributor

fixed by #4130

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants