Bug 10260 - smbclient shows no error if deleting a directory with del failed
Summary: smbclient shows no error if deleting a directory with del failed
Status: RESOLVED FIXED
Alias: None
Product: Samba 4.1 and newer
Classification: Unclassified
Component: libsmbclient (show other bugs)
Version: 4.1.0
Hardware: All All
: P5 regression (vote)
Target Milestone: ---
Assignee: Karolin Seeger
QA Contact: Samba QA Contact
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-11-10 22:02 UTC by Thomas Bork
Modified: 2013-11-26 19:28 UTC (History)
0 users

See Also:


Attachments
git-am fix for master/4.1.next (10.35 KB, patch)
2013-11-12 23:58 UTC, Jeremy Allison
no flags Details
git-am fix applies cleanly to master/v4-1-test (10.35 KB, patch)
2013-11-13 18:10 UTC, Jeremy Allison
no flags Details
git-am fix applies cleanly to master/v4-1-test (10.39 KB, patch)
2013-11-13 21:09 UTC, Jeremy Allison
no flags Details
git-am fix for 4.1.next. (10.98 KB, patch)
2013-11-14 22:52 UTC, Jeremy Allison
asn: review+
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Thomas Bork 2013-11-10 22:02:37 UTC
If I try to delete a file 'x' with 'del x' or 'rm x', which does not exist in the share, smbclient shows following error:

NT_STATUS_NO_SUCH_FILE listing \x

If I try to delete a directory 'A' with 'del A' or 'rm A', which does exist in the share, smbclient shows no error message but does not delete 'A' with 'del A', because it is a directory, which only can deleted with 'rmdir A'.

test # smbclient -V
Version 4.1.0-for-eisfair-1-patch-1
test # smbclient //test/tb -Utb
Enter tb's password:
Domain=[TEST] OS=[] Server=[]
smb: \> dir
  .                                   D        0  Wed Aug 14 11:19:44 2013
  ..                                  D        0  Fri Aug  9 02:34:52 2013
  A                                   D        0  Sun Nov 10 22:28:36 2013

                3939864 blocks of size 1024. 2642556 blocks available
smb: \> del x
NT_STATUS_NO_SUCH_FILE listing \x
smb: \> del A
smb: \> dir
  .                                   D        0  Wed Aug 14 11:19:44 2013
  ..                                  D        0  Fri Aug  9 02:34:52 2013
  A                                   D        0  Sun Nov 10 22:28:36 2013

                3939864 blocks of size 1024. 2642556 blocks available
smb: \>

This is a regression to Samba 3.6.x, which shows the correct error message:

test # smbclient -V
Version 3.6.19-for-eisfair-1-patch-1
test # smbclient //test/tb -Utb
Enter tb's password:
Domain=[TOMMAIK] OS=[Unix] Server=[Samba 3.6.19-for-eisfair-1-patch-1]
smb: \> dir
  .                                   D        0  Sun Nov 10 22:45:30 2013
  ..                                  D        0  Fri Aug  9 02:34:52 2013
  A                                   D        0  Sun Nov 10 22:28:36 2013

                61560 blocks of size 65536. 40490 blocks available
smb: \> del x
NT_STATUS_NO_SUCH_FILE listing \x
smb: \> del A
NT_STATUS_NO_SUCH_FILE listing \A
smb: \> dir
  .                                   D        0  Sun Nov 10 22:45:30 2013
  ..                                  D        0  Fri Aug  9 02:34:52 2013
  A                                   D        0  Sun Nov 10 22:28:36 2013

                61560 blocks of size 65536. 40491 blocks available
smb: \>

Please note, that there is some software, which relies on that error message as smbwebclient, which tries to delete data first as file (with del) and if the status is NO_SUCH_FILE then tries to delete data as directory (with rmdir).

As a side note:
smbclient from Samba 3.6.19 shows:
Domain=[TOMMAIK] OS=[Unix] Server=[Samba 3.6.19-for-eisfair-1-patch-1]

smbclient from Samba 4.1.0 shows:
Domain=[TEST] OS=[] Server=[]
Comment 1 Jeremy Allison 2013-11-12 21:52:01 UTC
Doesn't happen in master. I'm checking 4.1.x and 4.0.x now.

