Escalated to Bugzilla from IssueTracker
Customer Name: IBM Burlington Customer RH Login: etsslx2 Description of problem: The customer has found that the function of the "force create mode" option in Samba has changed between Samba 3.0.10 and 3.0.25b on RHEL 4, but the documentation has not. How reproducible: Export a share from a Samba 3.0.25b server and set "force create mode = 664". Mount that share on a Linux CIFS client and touch a file within it. The new file will not have the forced permissions. Steps to Reproduce: 1. Install the 2.6.9-68.27 kernel (hotfix for this customer) 2. Install Samba 3.0.25b-1.el4_6.4 3. Create a share in smb.conf and add the option "force create mode = 0664" 4. Mount that share using CIFS 5. Create a new file within that share using the touch command Actual results: The permissions will be set by the user's umask on the client. Expected results: According the manpage, the permissions on new files should have been 664 in this case due to the "force create mode" flag. Additional info: If the exact same steps are done with a 3.0.10 server, the permissions are set according to the "force create mode" option. From looking at the Samba source, it appears that the unix_perms_from_wire() code has been changed. CIFS will create the file with a NT_CreateAndX call, then set the permissions with a SetFileInfo call. From looking at the Samba logs, the file create has the correct permissions applied. The difference shows up when the SetFileInfo packet arrives. In older versions of Samba, lp_force_create_mode was referenced, but in the 3.0.25b version, it is not. Instead, unix_perms_from_wire() considers the file to already exist and thus uses the lp_force_security_mode instead. As a result, the customer can resolve this issue by using "force security mode = 0664" in smb.conf. This IT is to point out that the functionality has changed, to confirm that the change was intended, and to report a documentation defect against the smb.conf manpage since it does not indicate the uselessness of the "force create mode" option and the corresponding importance of the "force security mode" option. Since CIFS will always follow up the NTCreate with a SetFileInfo, I do not see a case where the "force create mode" will ever be used. This event sent from IssueTracker by vincew [SEG - Storage] issue 180573
SEG, Normally I'd fill out the escalation template, however IBM has done a delightful good job of this in the ticket description. Everything you need to know is there, aside from the question, "what are you asking SEG to do?" Here's that answer: -------- This IT is to point out that samba functionality has changed, to confirm that the change was intended, and to report a documentation defect against the smb.conf manpage since it does not indicate the uselessness of the "force create mode" option and the corresponding importance of the "force security mode" option. Since CIFS will always follow up the NTCreate with a SetFileInfo, I do not see a case where the "force create mode" will ever be used. -------- Basically, if you can help us confirm this change was intended, then we need to get this filed against documentation. If this change was *not* intended, then let's send this up as a bug. Thanks Jeremy Issue escalated to Support Engineering Group by: jwest. jwest assigned to issue for IBM-IGS. Internal Status set to 'Waiting on SEG' Status set to: Waiting on Tech This event sent from IssueTracker by vincew [SEG - Storage] issue 180573
Since the workaround of using "force security mode" works so well, this customer should be fine with that timetable. That said, releasing it as an errata or EUS for 4.7 might prevent a few calls from other customers running into the same thing. This event sent from IssueTracker by bottchen issue 180573
I think you're getting this issue confused with another. The function of the "force create mode" was changed between versions. If this change was intentional, then the manpage needs to be updated. If it was not intentional, which suspect is the case, then it needs to be reverted back to it's previous function. The reason I suspect it was unintentional is because the "force create mode" option has been rendered completely useless. Anyone that relied on that in the past would find that it no longer worked. As far as I know, unix extensions has nothing to do with this issue. I don't believe I mentioned it anywhere in my problem description. Internal Status set to 'Waiting on Support' Status set to: Waiting on Tech This event sent from IssueTracker by bottchen issue 180573
Hi Simo, Were you referring to filesystem ACL's in use on the underlying filesystem in question? Jeremy can we confirm whether this is the case or not (that customer is mounting the filesystem(s) these shares export with ACL's enabled)? Is this a system we already have a sysreport/sosreport from the customer for (from another case?) --vince Internal Status set to 'Waiting on Engineering' This event sent from IssueTracker by vincew issue 180573
I doubt the customer is using ACLs. I know in my testing in the lab, I was not. If the engineer is recalling correctly, why would force create mode be tied to ACLs? They seem like completely different functions. Also, keep in mind that this function changed between 3.0.10 and 3.0.25b. I do not know why a tie between ACLs and force create mode would have been introduced between minor releases. Internal Status set to 'Waiting on Support' Status set to: Waiting on Tech This event sent from IssueTracker by bottchen issue 180573
VINCE NOTES It does in fact appear to be as customer reports - smbd/trans2.c - comparing between 3.0.10 and 3.0.28 (latest rhel4 released packages) - we only seem to apply what is set in lp_force_create_mode in the switch case of ptype being PERM_NEW_FILE. for PERM_EXISTING_FILE lp_force_security_mode() is used instead. 3.0.10 doesn't make a distinction - it only looks at whether we are dealing with a file or a directory. "bottom half of function" code comparisons: in 3.0.10: if (VALID_STAT(*pst) && S_ISDIR(pst->st_mode)) { ret &= lp_dir_mask(SNUM(conn)); /* Add in force bits */ ret |= lp_force_dir_mode(SNUM(conn)); } else { /* Apply mode mask */ ret &= lp_create_mask(SNUM(conn)); /* Add in force bits */ ret |= lp_force_create_mode(SNUM(conn)); } return ret; } In 3.0.25: switch (ptype) { case PERM_NEW_FILE: /* Apply mode mask */ ret &= lp_create_mask(SNUM(conn)); /* Add in force bits */ ret |= lp_force_create_mode(SNUM(conn)); break; case PERM_NEW_DIR: ret &= lp_dir_mask(SNUM(conn)); /* Add in force bits */ ret |= lp_force_dir_mode(SNUM(conn)); break; case PERM_EXISTING_FILE: /* Apply mode mask */ ret &= lp_security_mask(SNUM(conn)); /* Add in force bits */ ret |= lp_force_security_mode(SNUM(conn)); break; case PERM_EXISTING_DIR: /* Apply mode mask */ ret &= lp_dir_security_mask(SNUM(conn)); /* Add in force bits */ ret |= lp_force_dir_security_mode(SNUM(conn)); break; } *ret_perms = ret; return NT_STATUS_OK; } However note that the 3.0.10 snip from above does not indicate that lp_force_security_mode settings are applied here - so it must be done elsewhere - so I do need to double-check function call flow and hopefully by doing so, be able to provide engineering what they need to decide what needs to be done (man page fix or correct code). Jeremy it might be best to wait until I've followed function calls around a bit to let IBM know, but if you do want to update them based on above, just know that I will be looking at this some more and posting again soonish. --vince This event sent from IssueTracker by vincew issue 180573
UPDATE I have been able to validate the code path of functions called when new files are created over the share (following a test run of the steps to reproduce), and can now say with complete confidence that 'force create mode' settings are NOT being applied. Furthermore, after tracing through the code, I have to agree that I don't understand why things are being done this way now. If 'force create mode' settings are only being applied when ptype indicates a new file, how can this ever happen if we don't call unix_perms_from_wire() until trans2 (after the file has already been created)? With the understanding of this section of code I have now, we should be able to clearly point out to Engineering what is going on and ask for an answer on whether things SHOULD be working this way or not. Personally, I don't think they should be, but they may be able to give a good explanation as to why. Notes and more info to follow once I double-check which of the three call_trans2setfilepathinfo calls made after the creation of a new file change the value of ptype -- since the value of ptype is evaluated in unix_perms_from_wire() and the decision about whether to apply lp_force_create_mode settings ensues. --vince This event sent from IssueTracker by vincew issue 180573
Can we make this bug publicly accessible, it would help discussing it with other samba.org developers
Simo, If at all possible, please just cc the individual samba.org developers you'd like feedback from. Thanks Jeremy
Simo, The executive summary version is: I think there is something wrong with the code as it is presently, but need someone to provide an authoritative answer and explanation if the code is working as designed. After considerable evaluation of the code path responsible for creating new files over Samba shares, and testing with highly verbose log levels, I believe that the point the customer is trying to make is correct. 'force create mode' does not work for files created from CIFS clients. I am not convinced there is a good reason for us to ignore 'force create mode' for Unix CIFS clients, not when we honor it for non-Unix clients. If there is a solid reason for us to require use of 'force security mode' to enforce filesystem permissions the Samba administrator desires to be set for all files created on a share, but only when the client is a CIFS client to the share, I am open to being educated on it. I do recall that support for a CIFS client's umask being honored on created files has been added to newer CIFS and Samba versions, but I'm still not convinced that a _client's_ umask should have higher precedence on the permissions set on a file than what a Samba administrator has specified in the smb.conf with a 'force create mode' parameter. I am also pretty sure that this isn't due to umask support. It looks to me like we aren't even bothering to look at lp_force_create_mode unless we land in unix_perms_from_wire() with ptype set to PERM_NEW_FILE. Yet for some reason by the time we have landed in unix_perms_from_wire() during call_trans2setfilepathinfo() processing, just after creating a new file, ptype is always PERM_EXISTING_FILE. I'm not sure we could ever arrive in unix_perms_from_wire() with ptype set to PERM_NEW_FILE, based on what I've seen testing. This is why I think something is wrong. This is a summary run-through of a repro test I did, taking small snips of the Samba client log file, and following the code along, and various notes. It may help save Engineering some time providing an authoritative answer. ---------- File created over cifs mount to samba server [2008/08/27 19:03:56, 3] smbd/process.c:switch_message(927) switch message SMBntcreateX (pid 3647) conn 0x85ec230 open_file_ntcreate() called to create file, kernel oplock set, etc. Send reply to client - reply_ntcreate_and_X(). Client sends trans2, causing call_trans2setfilepathinfo(): [2008/08/27 19:03:56, 3] smbd/trans2.c:call_trans2setfilepathinfo(5783) call_trans2setfilepathinfo(6) root-file1 (fnum -1) info_level=512 totdata=100 Info level 512 --> 0x200 <trans2.h> #define SMB_QUERY_FILE_UNIX_BASIC 0x200 /* UNIX File Info*/ #define SMB_SET_FILE_UNIX_BASIC 0x200 case SMB_SET_FILE_UNIX_BASIC: { status = smb_set_file_unix_basic() in smb_set_file_unix_basic(), we call unix_perms_from_wire() ---------- unix_perms_from_wire() makes its decision based on the value of ptype passed into the function. ptype comes from smb_set_file_unix_basic() -- based on the evaluation of (VALID_STAT(*psbuf)). We can only get PERM_NEW_FILE if this evaluates to 0. psbuf is passed to smb_set_file_unix_basic as 6th arg, and is &sbuf passed from call_trans2setfilepathinfo(). --------- Thank you, Vince This event sent from IssueTracker by vincew issue 180573
Vince, you are correct, I provided a summary of the bug to Jeremy Allison and he told me that the bug is fixed in 3.2.x This means we will have to fix it in 3.0.x. I am not sure if he is going to officially fix in in 3.0.x and the code in 3.2.x is different enough that we cannot blindly apply the patch as is. I will try to find some time to work on this next week.
For the benefit of the samba community, I'm making this BZ public. Thanks Jeremy West
Thanks, Simo. I'll look at 3.2 some also. --vince Internal Status set to 'Waiting on Engineering' This event sent from IssueTracker by vincew issue 180573
This request was evaluated by Red Hat Product Management for inclusion in a Red Hat Enterprise Linux maintenance release. Product Management has requested further review of this request by Red Hat Engineering, for potential inclusion in a Red Hat Enterprise Linux Update release for currently deployed products. This request is not yet committed for inclusion in an Update release.
Jeremy, Simo said he probably wasn't going to get this one taken care of in time for the deadline, but suggested we try the 3.0.33 packages to see if other changes to code would fix this problem. Rather than ask the customer to test it I am going to try it myself, as it is relatively straightforward to reproduce. I don't want to ask the customer to test something unless we have at least a certain level of confidence that it will solve their problem. --vince This event sent from IssueTracker by vincew issue 180573
Sorry for typo error, bad edit, that was supposed to be "Used the 0.el4.12 packages FROM Brew" in 2nd paragraph --vince This event sent from IssueTracker by vincew issue 180573
Agreed. It is definitely NOT fixed. This event sent from IssueTracker by vincew issue 180573
My personal opinion on this is that if a 'force create mode' is specified for a share, it should *NOT* be allowed to be overridden by ANY *client* behavior whatsoever. The setting is supposed to be there so the Samba server administrator is in charge of the permissions on newly-created files, not the client. Not everybody running Samba servers defines a 'force create mode' on their shares, but if they do, they probably have a good reason for doing so. (example.... members of a GROUP having write perms on a file, not just the owner...) So if it doesn't always work, why even have it at all? Note that same problem is likely with 'force directory mode' versus 'force directory security mode' for the same reason. However, rather than start (or revitalize, as the case may be) any arguments with upstream about whether a linux cifs client user's umask having "final word" is CORRECT or not, I am fine with what the general consensus (as reflected in #samba today) seems to be so long as the Samba docs are updated to state that 'force create mode' will not override permissions set by Linux cifs clients' umasks when unix extensions are enabled, and that 'force security mode' must be used to override them. IBM already has their workaround, so if we choose to address this by updating the docs upstream rather than the code, they should have nothing to complain about. --vince This event sent from IssueTracker by vincew issue 180573
To all, I apologize for not having spotting this earlier but my previous analysis of this bug was faulty. After carefully analyzing both Samba and cifs.ko code and verifying it with network traces I am going to actually reassign this bug to the cifs.ko component. I am now convinced that this is not a samba bug. Moreover it is not even a documentation bug because samba behaves as "expected" although I understand why we get the "unexpected" outcome in the test cases. Here is why: The problem lies in the fact that cifs.ko does not yet use a posix open call even when posix extensions are negotiated. BEcause of that it can pass the desired mode to the server on the open. Instead cifs.ko uses a normal open and *then* after the server returns successfully it goes and set the mode according to its umask. From the POV of samba this is just a regular chmod operation. Samba has no way to distinguish between the chmod perfomred immediately after the file is created by cifs and a chmod explicitly requested by an application running on the client machine using cifs.ko Needless to remember that 'force create mode' makes sense exclusively at creation time and does not affect chmod operations by design, that's what 'force security mode' is for, and that's why 'force security mode' "works". Because of that I have no way to solve this bug as a samba bug, and I am reassigning it to the cifs kernel component, where Jeff can pick it up and deal with it. Again apologies for the time lost but this is the right solution for this specific bug.
Just to be clear... This bug was opened against RHEL4. Are the Linux CIFS clients also RHEL4?
Ah just another comment that refers to the explanation in #19 about the change from 3.0.10 to 3.0.28 The change actually did happen because always using force create mode was actually a bug in 3.0.10
It's highly unlikely we'll be able to address this in RHEL4. RHEL5 is a much more likely target.
Changing IT to RHEL5 release per Jeff's plans with upstream CIFS work. Please let me know if you need anything. Testing, or whatever. --vince Internal Status set to 'Waiting on Engineering' Version changed from '4' to '5' This event sent from IssueTracker by vincew issue 180573
Patchset submitted upstream: Subject: [PATCH 1/2] cifs: refactor new_inode() calls and inode initialization Subject: [PATCH 2/2] cifs: have cifs_create use POSIX create if it's available ...once I get some comment or acceptance there, I'll work on backporting them for RHEL5.
Updating PM score.
The problem is a little more complex, actually... We need to handle opens and reopens the same way. There's another wrinkle too, samba has a bug that makes posix opens fail when SMB_O_CREAT isn't set. That was fixed upstream by JRA today, but all of the servers in the field (RHEL and otherwise) have this bug as of today. So it may be a bit before this is really properly fixed, even upstream.