SLES's cifsd and Apple's smbx do not correctly handle FILE_NON_DIRECTORY_FILE which prevents recursive copies in gvfs from working correctly [1] since GVFS tries to open the directory, expecting ENOTDIR, but it suceeds and appears as a zero byte file. This can be trivially reproduced by executing "get some_dir" in smbclient against one of the broken servers. Instead of getting NT_STATUS_FILE_IS_A_DIRECTORY, you get a zero byte file. Would it be possible for a workaround to be put in libsmbclient if these buggy servers are detected? I could implement a racy workaround in gvfs by stat()ing before every open but AFAIK libsmbclient doesn't expose the type of the server so this would have to be done for every open against every type of server... Thanks! [1] https://bugzilla.gnome.org/show_bug.cgi?id=599354
Can you get me a trace on this ? The workaround would be to detect the open coming back with an attribute FILE_DIRECTORY, and then having the libsmbclient close the returned handle and return -1,ENOTDIR in that that. But I really need to see the wire traces first. Thanks, Jeremy.
Created attachment 9903 [details] Trace when running smbclient Attached is the trace running the following commands against a SLES 11 SP2 cifsd server: """ [ross@twerp ~]$ smbclient -U ubuntu //192.168.0.1/myvol Enter ubuntu's password: Domain=[WORKGROUP] OS=[SUSE LINUX 10.1] Server=[SUSE LINUX 10.1] smb: \> cd ubuntu smb: \ubuntu\> ls . D 0 Sun Apr 13 13:55:37 2014 .. D 0 Sun Apr 13 13:49:52 2014 poo DA 0 Sun Apr 13 13:55:35 2014 test DA 0 Sun Apr 13 13:53:20 2014 48310 blocks of size 4096. 48167 blocks available smb: \ubuntu\> get test getting file \ubuntu\test of size 0 as test (0.0 KiloBytes/sec) (average 0.0 KiloBytes/sec) smb: \ubuntu\> exit """ The trace was captured with: sudo tcpdump -i ens3 -s 0 -p -n -w smbclient.trace Thanks for looking into this.
Created attachment 9904 [details] Test patch for 4.1.x Can you test this fix to see if it addresses the problem ? Thanks, Jeremy.
Oh yeah, can you also get me a trace against Apple's server too, as I thought that server only supported SMB2, not SMB1 - so I need to know if I need to put the same fix into the SMB2 client codepath as well as the SMB1 client codepath. Jeremy.
I tested the patch and it worked successfully against cifsd and smbx. Cool! Nevertheless, I captured a trace when running unpatched samba against smbx: """ [liveuser@localhost ~]$ smbclient -U john //192.168.0.1/john Enter john's password: Domain=[JOHNS-MAC-PRO] OS=[Darwin] Server=[@(#)PROGRAM:smbd PROJECT:smbx-136.18] smb: \> ls . DA 0 Thu Apr 3 18:35:27 2014 .. DA 0 Thu Apr 3 18:35:27 2014 .bash_history AH 860 Sun Apr 13 13:25:19 2014 .CFUserTextEncoding AH 3 Thu Apr 3 17:39:15 2014 .DS_Store AH 15364 Tue May 6 03:42:53 2014 .Trash DAH 0 Fri Apr 4 16:15:20 2014 Desktop DA 0 Fri Apr 4 16:22:33 2014 Documents DA 0 Fri Apr 4 16:42:43 2014 Downloads DA 0 Tue May 6 03:41:59 2014 Library DAH 0 Thu Apr 3 18:30:21 2014 Movies DA 0 Thu Apr 3 17:57:58 2014 Music DA 0 Thu Apr 3 17:39:15 2014 Pictures DA 0 Thu Apr 3 17:39:15 2014 Public DA 0 Thu Apr 3 17:39:15 2014 60783 blocks of size 524288. 29155 blocks available smb: \> get Library getting file \Library of size 0 as Library (0.0 KiloBytes/sec) (average 0.0 KiloBytes/sec) smb: \> exit [liveuser@localhost ~]$ stat Library File: ‘Library’ Size: 0 Blocks: 0 IO Block: 4096 regular empty file Device: fd00h/64768d Inode: 45008 Links: 1 Access: (0644/-rw-r--r--) Uid: ( 1000/liveuser) Gid: ( 1000/liveuser) Context: unconfined_u:object_r:user_home_t:s0 Access: 2014-05-06 03:51:02.772665541 -0400 Modify: 2014-05-06 03:51:02.772665541 -0400 Change: 2014-05-06 03:51:02.772665541 -0400 Birth: - """ Captured with: sudo tcpdump -i p2p1 -s 0 -p -n -w smbclient-apple.trace
Created attachment 9906 [details] Trace when running smbclient against smbx
Oh interesting, I didn't think the Apple server supported SMB1. Oh well, I'm guessing that their SMB2 server does the right thing. I'll see if I can get these fixes into master. Jeremy.
Actually, can you check if their SMB2 server does the right thing by connecting to it using smbclient -mSMB3 (or -mSMB2 if it doesn't support SMB3) and seeing if you can open a directory in the same way ? Thanks, Jeremy.
I tested with -mSMB2 and -mSMB3 and both gave the same result as not specifying the protocol, i.e. successfully retrieving the directory as a file. I have attached traces for each of those attempts. I then tried with the patch and -mSMB2 and -mSMB3 and and it returned NT_STATUS_FILE_IS_A_DIRECTORY for both.
Created attachment 9908 [details] Trace when using smb2
Created attachment 9909 [details] Trace when using smb3
Ah, I took a look - the server is still selecting SMB1, that's why it's still working (using the patch path). Try using: -mSMB2 and the add "client min protocol = SMB2" in your smb.conf to force the client to only offer SMB2. See if the server accepts..
No, the server did not accept it, nor SMB3. Note that this was testing against OS X 10.8.2, I don't know if anything would have changed with other versions. The wire trace is attached. """ [liveuser@localhost ~]$ smbclient -mSMB2 -U john //192.168.1.72/john Enter john's password: protocol negotiation failed: NT_STATUS_IO_TIMEOUT """ For completeness, I tried with smbclient4 and SMB2 (what's the difference?). This allowed connecting but didn't work to get files or directories. Wire trace attached. """ [liveuser@localhost ~]$ smbclient4 -mSMB2 -U john //192.168.1.72/john Password for [MYGROUP\john]: smb: \> ls . DA 0 Wed May 7 03:39:03 2014 .. DA 0 Wed May 7 03:39:03 2014 .bash_history AH 900 Wed May 7 03:39:11 2014 .CFUserTextEncoding AH 3 Thu Apr 3 17:39:15 2014 .DS_Store AH 15364 Tue May 6 03:42:53 2014 .Trash DAH 0 Fri Apr 4 16:15:20 2014 .viminfo AH 710 Wed May 7 03:39:03 2014 Desktop DA 0 Fri Apr 4 16:22:33 2014 Documents DA 0 Fri Apr 4 16:42:43 2014 Downloads DA 0 Tue May 6 03:41:59 2014 Library DAH 0 Thu Apr 3 18:30:21 2014 Movies DA 0 Thu Apr 3 17:57:58 2014 Music DA 0 Thu Apr 3 17:39:15 2014 Pictures DA 0 Thu Apr 3 17:39:15 2014 Public DA 0 Thu Apr 3 17:39:15 2014 7780342 blocks of size 4096. 3726627 blocks available smb: \> get Library NT_STATUS_NOT_SUPPORTED opening remote file \Library smb: \> cd Desktop smb: \Desktop\> ls . DA 0 Fri Apr 4 16:22:33 2014 .. DA 0 Wed May 7 03:39:03 2014 .DS_Store AH 12292 Wed May 7 03:40:06 2014 .localized AH 0 Thu Apr 3 17:39:15 2014 d1 DA 0 Fri Apr 4 16:07:32 2014 dude DA 0 Fri Apr 4 16:42:11 2014 TotalFinder-1-1.5.22ddd copy.dmg A 10061824 Thu Apr 3 18:39:33 2014 TotalFinder-1-1.5.22ddd.dmg A 10061824 Thu Apr 3 18:39:33 2014 7780342 blocks of size 4096. 3726626 blocks available smb: \Desktop\> get TotalFinder-1-1.5.22ddd.dmg NT_STATUS_NOT_SUPPORTED opening remote file \Desktop\TotalFinder-1-1.5.22ddd.dmg smb: \Desktop\> exit """
Created attachment 9914 [details] Trace when using smb2 (forced)
Created attachment 9915 [details] Trace when using smbclient4 and smb2 (forced)
Created attachment 9928 [details] git-am fix for master After some push back from metze about the right layer to fix this, here is an updated patch that should work identically to the original, but moves the fix up one later to cli_open(). As cli_open() has no asynchronous variants, we can treat it as a special case. This version leaves the behavior of the synchronous and asynchronous versions of cli_ntcreate() and cli_nttrans_create() unmodified. If you could test this out please I'd appreciate it ! Thanks, Jeremy.
(In reply to comment #16) > Created attachment 9928 [details] > git-am fix for master > > After some push back from metze about the right layer to fix this, here is an > updated patch that should work identically to the original, but moves the fix > up one later to cli_open(). Oh yeah, in addition it also fixes the bug if it happens with any SMB2 servers too :-).
Created attachment 9929 [details] git-am fix for master. All right Metze, here's the goddam rewrite you insisted upon :-). Add a return parameter of struct smb_create_returns *cr to cli_ntcreate() cli_ntcreate_recv() cli_nttrans_create() cli_nttrans_create_recv() and fixes everything up... It's now a 4-patch set.
(In reply to comment #18) > Created attachment 9929 [details] > git-am fix for master. > > All right Metze, here's the goddam rewrite > you insisted upon :-). > > Add a return parameter of struct smb_create_returns *cr to > cli_ntcreate() > cli_ntcreate_recv() > cli_nttrans_create() > cli_nttrans_create_recv() > > and fixes everything up... It's now a 4-patch set. I applied this to 4.1 and tested against SLES's cifsd and Apple's smbx and it worked successfully. Thanks!
Created attachment 9932 [details] Patches for v4-1-test
Jeremy, 4.0 seems to need different patches.
@Ross: When you talk about SLES cifsd the actual product in use is a Novell Open Enterprise Server powered by SUSE Linux Enterprise Server?
(In reply to comment #22) > @Ross: When you talk about SLES cifsd the actual product in use is a Novell > Open Enterprise Server powered by SUSE Linux Enterprise Server? Yes. Confusingly, SLES provides Samba and SLES+OES provides a custom server which shows up as cifsd.
Comment on attachment 9932 [details] Patches for v4-1-test LGTM. Karolin please push to 4.1`.next, and I'll work on finishing up a 4.0.next patchset.
Karolin, reassign back to me once the 4.1.next patch has gone in and I'll back port to 4.0.next.
(In reply to comment #25) > Karolin, reassign back to me once the 4.1.next patch has gone in and I'll back > port to 4.0.next. Pushed to autobuild-4-1-test.
(In reply to comment #26) > (In reply to comment #25) > > Karolin, reassign back to me once the 4.1.next patch has gone in and I'll back > > port to 4.0.next. > > Pushed to autobuild-4-1-test. Patchset breaks the build: [2258/4124] Compiling source3/passdb/pdb_tdb.c ../source3/libsmb/cli_np_tstream.c: In function 'tstream_cli_np_open_done': ../source3/libsmb/cli_np_tstream.c:207: error: too few arguments to function 'cli_ntcreate_recv' [2259/4124] Compiling source3/lib/server_mutex.c ../source3/libads/kerberos.c: In function 'smb_krb5_get_ntstatus_from_krb5_error_init_creds_opt': ../source3/libads/kerberos.c:126: warning: 'krb5_get_init_creds_opt_get_error' is deprecated (declared at default/source4/heimdal/lib/krb5/krb5-protos.h:2004) Waf: Leaving directory `/memdisk/kseeger/a41/b932397/samba/bin' Build failed: -> task failed (err #1): {task: cc cli_np_tstream.c -> cli_np_tstream_65.o} make: *** [all] Error 1 Re-assigning to Jeremy.
Created attachment 9992 [details] Fixed patch for 4.1.next Fixed the patch (was missing one extra NULL argument to cli_ntcreate_recv() in source3/libsmb/cli_np_tstream.c). Sorry for the breakage. Thanks, Jeremy.
Re-assigning to Karolin for inclusion in 4.1.next. Thanks Volker ! Jeremy.
(In reply to comment #29) > Re-assigning to Karolin for inclusion in 4.1.next. > > Thanks Volker ! > > Jeremy. Pushed to autobuild-v4-1-test.
(In reply to comment #30) > (In reply to comment #29) > > Re-assigning to Karolin for inclusion in 4.1.next. > > > > Thanks Volker ! > > > > Jeremy. > > Pushed to autobuild-v4-1-test. Pushed to v4-1-test. Closing out bug report. Thanks!