Jeremy.
Comment 2 Jeremy Allison 2013-11-12 22:06:50 UTC
Ah, ok. Fails using SMB2 - but not SMB1. I'll investigate.
Comment 3 Jeremy Allison 2013-11-12 23:58:48 UTC
Created attachment 9411 [details]
git-am fix for master/4.1.next

Thomas can you test this fix ? It works fine for me in master/4.1.next. Once you've confirmed it's good I'll get it into master and 4.1.next.

Thanks !

Jeremy.
Comment 4 Thomas Bork 2013-11-13 17:29:15 UTC
Am 13.11.2013 00:58, schrieb samba-bugs@samba.org:

> Thomas can you test this fix ? It works fine for me in master/4.1.next. Once
> you've confirmed it's good I'll get it into master and 4.1.next.

Cannot apply the patch to 4.1.1:

referencetest26 2.2.4 # patch --dry-run -p1 
<bug-10260-master_smbclient_del_dir.patch
checking file source3/smbd/dir.c
checking file source3/smbd/proto.h
checking file source3/smbd/reply.c
checking file source3/include/proto.h
Hunk #1 succeeded at 480 (offset 5 lines).
checking file source3/lib/util.c
checking file source3/smbd/dir.c
Reversed (or previously applied) patch detected!  Assume -R? [n]
Apply anyway? [n]
Skipping patch.
1 out of 1 hunk ignored
checking file source3/smbd/proto.h
Hunk #1 FAILED at 218.
1 out of 1 hunk FAILED
checking file source3/libsmb/cli_smb2_fnum.c
checking file source3/libsmb/cli_smb2_fnum.h
checking file source3/libsmb/clilist.c

Seems I really have to download 4.1-test. This is difficult to me in the 
moment (no git on this machine).

Thanks for your help!
Comment 5 Thomas Bork 2013-11-13 17:49:18 UTC
Am 13.11.2013 18:29, schrieb samba-bugs@samba.org:

> Seems I really have to download 4.1-test. This is difficult to me in the
> moment (no git on this machine).

Using a snapshot from http://repo.or.cz/w/Samba.git/tree/v4-1-test the 
result is the same:

referencetest26 2.2.4 # patch --dry-run -p1 
<bug-10260-master_smbclient_del_dir.patch
checking file source3/smbd/dir.c
checking file source3/smbd/proto.h
checking file source3/smbd/reply.c
checking file source3/include/proto.h
Hunk #1 succeeded at 480 (offset 5 lines).
checking file source3/lib/util.c
checking file source3/smbd/dir.c
Reversed (or previously applied) patch detected!  Assume -R? [n]
Apply anyway? [n]
Skipping patch.
1 out of 1 hunk ignored
checking file source3/smbd/proto.h
Hunk #1 FAILED at 218.
1 out of 1 hunk FAILED
checking file source3/libsmb/cli_smb2_fnum.c
checking file source3/libsmb/cli_smb2_fnum.h
checking file source3/libsmb/clilist.c
Comment 6 Jeremy Allison 2013-11-13 18:01:59 UTC
I'll take a look and see what I messed up asap. Thanks for testing !
Jeremy.
Comment 7 Jeremy Allison 2013-11-13 18:10:52 UTC
Created attachment 9417 [details]
git-am fix applies cleanly to master/v4-1-test

Ok, here is the patch I have in master.

For me it applies cleanly to both master and v4-1-test.

My v4-1-test tree is at git rev: 2cfa1ef6a9e2728c9cdd185d6d08a452e0485200 as top of tree, and the patch I posted applies cleanly for me.

Hope this helps you reproduce the patch application..

Jeremy.

Current v4-1-test top-of-tree.
-----------------------------------------------------

