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

There will be errors when uploading some small files, and the service is samba #240

Closed
maoabc opened this issue Dec 2, 2022 · 3 comments

Comments

@maoabc
Copy link

maoabc commented Dec 2, 2022

../../source3/smbd/server.c:1925: [2022/12/01 11:33:57.880642, all 0] main
smbd version 4.10.18 started.
Copyright Andrew Tridgell and the Samba Team 1992-2019
../../source3/rpc_server/rpc_modules.c:52: [2022/12/01 11:33:58.053470, all 0, pid=13781] register_rpc_module
register_rpc_module: RPC module mdssvc already loaded!
../../source3/smbd/vfs.c:759: [2022/12/02 23:46:46.372977, vfs 0, pid=919] vfs_pwrite_data
PANIC: assert failed at ../../source3/smbd/vfs.c(759): req->unread_bytes == N
../../source3/lib/util.c:827: [2022/12/02 23:46:46.373587, all 0, pid=919] smb_panic_s3
PANIC (pid 919): assert failed: req->unread_bytes == N
../../lib/util/fault.c:265: [2022/12/02 23:46:46.394891, all 0, pid=919] log_stack_trace
BACKTRACE: 26 stack frames:
#0 /var/packages/SMBService/target/usr/lib/libsamba-util.so.0(log_stack_trace+0x30) [0x7f25ccd18e00]
#1 /var/packages/SMBService/target/usr/lib/libsmbconf.so.0(smb_panic_s3+0x18) [0x7f25cb389c28]
#2 /var/packages/SMBService/target/usr/lib/libsamba-util.so.0(smb_panic+0x2d) [0x7f25ccd18efd]
#3 /var/packages/SMBService/target/usr/lib/samba/libsmbd-base-samba4.so(vfs_pwrite_data+0x1ae) [0x7f25cc8d7dee]
#4 /var/packages/SMBService/target/usr/lib/samba/libsmbd-base-samba4.so(+0xe2a85) [0x7f25cc887a85]
#5 /var/packages/SMBService/target/usr/lib/samba/libsmbd-base-samba4.so(write_file+0x441) [0x7f25cc8884e1]
#6 /var/packages/SMBService/target/usr/lib/samba/libsmbd-base-samba4.so(smbd_smb2_request_process_write+0x44f) [0x7f25cc9042af]
#7 /var/packages/SMBService/target/usr/lib/samba/libsmbd-base-samba4.so(smbd_smb2_request_dispatch+0xca3) [0x7f25cc8f85f3]
#8 /var/packages/SMBService/target/usr/lib/samba/libsmbd-base-samba4.so(+0x15425b) [0x7f25cc8f925b]

@sahlberg
Copy link
Owner

sahlberg commented Dec 2, 2022

This looks like a bug in samba.
No matter what libsmb2 does it should not be able to cause samba to panic.
Please open a bug with samba,

@maoabc
Copy link
Author

maoabc commented Dec 4, 2022

Thanks, I tested samba 4.4.18 and found no problem, not sure if the latest version fixes this bug

@maoabc maoabc closed this as completed Dec 4, 2022
@hholtmann
Copy link
Contributor

We ran into this bug when testing against some older Samba servers (e.g. Version 4.3.11 and Version 4.4.16). These versions are the latest ones on some WD NAS and QNAP NAS devices, which are not updated anymore (but still in use by many people). The crash log of samba always contains req->unread_bytes == N, which points to more or less bytes written than being expected.
Comparing the Wireshark trace of libsmb2 and smbclient showed one major difference.
Assuming we have a file of size of 18735 smbclient sends exactly 18735 bytes as data to the server, while libsmb2 sends 18736 bytes to the server (though specifying 18735 as write length in the write request). This is caused by libsmb2 performing smb2_pad_to_64bit on the data packet and adding one extra byte at the very end in the example case. This causes the older Samba servers to crash.

For testing I changed the function smb2_cmd_write_async as following:
Call smb2_pad_to_64bit before smb2_add_iovector for the data packet is called. This prevents smb2_pad_to_64bit to be performed on the data packet. Another option would be to completely remove smb2_pad_to_64bit being called in smb2_cmd_write_async , but I am not sure if the padding for the other 2 vectors (SMB header, SMB write command) is done beforehand.
After this change the samba server does not crash anymore

I am not in expert in SMB internals, but as smbclient (of the samba project) and also the macOS do not perform any padding on the data packet, this seem to be a change that should be taken into consideration.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants