sahlberg / libsmb2

SMB2/3 userspace client
Other
325 stars 136 forks source link

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

Closed maoabc closed 1 year ago

maoabc commented 1 year ago

../../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 commented 1 year ago

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 commented 1 year ago

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

hholtmann commented 1 year ago

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.