commit 2cfa1ef6a9e2728c9cdd185d6d08a452e0485200
Author: Björn Jacke <bj@sernet.de>
Date:   Wed Nov 6 12:37:07 2013 +0100

    xattr: fix listing EAs on *BSD for non-root users
    
    Thanks to Stefan Rompf for reporting.
    
    This fixes bug #10247
    
    Signed-off-by: Bjoern Jacke <bj@sernet.de>
    Reviewed-by: Jeremy Allison <jra@samba.org>
    
    Autobuild-User(master): Jeremy Allison <jra@samba.org>
    Autobuild-Date(master): Fri Nov  8 20:43:30 CET 2013 on sn-devel-104
    (cherry picked from commit 374b2cfde74e0c61f4b2da724b30d0e430596092)
    
    Autobuild-User(v4-1-test): Karolin Seeger <kseeger@samba.org>
    Autobuild-Date(v4-1-test): Tue Nov 12 13:28:26 CET 2013 on sn-devel-104
Comment 8 Thomas Bork 2013-11-13 19:02:04 UTC
>      xattr: fix listing EAs on *BSD for non-root users

This is the same here:
http://repo.or.cz/w/Samba.git/shortlog/refs/heads/v4-1-test

Your new patch is functional the same as the old one:
referencetest26 2.2.4 # diff -Nurp 
bug-10260-master_smbclient_del_dir.patch 
bug-10260-master_smbclient_del_dir_1.patch
--- bug-10260-master_smbclient_del_dir.patch    2013-11-13 
18:01:02.066570900 +0100
+++ bug-10260-master_smbclient_del_dir_1.patch  2013-11-13 
19:42:13.426787100 +0100
@@ -1,4 +1,4 @@
-From f54ef3aac5beff8b004db1de3b6344db9194ea0c Mon Sep 17 00:00:00 2001
+From e7686a35df49852ee6c2e1cc1b1e3495ee69bb95 Mon Sep 17 00:00:00 2001
  From: Jeremy Allison <jra@samba.org>
  Date: Tue, 12 Nov 2013 15:17:26 -0800
  Subject: [PATCH 1/3] Bug #10260 - smbclient shows no error if deleting a
@@ -69,7 +69,7 @@ index 3f59df8..b160290 100644
  1.8.4.1


-From a061a0df6e3fa3ce9b32864ac3bb269f027f4b95 Mon Sep 17 00:00:00 2001
+From 45796f8d1d74cc8089c6ea561a00caa34836163b Mon Sep 17 00:00:00 2001
  From: Jeremy Allison <jra@samba.org>
  Date: Tue, 12 Nov 2013 15:32:42 -0800
  Subject: [PATCH 2/3] Bug #10260 - smbclient shows no error if deleting a
@@ -200,7 +200,7 @@ index a373cd6..a550dd7 100644
  1.8.4.1


-From 09a1d7a7b3145b93d26bb26ee72205aa9f53a15a Mon Sep 17 00:00:00 2001
+From d08a50fbc1103294b0f6770bbfd101aedb0e3f99 Mon Sep 17 00:00:00 2001
  From: Jeremy Allison <jra@samba.org>
  Date: Tue, 12 Nov 2013 15:55:51 -0800
  Subject: [PATCH 3/3] Bug #10260 - smbclient shows no error if deleting a


Thats why the problem remains the same. Sorry.
Made a new try: Downloaded

http://repo.or.cz/w/Samba.git/snapshot/2cfa1ef6a9e2728c9cdd185d6d08a452e0485200.tar.gz

The problem remains the same...
Comment 9 Jeremy Allison 2013-11-13 19:26:31 UTC
I'm sorry you're having trouble, but you're doing something wrong:

This page:

http://repo.or.cz/w/Samba.git/shortlog/refs/heads/v4-1-test

Download the link: 

http://repo.or.cz/w/Samba.git/snapshot/2cfa1ef6a9e2728c9cdd185d6d08a452e0485200.tar.gz

Then:

$ gzip -d -c ../Downloads/Samba*.gz | tar xvf -
$ cd Samba/
$ patch -p1 </tmp/look
patching file source3/smbd/dir.c
patching file source3/smbd/proto.h
patching file source3/smbd/reply.c
patching file source3/include/proto.h
Hunk #1 succeeded at 480 (offset 5 lines).
patching file source3/lib/util.c
patching file source3/smbd/dir.c
patching file source3/smbd/proto.h
patching file source3/libsmb/cli_smb2_fnum.c
patching file source3/libsmb/cli_smb2_fnum.h
patching file source3/libsmb/clilist.c
$

