Bug 7509 - smb_acl_to_posix: ACL is invalid for set (Invalid argument)
Summary: smb_acl_to_posix: ACL is invalid for set (Invalid argument)
Status: RESOLVED FIXED
Alias: None
Product: Samba 3.5
Classification: Unclassified
Component: File services (show other bugs)
Version: 3.5.3
Hardware: x86 Linux
: P3 regression
Target Milestone: ---
Assignee: Karolin Seeger
QA Contact: Samba QA Contact
URL: http://thread.gmane.org/gmane.network...
Keywords:
: 7661 (view as bug list)
Depends on:
Blocks: 8399
  Show dependency treegraph
 
Reported: 2010-06-10 16:37 UTC by Oliver Freyd
Modified: 2011-10-08 17:54 UTC (History)
8 users (show)

See Also:


Attachments
attempt of a fix for invalid ACLs (2.70 KB, patch)
2010-06-10 16:39 UTC, Oliver Freyd
no flags Details
Level 10 log and network trace (309.49 KB, application/octet-stream)
2011-08-17 08:10 UTC, Ariel Dembling
no flags Details
git-am fix for 3.6.1 (12.25 KB, patch)
2011-09-02 21:46 UTC, Jeremy Allison
no flags Details
git-am fix for 3.5.x. (11.79 KB, patch)
2011-09-02 22:14 UTC, Jeremy Allison
no flags Details
git-am fix for 3.6.1 (10.42 KB, patch)
2011-09-07 18:07 UTC, Jeremy Allison
no flags Details
git-am fix for 3.5.x. (10.05 KB, patch)
2011-09-07 18:20 UTC, Jeremy Allison
no flags Details
git-am fix for 3.6.1 (8.20 KB, patch)
2011-09-08 18:59 UTC, Jeremy Allison
metze: review+
obnox: review+
Details
git-am fix for 3.5.x. (7.93 KB, patch)
2011-09-08 19:02 UTC, Jeremy Allison
metze: review+
obnox: review+
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Oliver Freyd 2010-06-10 16:37:11 UTC
This cropped up in a migration from an NT4 domain to samba, 
when the shares are copied from a windows server to the samba server,
using robocopy with /copyall, so it will copy the data, then set the ACLs
on the files. 

On some files, I get the following errors in the log:

[2010/06/04 16:12:09.920879,  0] 
modules/vfs_posixacl.c:349(smb_acl_to_posix)
   smb_acl_to_posix: ACL is invalid for set (Invalid argument)
[2010/06/04 16:12:09.920894,  2] smbd/posix_acls.c:2895(set_canon_ace_list)
   set_canon_ace_list: sys_acl_set_file type directory default failed 
for file Installations/IMRE Singapore/IMRE on-site/settings (Invalid 
argument).
[2010/06/04 16:12:09.920909,  3] smbd/posix_acls.c:3979(set_nt_acl)
   set_nt_acl: failed to set default acl on directory Installations/IMRE 
Singapore/IMRE on-site/settings (Invalid argument).
[2010/06/04 16:12:09.920926,  3] smbd/error.c:80(error_packet_set)
   error packet at smbd/nttrans.c(1909) cmd=160 (SMBnttrans) 
NT_STATUS_INVALID_PARAMETER

This is given back to robocopy, which in seems to delete the file in case
of an error, which makes the error much worse, of course.

So I started debugging samba, and found a possible reason for the error:
The NT ACL is converted to a posix ACL, which has is then tested with
acl_valid(). It is invalid in my case. It contains 2 equal entries of
type ACL_USER_OBJ. 
Looking where that comes from, further up the callstack I found there
is an NT ACL S-1-3-0 uid 1044 which seems to be "Creator Owner", 
and another one with a "real" SID, and both map to the same posix ace.
So they have to be unified. 

I just did a small patch that keeps the USER_OBJ_ACE in a pointer,
and combines the permission bits.
Also, for debugging, I added a log line printing the ACL if acl_valid
fails. 

Still I wonder why this error happens only on a few files on my filesystem.
Anyway, the patch fixes it.
Comment 1 Oliver Freyd 2010-06-10 16:39:48 UTC
Created attachment 5789 [details]
attempt of a fix for invalid ACLs
Comment 2 Adam Buchbinder 2011-03-29 15:53:12 UTC
I think this is the same issue as bug 7661.

