Attempting to copy certain files from a Samba 3.6 share to a local Windows 7 PC fails an error popup stating: Error 0x800700FF: The extended attributes are inconsistent. After experimenting with several options, I have concluded that (1) this only happens with max protocol set to SMB2, and (2) it only happens with certain xattrs. It seems to have something to do with the characters in the xattr name. I found that if there is a hyphen or underscore within the name, this error will occur. So, 'ab' will work, but 'a-b' or 'a_b' will fail.
Created attachment 7855 [details] Patch to skip Solaris internal xattrs I ran across this issue on a Solaris system due to some internal xattrs, and the attached patch works around my specific issue. But it is also reproducible on Linux if I manually give a file an xattr with a dash or hyphen using setfattr.
Ok, so here's the strange thing.... I just wrote a torture test that creates a file over SMB2 with an EA name of "hyphen-name" and it works fine. So it's not just the name that matters. Can you upload a capture of the experiment you did to get the error "The extended attributes are inconsistent" message ? I need to be able to reproduce this with a torture test before I know I've correctly fixed it. As mentioned above, it's not just the name that matters here. Jeremy.
Sorry, forgot to mention - I was creating against a Win7 SMB2 server (just for completeness).
Created attachment 8668 [details] Wireshark trace of error root@justin-rn314:/data/Backup# getfattr -d New\ Rich\ Text\ Document.rtf # file: New Rich Text Document.rtf user.DOSATTRIB=0sMHgyMAAAAwADAAAAEQAAACAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAH6oBhq2Jc4BAAAAAAAAAAA= user.SAMBA_PAI=0sAgSABwAAAAABYwAAAAAAYwAAAAAAYwAAAAABYwAAAAABYgAAAAAAYgAAAAAC/////w== user.SUNW_ro="abc" user.a-b user.a_b
Created attachment 8669 [details] good capture Changed a_b EA to name a_bb. (Size also +200, but that doesn't matter). Jeremy (working as Justin).
I understand what is going on here. Patch to follow.
Created attachment 8685 [details] git-am fix for 3.6.next. Justin, can you confirm this fixes the problem for you and I'll get a 2nd Team member review. Thanks ! Jeremy.
Created attachment 8686 [details] One more minor fix on top. Justin, please check it fixes the problem. Thanks, Jeremy.
Created attachment 8687 [details] git-am fix for master and 4.0.next. Justin, please check and confirm it fixes the customer issue. Cheers, Jeremy.
Created attachment 8688 [details] bad capture from solaris I tried the 3.6 patch against my Solaris platform (after removing my skip Solaris ACL patch), and it didn't solve the issue. Here's my xattr list. From a Win7 client, all I did was create a new file on the share, and then try to copy it to my desktop. root@justin-res:/data/backup# runat "New Text Document.txt" ls -l -la total 4 drwxrwxrwt 4 guest guest 5 Mar 26 19:29 . -rwxrwxrwx+ 1 guest guest 0 Mar 26 19:29 .. -r--r--r-- 1 root root 156 Mar 26 19:29 SUNWattr_ro -rw-r--r-- 1 root root 472 Mar 26 19:29 SUNWattr_rw -rw-rw---- 1 guest guest 56 Mar 26 19:29 user.DOSATTRIB
This is a different problem. It's now correctly returning the 2 Solaris EA's of SUNWattr_ro and SUNWattr_rw, neither of which are zero length. Who creates these attributes ? It isn't Samba. This is a different test case from the one you originally reported. If you test with the original EA's of: user.SUNW_ro="abc" user.a-b user.a_b as we had in the test case, I think you'll find it works :-). If we can I'd like to get the original test case verified as fixed, then I can work on this new issue either as an auxiliary case (if it's a related problem) or as a new bug. SUNWattr_ro is being correctly returned (the ls shows a size of 156, which is correctly returned inside the EA). However SUNWattr_rw show an ls size of 472, but is being returned inside the EA as a length of 216. Of course the client shouldn't care about any of that :-). Jeremy.
Oh never mind :-). Wireshark is missing reading the upper length bytes of 0x1, which when added to the lower length byte of 0xd8 leaves the correct length (0x1d8 == 472). So I can't immediately see why Windows is objecting to this EA return.
You are correct, the original test case does indeed work now. :-) This new issue does appear to be related to the EA data lengths somehow, because I can reproduce it on Linux by creating EA's with data lengths matching those on the Solaris box.
Created attachment 8690 [details] git-am fix for 3.6.x Found the problem. From the additional patch.. The spec lies when it says that NextEntryOffset is the only value considered when finding the next EA. We were adding 4 more extra pad bytes than needed (i.e. if the next entry already was on a 4 byte boundary, then we were adding 4 additional pad bytes). Try with this patchset. Fixes the problem for me. Jeremy.
Created attachment 8691 [details] git-am fix for master and 4.0.x Same patchset for master/4.0.x. Once Justin confirms I'll submit to master and then get review on 4.0.x. Jeremy.
Great! That's got it fixed for me on both platforms.
Created attachment 8696 [details] Updated 4.0 patch Justin, sorry to trouble you but there were a couple of (mostly cosmetic :-) change requests to the 4.0.x patch, so I was wondering if you could re-test with this version just to make sure what goes into 4.0.x works for your product. Sorry for the trouble, Jeremy.
No problem. I have confirmed that the updated 4.0 patch also works for me.
Comment on attachment 8690 [details] git-am fix for 3.6.x David, please review the 3.6.next version (minimal patch for 3.6.next only).
Created attachment 8699 [details] git-am fix for 4.0.x. This is the fix that was pushed to master. David, please +1 on this and the 3.6.x fix and I'll re-assign to Karolin. Jeremy.
Created attachment 8701 [details] Latest git-am fix for 4.0 containing uninitialized variables change.
Comment on attachment 8690 [details] git-am fix for 3.6.x Uninitialized variables change not needed for 3.6.x, get_ea_list_from_file() already returns NULL if no EA list.
Marking this one as a regresssion in 4.0.x, as we really do need to get this in for 4.0.next. Patch just needs to go into master (I'm working with David to get this finalized). Jeremy.
Comment on attachment 8701 [details] Latest git-am fix for 4.0 containing uninitialized variables change. Jeremy, the patch for 4.0 should also carry the selftest/knownfail change in master (7bee3ef68490bb38942d717e03e203d00be32f9f), right?
Comment on attachment 8690 [details] git-am fix for 3.6.x Would have been good to include the same fill_ea_chained_buffer() size calculation change as is in master (d9e7c8219fd8b3d770301a87bc1cd62b07b989ca), but looks okay as is.
Comment on attachment 8690 [details] git-am fix for 3.6.x I want to make as few changes to 3.6.x as possible. This is the minimal fix (IMHO). Jeremy.
Created attachment 8719 [details] git-am fix for 4.0.x that went into master. Here's the fix that went into master (with the added selftest/knownfail change). Jeremy.
Comment on attachment 8719 [details] git-am fix for 4.0.x that went into master. Looks good.
Re-assigning to Karolin for inclusion in 3.6.next and 4.0.next. Jeremy.
Pushed to v3-6-test.
Applying the patch set to v4-0-test fails with: user@host:/data/git/samba/v4-0-test$ git am bug-9130-4.0.patch Applying: Ensure we can never return an uninitialized EA list. Applying: Modify fill_ea_chained_buffer() to be able to do size calculation only, no marshalling. Applying: Change estimate_ea_size() to correctly estimate the EA size over SMB2. Applying: Fix bug #9130 - Certain xattrs cause Windows error 0x800700FF Applying: Fix bug #9130 - Certain xattrs cause Windows error 0x800700FF Applying: Add a test to show that zero-length EA's are never returned over SMB2. error: patch failed: selftest/knownfail:159 error: selftest/knownfail: patch does not apply Patch failed at 0006 Add a test to show that zero-length EA's are never returned over SMB2.
Hmmm. I remember fixing that issue, but maybe I did it for the patch that got obsoleted by the later fix that went into master. Can you hold off on 4.0.next until tomorrow morning (Monday Pacific Time) when I'll get to this asap ? It's just a matter of fixing the surrounding diff context for the selftest/knownfail fix, all the rest of the patch applies cleanly. Sorry for the problem. Jeremy.
Created attachment 8733 [details] Updated git-am fix for 4.0.x This is identical to the fix that David reviewed, with the change of the diff -u context in the selftest/knownfail change. Applies cleanly to 4.0.next. Please apply (I decided it was quicker to do it now than wait for Monday morning Pacific Time :-). Cheers, Jeremy.
(In reply to comment #33) > Created attachment 8733 [details] > Updated git-am fix for 4.0.x > > This is identical to the fix that David reviewed, with the change of the diff > -u context in the selftest/knownfail change. > > Applies cleanly to 4.0.next. > > Please apply (I decided it was quicker to do it now than wait for Monday > morning Pacific Time :-). > > Cheers, > > Jeremy. Pushed to autobuild-v4-0-test. Thanks for the quick reply!
Pushed to v4-0-test. Closing out bug report. Thanks!