Please try and reproduce that. I'm at a loss to see why you can't.

Jeremy
Comment 10 Thomas Bork 2013-11-13 19:58:11 UTC
Checking the patch with --dry-run shows:

referencetest26 # patch --dry-run -p1 <look.txt
checking file source3/smbd/dir.c
checking file source3/smbd/proto.h
checking file source3/smbd/reply.c
checking file source3/include/proto.h
Hunk #1 succeeded at 480 (offset 5 lines).
checking file source3/lib/util.c
checking file source3/smbd/dir.c
Reversed (or previously applied) patch detected!  Assume -R? [n]
Apply anyway? [n]
Skipping patch.
1 out of 1 hunk ignored
checking file source3/smbd/proto.h
Hunk #1 FAILED at 218.
1 out of 1 hunk FAILED
checking file source3/libsmb/cli_smb2_fnum.c
checking file source3/libsmb/cli_smb2_fnum.h
checking file source3/libsmb/clilist.c

But applying the patch anyway shows:

referencetest26 # patch -p1 <look.txt
patching file source3/smbd/dir.c
patching file source3/smbd/proto.h
patching file source3/smbd/reply.c
patching file source3/include/proto.h
Hunk #1 succeeded at 480 (offset 5 lines).
patching file source3/lib/util.c
patching file source3/smbd/dir.c
patching file source3/smbd/proto.h
patching file source3/libsmb/cli_smb2_fnum.c
patching file source3/libsmb/cli_smb2_fnum.h
patching file source3/libsmb/clilist.c

:-)
Sorry, the problem was the not applied first part of source3/smbd/dir.c 
due --dry-run.

Okay, testing now...
Comment 11 Thomas Bork 2013-11-13 20:45:09 UTC
Confirming that the patch does it:

test # smbclient -V
Version 4.1.2-UNKNOWN-for-eisfair-1-patch-1
test # smbclient //test/tb -Utb
Enter tb's password:
Domain=[TEST] OS=[] Server=[]
smb: \> dir
   .                                   D        0  Sun Nov 10 22:45:30 2013
   ..                                  D        0  Fri Aug  9 02:34:52 2013
   A                                   D        0  Sun Nov 10 22:28:36 2013

                 3939864 blocks of size 1024. 2635032 blocks available
smb: \> del x
NT_STATUS_NO_SUCH_FILE listing \x
smb: \> del A
NT_STATUS_NO_SUCH_FILE listing \A
smb: \>


Thanks a lot!


Do you know, why smbclient from Samba 4.1.x only shows
Domain=[TEST] OS=[] Server=[]

and smbclient from Samba 3.6.x shows the complete information
Domain=[TOMMAIK] OS=[Unix] Server=[Samba 3.6.19-for-eisfair-1-patch-1]
?

Should I open a new bug report for this?
Comment 12 Jeremy Allison 2013-11-13 20:55:33 UTC
Yeah, that's a different issue. New bug report please :-).

Jeremy.
Comment 13 Jeremy Allison 2013-11-13 21:09:00 UTC
Created attachment 9418 [details]
git-am fix applies cleanly to master/v4-1-test

This is what I'm going to propose on samba-technical for master.

Correct patch messages, uses a bool of processed_file, instead of a count (which isn't needed).

Jeremy.
Comment 14 Jeremy Allison 2013-11-14 22:52:01 UTC
Created attachment 9422 [details]
git-am fix for 4.1.next.

Contains git-cherry-pick info from master.

Jeremy.
Comment 15 Andreas Schneider 2013-11-15 15:25:57 UTC
Comment on attachment 9422 [details]
git-am fix for 4.1.next.

LGTM
Comment 16 Jeremy Allison 2013-11-15 16:38:21 UTC
Re-assigned to Karolin for inclusion in 4.1.next.

Thanks !

Jeremy.
Comment 17 Karolin Seeger 2013-11-22 10:30:02 UTC
Pushed to autobuild-v4-1-test.
Comment 18 Karolin Seeger 2013-11-26 19:28:04 UTC
Pushed to v4-1-test.
Closing out bug report.

Thanks!