Bug 450015 - CIFS needs POSIX create capability
CIFS needs POSIX create capability
Status: CLOSED DUPLICATE of bug 465143
Product: Red Hat Enterprise Linux 5
Classification: Red Hat
Component: kernel (Show other bugs)
5.3
All Linux
high Severity high
: rc
: ---
Assigned To: Jeff Layton
Red Hat Kernel QE team
:
Depends On:
Blocks: 483701 493728 740832 743467
  Show dependency treegraph
 
Reported: 2008-06-04 15:10 EDT by Issue Tracker
Modified: 2014-06-18 03:37 EDT (History)
9 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
: 493728 (view as bug list)
Environment:
Last Closed: 2009-04-22 14:56:22 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


Attachments (Terms of Use)

  None (edit)
Description Issue Tracker 2008-06-04 15:10:56 EDT
Escalated to Bugzilla from IssueTracker
Comment 1 Issue Tracker 2008-06-04 15:10:58 EDT
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
Comment 2 Issue Tracker 2008-06-04 15:10:59 EDT
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
Comment 9 Issue Tracker 2008-06-09 11:33:50 EDT
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
Comment 12 Issue Tracker 2008-07-08 16:53:01 EDT
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
Comment 16 Issue Tracker 2008-07-31 16:41:14 EDT
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
Comment 17 Issue Tracker 2008-08-01 12:40:00 EDT
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
Comment 19 Issue Tracker 2008-08-04 18:26:12 EDT
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
Comment 21 Issue Tracker 2008-08-28 15:25:32 EDT
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
Comment 22 Simo Sorce 2008-08-28 18:04:44 EDT
Can we make this bug publicly accessible, it would help discussing it with other samba.org developers
Comment 23 Jeremy West 2008-08-28 18:37:21 EDT
Simo,

If at all possible, please just cc the individual samba.org developers you'd like feedback from.

Thanks
Jeremy
Comment 24 Issue Tracker 2008-08-28 19:30:05 EDT
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
Comment 25 Simo Sorce 2008-08-28 20:09:46 EDT
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.
Comment 26 Jeremy West 2008-08-29 00:39:46 EDT
For the benefit of the samba community, I'm making this BZ public.

Thanks
Jeremy West
Comment 27 Issue Tracker 2008-08-29 10:41:37 EDT
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
Comment 37 RHEL Product and Program Management 2008-09-18 15:16:02 EDT
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.
Comment 44 Issue Tracker 2008-12-18 15:37:06 EST
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
Comment 46 Issue Tracker 2008-12-18 16:31:12 EST
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
Comment 47 Issue Tracker 2009-01-15 15:29:31 EST
Agreed.  It is definitely NOT fixed.


This event sent from IssueTracker by vincew 
 issue 180573
Comment 50 Issue Tracker 2009-01-23 12:02:16 EST
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
Comment 51 Simo Sorce 2009-01-23 16:34:05 EST
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.
Comment 53 Jeff Layton 2009-01-23 16:41:32 EST
Just to be clear...

This bug was opened against RHEL4. Are the Linux CIFS clients also RHEL4?
Comment 54 Simo Sorce 2009-01-23 16:43:21 EST
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
Comment 56 Jeff Layton 2009-01-23 17:36:21 EST
It's highly unlikely we'll be able to address this in RHEL4. RHEL5 is a much more likely target.
Comment 58 Issue Tracker 2009-01-28 13:34:34 EST
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
Comment 59 Jeff Layton 2009-02-06 15:01:17 EST
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.
Comment 60 RHEL Product and Program Management 2009-02-16 10:22:03 EST
Updating PM score.
Comment 62 Jeff Layton 2009-02-25 20:04:42 EST
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.

Note You need to log in before you can comment on or make changes to this bug.