I added this patch to the current Debian version (2:3.5.6~dfsg-3squeeze2), and it's partially addressed the problem. The 'CREATOR OWNER' and 'CREATOR GROUP' entries still appear in the Security properties sheet (on Windows) for directories (but not files), and if I try to remove them, it appears to work, but they're there again when I re-open the Security properties sheet.

However, I can now add users or groups and set permissions for them, which I couldn't do before, so the patch is definitely an improvement.

There are still some quirks: for instance, if I add a user with read-execute permissions, the permissions appear normally, but if I add a group with read-execute permissions, the property sheet shows 'Special Permissions', and the specific permissions granted are 'Read & Execute' on 'This folder only' and 'Full Control' on 'Subfolders and files only', which is not what I checked off. I can edit these explicitly in the 'Advanced' dialog, but changing them in the main view again splits the ACL into two separate entries.

Additionally, there's no obvious way to tell who the owning group is from the Windows Security property sheet (the owning user is under the 'Owner' tab of the 'Advanced' dialog); if you try to edit the ACL for the owning group in certain ways (it seems to be rather finicky), an error is presented, reading 'A device attached to the system is not functioning.'

The client is Windows XP SP3.

I'm not sure to what extent these are separate issues, problems with the pre-patched code, or problems with the patch; I'd be happy to open up separate bugs if necessary, if someone can give me a bit of guidance.
Comment 3 Björn Jacke 2011-07-29 22:21:30 UTC
does setting "force unknown acl user = yes" fix this for you ?
Comment 4 Zefir01 2011-08-03 14:35:59 UTC
(In reply to comment #3)
> does setting "force unknown acl user = yes" fix this for you ?

I tested this problem. this setting does not fix key problems.
Comment 5 cesarkallas 2011-08-03 23:20:40 UTC
This settings doesn't fix the acl problem.
Comment 6 Volker Lendecke 2011-08-06 15:04:01 UTC
Please upload a debug level 10 log of smbd together with network traces leading to the failing operation. Please see

http://wiki.samba.org/index.php/Capture_Packets

and

http://wiki.samba.org/index.php/Client_specific_Log
Comment 7 Ariel Dembling 2011-08-17 08:10:18 UTC
Created attachment 6787 [details]
Level 10 log and network trace
Comment 8 Vladimir Kuklin 2011-08-31 14:56:12 UTC
Hi guys

I am experiencing the same problem.

Debian Squeeze 

These versions are also affected:

2:3.5.11~dfsg-1~bpo60+1
2:3.5.6~dfsg-3squeeze5

While trying to set acl "CREATOR OWNER" and "CREATOR GROUP" appear again and again. Removing them makes possible to alter ACE's for other users but leave no possibility to use "CREATOR OWNER" and "CREATOR GROUP" ACE's.

Any ideas?
Comment 9 Jeremy Allison 2011-09-01 00:05:56 UTC
Ok, got a good test case. Working on a fix for 3.5.x and 3.6.1.
Jeremy.
Comment 10 Jeremy Allison 2011-09-02 01:10:09 UTC
*** Bug 7661 has been marked as a duplicate of this bug. ***
Comment 11 Vladimir Kuklin 2011-09-02 10:41:32 UTC
Ok


The problem is that acl_valid(3) from libacl1 library is being called with duplicate ACE entries. That is, if I create a directory with user 'A' and then set CREATOR OWNER permission, posix ACL that is been constructed contains duplicate ACE's for this user that is incorrect in terms of libacl.

Oliver has already addressed this issue. But, unfortunately his patch was not included into debian source package. I will try it out and check if this helps.
Nevertheless, even without testing of Oliver's patch there remains a problem. This patch merges permission bits gained from CREATOR OWNER and the corresponding user NT ACE's. But then we must somehow split them again when translating from POSIX ACLs to NT ACLs. Maybe we should use extended attributes for a file, that will contain some type of "merging directives".
Comment 12 Jeremy Allison 2011-09-02 15:40:43 UTC
It's ok, I understand the problem fully. I have a patch here for 3.5.x and 3.6.1 that's currently under test.

Jeremy
Comment 13 Jeremy Allison 2011-09-02 21:46:57 UTC
Created attachment 6853 [details]
git-am fix for 3.6.1

Volker - please review. This splits up the patch into 5 separate commits in order to make it easier to understand, but all have to be applied in order to fix the bug. The 3.5.x version will follow.

Jeremy.
Comment 14 Jeremy Allison 2011-09-02 22:14:12 UTC
Created attachment 6854 [details]
git-am fix for 3.5.x.

Same fix as 3.6.1 back-ported to 3.5.x.
Jeremy.
Comment 15 Jeremy Allison 2011-09-06 03:40:02 UTC
Comment on attachment 6853 [details]
git-am fix for 3.6.1

Canceling request for review - will post a new version of this patch early next week.
Comment 16 Jeremy Allison 2011-09-06 03:40:16 UTC
Comment on attachment 6854 [details]
git-am fix for 3.5.x.

Canceling request for review - will post a new version of this patch early next week.
Comment 17 Jeremy Allison 2011-09-07 18:07:39 UTC
Created attachment 6864 [details]
git-am fix for 3.6.1

Fixed attachment - removed part #5 of previous patch.
Comment 18 Jeremy Allison 2011-09-07 18:20:29 UTC
Created attachment 6865 [details]
git-am fix for 3.5.x.

Fixed attachment - removed part #5 of previous patch.
Comment 19 Jeremy Allison 2011-09-07 23:54:26 UTC
Comment on attachment 6864 [details]
git-am fix for 3.6.1

Ok, I'm gonna do one last round on this one :-).
Comment 20 Jeremy Allison 2011-09-07 23:54:53 UTC
Comment on attachment 6865 [details]
git-am fix for 3.5.x.

Ok, one last round on this one (to get the patch right :-).
Comment 21 Jeremy Allison 2011-09-08 18:59:03 UTC
Created attachment 6868 [details]
git-am fix for 3.6.1

Patch now down to 3 parts. Other change will be attached to bug #8443.
Comment 22 Jeremy Allison 2011-09-08 19:02:44 UTC
Created attachment 6869 [details]
git-am fix for 3.5.x.

Patch now down to 3 parts. Other change will be attached to bug #8443.
Comment 23 Jeremy Allison 2011-09-29 15:35:38 UTC
Comment on attachment 6868 [details]
git-am fix for 3.6.1

Adding Michael Adams as he may have more time to review for 3.6.1.
Comment 24 Jeremy Allison 2011-09-29 15:36:42 UTC
Comment on attachment 6869 [details]
git-am fix for 3.5.x.

Adding Michael Adams as he may have more time to review.
Comment 25 Jeremy Allison 2011-10-03 21:22:42 UTC
Comment on attachment 6868 [details]
git-am fix for 3.6.1

vl is out. Changing to metze review.
Comment 26 Jeremy Allison 2011-10-03 21:23:07 UTC
Comment on attachment 6869 [details]
git-am fix for 3.5.x.

vl is out. Changing to metze review.
Comment 27 Jeremy Allison 2011-10-03 21:23:44 UTC
Setting this to blocker.

I *really* need this reviewed and in for 3.5.next and 3.6.1.

Jeremy.
Comment 28 Stefan Metzmacher 2011-10-05 09:11:41 UTC
Comment on attachment 6868 [details]
git-am fix for 3.6.1

Looks ok
Comment 29 Stefan Metzmacher 2011-10-05 09:12:28 UTC
Comment on attachment 6869 [details]
git-am fix for 3.5.x.

Looks ok
Comment 30 Michael Adam 2011-10-05 09:20:55 UTC
Comment on attachment 6868 [details]
git-am fix for 3.6.1

Looks good!
Comment 31 Michael Adam 2011-10-05 09:21:37 UTC
Comment on attachment 6869 [details]
git-am fix for 3.5.x.

Also for 3.5
Comment 32 Michael Adam 2011-10-05 09:22:14 UTC
Assigning to Karolin for inclusion into release branches.
Comment 33 Karolin Seeger 2011-10-08 17:54:14 UTC
Pushed to v3-5-test and v3-6-test.
Closing out bug report.

Thanks!