Bug 13861

This is an expected behavior change. It's unfortunate, but there was no way to fix this problem in a backward-compatible way. To get the legacy behavior when unix extensions are enabled, you'll need to add the "forceuid" and/or the "forcegid" mount options.

So the option has been renamed for some reason ? Google doesn't turn up much of any sense for me right now, and I'm obviously not 'in the lopp' on what the Samba people are up to.

Assuming this really does have to be done, could we not log a warning along the lines of 'old mount option used, change to forceXXX' - that would solve a lot of head scratching !

The option wasn't renamed. There were new ones added. The reasoning is in this patch description:

http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=4ae1507f6d266d0cc3dd36e474d83aad70fec9e4

IOW, you'll still need to use the "uid=" option but if you want the ownership information provided by the server to be overridden then you need to add the forceuid option.

The solution doesn't lend itself well to printk warnings at mount time either given that uid= is still a valid option. I don't like making "silent" behavior changes, but I believe we had no choice but to do this to fix the problems with the current behavior.

Well, I can't argue much with both IBM and RedHat can I :-)

I don't see why "not possible to have a mount by an unprivileged
user that shows the server's file ownership info." is such a problem that we would be prepared to introduce a silent change of behaviour, breaking peoples (long time) working set ups, and all without any warning. 
There is obviously a use case I've missed, because I think if you specify a particular local user to use, that should override what the server sends, after all I obviously know what I am doing :-)

If the idea is to add the feature that local non-root users can mount without override, why not add a new option (useserverownership=) that defaults to 'no' the old behaviour, but can be set 'yes' for users who have mounts in /etc/fstab and want the new behaviour. 
Or alternatively, default the new forceuid= option to what it needs to be to not break existing installs (which logically has the same effect as my hypothetical 'useserverownership'). People who need the new feature can turn it the other way in fstab.

That would mean everyone* caries on as normal, and people who want a different behaviour can go turn it on. I just don't understand why the default has to change, I guess.

Could we really not print a warning like "uid= without forceuid=, this may no longer do what you want [url]" ? With a good URL (not the commit message, which is fairly unclear) that would mitigate a lot of the pain.

Apologies if the patch description was unclear. Adding Steve to the discussion here since he has a stake in this as well...

My inclination is not to clutter up the logs with printk spam, but I'm willing to listen to arguments to the contrary.

I think changing the default behavior of the uid= option was the right thing to do. The existing behavior made *absolutely* no sense. When the mount was done by root (uid = 0) then we trusted that the ownership information from the server was correct. When done by an unprivileged user (uid != 0), we clobbered all of the ownership info from the server.

While it means some minor pain for people depending on the legacy behavior, I believe it means a cleaner set of mount options going forward.

Oh, and you certainly can argue with Red Hat and IBM, but since this comes down to a matter of default behavior, it's probably best to discuss it among a wider audience than bugzilla.

The usual forum is here:

https://lists.samba.org/mailman/listinfo/linux-cifs-client

Breaking existing setups was very very bad of us.  I didn't know about this.

It would have been better to leave the uid= behaviour unaltered and to add *new* mount options for the new functionality.  Then migrate userspace over to stop using uid= altogether.  Then deprecate and finally remove uid= support.

To aid in this process we could print a warning if userspace uses uid=.

At the very very least the kernel should be changed (immediately!) to emit a printk when it detects uid=, directing operators to the documentation which will allow them to unbreak their machines.  But this is a worse solution.


Please don't let us release 2.6.31 with this regression(!) in place in its current form.

Fair enough. I dislike saddling people that have what I consider to be more "correct" setups with extra mount options. But, if it's too costly to break existing setups this way I'm ok with reversing the default for now to ease the transition. Here's what I propose:

We'll back out the existing patch and add new "noforceuid" and "noforcegid" options instead.

Once that is done, the question becomes whether we should begin a transition to a different default. I argue yes. The existing default is more confusing IMO.

To bring that about we'll add a printk that warns that the default is going to change in the future (maybe in 2.6.33) and that the user's preferences will need to be declared by adding either forceuid or noforceuid to the options (i.e., the printk will only fire when neither is present). Ditto for forcegid.

We'll leave that printk in place for 2 releases, and then switch the default.

Sound reasonable? If so, I'll code up some patches and post them for review in the next day or so.

Sounds good, thanks.

Well, I'm not quite sure what Linus is going to say to this and the "kdesu broken" thread on the LKML is a good example of what happens when existing user space is broken.  FWIW.

I don't like the idea of warning about upcoming behavior changes in the kernel dmesg (seems like that could result in printk spam), but I could certainly live with new mount parms to accomplish Jeff's goal of making the uid behavior for unpriv mounts more sensible.

(clarifying my previous comment) I don't mind going back to the old uid behavior (but adding new mount parms to accomplish Jeff's goal) also am interested if adding warnings in user space (mount.cifs) would be possible

What I need clarification on is what you think the default should (eventually) be. Should we transition this code so that server provided uid's are the default even when uid= is specified? Or should we leave the default as-is?

If we're planning to leave the default as-is, we can forgo warnings altogether (and maybe can change the parameter name to "serveruids" or maybe "servergids" or something).

If we want to change the default eventually, then we need to add a warning to make it clear when and how it'll be changing. Doing this is userspace is tough. How will mount.cifs know what the kernel's default is? I think the only reasonable way to do that is from in-kernel with a printk.

The good news is that it needn't throw printk spam forever. We'll only throw the warning when someone specifies uid= and doesn't add forceuid or noforceuid. If the admin adds those options then we can silence it.

I will discuss options with Jeff tomorrow, but this reminds me that for 2.6.32 or 2.6.33 we need to investigate:

1) how to detect the case where uids on server and client don't match (not always easy to do, but there are some special cases where we can tell)
2) Uid mapping (multiple ways to do this)

I reviewed Jeff's proposed patch, and am ok with it with minor corrections (a warning message that was left out).

Patch posted to LKML, linux-fsdevel and linux-cifs-client this morning. Testing and/or review would be appreciated before I ask Steve to push this to Linus.

Handled-By : Jeff Layton <jlayton@redhat.com>
Patch : http://patchwork.kernel.org/patch/38498/

Merged into cifs-2.6.git

Will request upstream merge within a few days.

Fixed by commit 9b9d6b2434fe942895c341b9a982f158529788ec .