Bug 12172 - a snapshot folder cannot be accessed via SMB1
Summary: a snapshot folder cannot be accessed via SMB1
Alias: None
Product: Samba 4.1 and newer
Classification: Unclassified
Component: VFS Modules (show other bugs)
Version: 4.3.11
Hardware: All All
: P5 normal (vote)
Target Milestone: ---
Assignee: Uri Simchoni
QA Contact: Samba QA Contact
Depends on: 12166
  Show dependency treegraph
Reported: 2016-08-24 04:00 UTC by Uri Simchoni
Modified: 2017-11-07 00:27 UTC (History)
3 users (show)

See Also:

git-am fix for 4.5.x (18.23 KB, patch)
2016-08-25 20:58 UTC, Uri Simchoni
jra: review+
git-am fix for 4.4.x and 4.3.x (14.51 KB, patch)
2016-08-25 20:58 UTC, Uri Simchoni
jra: review+
git-am fix for 4.4.next and 4.3.next (14.46 KB, patch)
2016-09-15 10:04 UTC, Uri Simchoni
jra: review+

Note You need to log in before you can comment on or make changes to this bug.
Description Uri Simchoni 2016-08-24 04:00:06 UTC
(This is probably a degradation introduced by the fix to https://bugzilla.samba.org/show_bug.cgi?id=11580)

To reproduce on a system that supports volume snapshots:
1. Create a subfolder "d" in a Samba share backed by a volume with snapshots
2. Create a text file "d\f.txt"
3. Create a snapshot
4. Modify d\f.txt content
5. From an 2003R2 client, open the share and right click Properties->Previous versions on d\f.txt - the previous version can be seen and accessed.
6. Right-click Properties->Previous versions on d - the previous directory version is displayed
7. Double click the previous version
Expected - previous version opens in a new explorer window
Actual - error message (access denied)

Also reproducible via smbclient (following meta-script assumes only one snapshot exists):

snapshot=`smbclient <credentials> //server/share -c "allinfo d" | grep "^GMT-"`
smbclient <credentials> //server/share -c "ls $snapshot\d\*"
Comment 1 Uri Simchoni 2016-08-24 04:05:05 UTC
Problem does not exist on SMB2 (the root cause is that in SMB1 server, the trans_findfirst handler receives "@GMT-<timestamp>\d\*" as findfirst argument and sends it verbatim to some VFS functions, which don't expect the trailing *. In SMB2 we don't have that - the directory is opened and the handle is queried).
Comment 2 Uri Simchoni 2016-08-24 04:05:54 UTC
(In reply to Uri Simchoni from comment #0)
Of course, to obtain the snapshot in smbclient-based reproduction, the grepping should be for @GMT-, not GMT-.
Comment 3 Jeremy Allison 2016-08-24 16:54:32 UTC
Why doesn't call_trans2findfirst() strip out the '*' as the trailing mask ?

Can you explain further ?
Comment 4 Uri Simchoni 2016-08-25 20:58:04 UTC
Created attachment 12404 [details]
git-am fix for 4.5.x
Comment 5 Uri Simchoni 2016-08-25 20:58:55 UTC
Created attachment 12405 [details]
git-am fix for 4.4.x and 4.3.x
Comment 6 Uri Simchoni 2016-08-25 21:00:29 UTC
The 4.4.x fix is the same, but the patch set does not include testing using SMB2 because that depends on client fixes which are not being backported to 4.4.x and 4.3.x
Comment 7 Uri Simchoni 2016-08-25 21:02:01 UTC
The added dependency is just because of the unit tests. The 4.4.x fix does not depend on anything.
Comment 8 Jeremy Allison 2016-08-25 23:05:03 UTC
Comment on attachment 12404 [details]
git-am fix for 4.5.x

Comment 9 Jeremy Allison 2016-08-25 23:31:01 UTC
Comment on attachment 12405 [details]
git-am fix for 4.4.x and 4.3.x


Question - do you also want to back-port the DFS+shapshot server fixes ?

I wasn't planning to do that for 4.4.x or below - certainly not the client changes.

Let me know.
Comment 10 Jeremy Allison 2016-08-25 23:31:27 UTC
Re-assigning to Karolin for inclusion in 4.5.x, 4.4.x, 4.3.x.
Comment 11 Uri Simchoni 2016-08-26 04:13:16 UTC
(In reply to Jeremy Allison from comment #9)
The DFS server fixes do not affect this bug, and the client fixes enable testing with SMB2. Since I saw that the client fixes are not being backported further than 4.5.x, I didn't backport the SMB2 testing. It turned out to be pretty clean too, since it amount to just omitting one patch, so I'm all good.
Comment 12 Stefan Metzmacher 2016-08-28 16:13:20 UTC
Pushed to autobuild-v4-5-test.
Comment 13 Stefan Metzmacher 2016-08-29 02:49:05 UTC
Pushed to v4-5-test.
Comment 14 Karolin Seeger 2016-09-13 10:08:10 UTC
Patch does not apply on current v4-4-test and v4-3-test:

user@host:/data/git/samba/v4-4-test$ git am sc-smb1-ls-44.patch
Applying: selftest: add content to files created during shadow_copy2 test
Applying: selftest: check file readability in shadow_copy2 test
Applying: selftest: test listing directories inside snapshots
error: patch failed: selftest/knownfail:325
error: selftest/knownfail: patch does not apply
Patch failed at 0003 selftest: test listing directories inside snapshots

Re-assigning to Uri.
Comment 15 Uri Simchoni 2016-09-13 10:28:37 UTC
I've refreshed my v4-4-test and v4-3-test branches from samba.org and it applies cleanly. I'll wait for a day or so to see whether in-flight patches cause the patch to fail.
Comment 16 Uri Simchoni 2016-09-15 10:04:09 UTC
Created attachment 12467 [details]
git-am fix for 4.4.next and 4.3.next

This is a patch for 4.4.next - the only difference from failed patch is in the "knownfail".

The new patch currently does not apply to v4-3-test, and the old patch still applies to v4-3-test.

After the fix for https://bugzilla.samba.org/show_bug.cgi?id=12149 lands in v4-3-test, I'll check which patch is right for 4.3.x.
Comment 17 Uri Simchoni 2016-09-15 19:23:41 UTC
Assigning to Karolin for inclusion in 4.4. next.

As for 4.3.x - I think the bug needs to be fixed because it is a degradation of the 4.3 series and it got fixed in master before the release 4.5.0 (i.e. before 4.3 went security-only). So if we issue a last bugfix release it should be in it.

However, we seem to be in some stalemate - the original patch for 4.4 and 4.3 still applies in current public v4-3-test. It probably failed because there are some fixes which for some reason haven't landed in v4-3-test so far.

If the fix is applied to v4-4-test, please don't close the bug but reassign to me. I'll sample v4-3-test from time to time and if it changes I'll check whether the original patch finally got broken, which would allow me to issue a corrected patch.

Comment 18 Uri Simchoni 2016-09-16 20:16:35 UTC
Comment on attachment 12467 [details]
git-am fix for 4.4.next and 4.3.next

The 4.4 fix is also good for 4.3.
Comment 19 Karolin Seeger 2016-09-20 07:12:04 UTC
Patch for 4.5 does not apply on current v4-5-test branch:

user@host:/data/git/samba/v4-5-test$ git am sc-smb1-ls-45.patch 
Applying: s2-selftest: run shadow_copy2 test both in NT1 and SMB3 modes
error: patch failed: source3/script/tests/test_shadow_copy.sh:5
error: source3/script/tests/test_shadow_copy.sh: patch does not apply
error: patch failed: source3/selftest/tests.py:186
error: source3/selftest/tests.py: patch does not apply
Patch failed at 0001 s2-selftest: run shadow_copy2 test both in NT1 and SMB3 modes
Comment 20 Karolin Seeger 2016-09-20 07:15:04 UTC
Pushed to autobuild-v4-{3,4}-test.
Comment 21 Karolin Seeger 2016-09-20 09:48:37 UTC
(In reply to Karolin Seeger from comment #14)
Patch has already been pushed to v4-5-test.
Sorry for the noise!