2.6.31 kernel breaks CIFS mounts without warning

[Tom Chiverton-7]

I tried out an RC of the 2.6.31 kernel recently, and discovered that under
that kernel the 'uid' option to mount.cifs has changed behaviour and the fix is
to add some new other option. At a guess, in almost all cases of giving a uid=
in fstab (different user UID locally and remotely) this change makes working
things stop.

Based on discussion at http://bugzilla.kernel.org/show_bug.cgi?id=13861 
(initially, I thought this was a bug) it was suggested I bring the discussion
here to see what a wider set of people thought.

Should we really not (at least) print a warning ('uid without forceuid, this
might be wrong as behaviour has changed [url]') ?
I spent a good while changing things on the server and client before I though
to roll back the kernel, and there's nothing obvious in Google (or the Samba
site) about this.

Also, why don't we just default the new option (forceuid) such that mounts
keep working (i.e. the old behaviour), and then people who prefer the new
behaviour can enable it if they want ?

That seems to make more sense to me, rather than changing long established
behaviour (it might well be for the better, but the side effects out weigh
this, I feel).
Swapping the default would create fstab 'spam' but that's still better than
'printk spam', which in turn is better than having stuff break without warning
or notice.

--
Tom
2 + 2 = 5 for extremely large values of 2.
_______________________________________________
linux-cifs-client mailing list
linux-cifs-client@...
https://lists.samba.org/mailman/listinfo/linux-cifs-client

[Jeff Layton-2]

On Wed, 29 Jul 2009 19:31:43 +0100
Tom Chiverton <bugzilla.kernel.org@...> wrote:

> I tried out an RC of the 2.6.31 kernel recently, and discovered that under
> that kernel the 'uid' option to mount.cifs has changed behaviour and the fix is
> to add some new other option. At a guess, in almost all cases of giving a uid=
> in fstab (different user UID locally and remotely) this change makes working
> things stop.
>
> Based on discussion at http://bugzilla.kernel.org/show_bug.cgi?id=13861 
> (initially, I thought this was a bug) it was suggested I bring the discussion
> here to see what a wider set of people thought.
>
> Should we really not (at least) print a warning ('uid without forceuid, this
> might be wrong as behaviour has changed [url]') ?
> I spent a good while changing things on the server and client before I though
> to roll back the kernel, and there's nothing obvious in Google (or the Samba
> site) about this.
>
> Also, why don't we just default the new option (forceuid) such that mounts
> keep working (i.e. the old behaviour), and then people who prefer the new
> behaviour can enable it if they want ?
>
> That seems to make more sense to me, rather than changing long established
> behaviour (it might well be for the better, but the side effects out weigh
> this, I feel).
> Swapping the default would create fstab 'spam' but that's still better than
> 'printk spam', which in turn is better than having stuff break without warning
> or notice.
>

Agreed. I'm afraid I was a bit too cavalier on changing the default
behavior. Mea culpa...

I think the right solution is to bring back the original default
behavior for now. Here's the patch I have so far:

http://git.samba.org/?p=jlayton/cifs.git;a=commit;h=1561a1a2c00faed82b6a65146ccaf908ea1a5552

...it's fairly straightforward. I've lightly tested it and it seems to
work correctly. Once that patch or something like it is in place, we
need to determine what we want the default to ultimately be.

There are a couple of problems with keeping this as the default
behavior:

A) these options get added automatically by /bin/mount for
unprivileged user mounts. Therefore it's not always a conscious
decision to use uid=. This means that when a mount is done by root, we
default to using the ownership info from the server. When the mount is
done by an unprivileged user, the ownership info is lost by default.
I'll argue that this behavior is unintuitive and that the clobbering of
ownership information from the server ought to be conditional on new
mount options for consistency's sake.

B) when ownership information is clobbered, the mode is not. So you're
taking a mode from the server, and blindly applying it to different
owners. Changing ownership of a file changes the meaning of the mode
bits. That's fine if you want it, but it should be a conscious decision
IMO. It should take more than just adding the uid= option to enable it.

IMO, uid=/gid= ought to be how we specify the default uid/gid for the
mount. That's what the ownership of the file should be when the
server doesn't provide one.

That said, I don't want to change the default behavior w/o some buy-in
by others here. Anyone have an argument to the contrary?

--
Jeff Layton <jlayton@...>
_______________________________________________
linux-cifs-client mailing list
linux-cifs-client@...
https://lists.samba.org/mailman/listinfo/linux-cifs-client

[Jeff Layton-2]

On Thu, 30 Jul 2009 09:04:58 -0400
Jeff Layton <jlayton@...> wrote:

> On Wed, 29 Jul 2009 19:31:43 +0100
> Tom Chiverton <bugzilla.kernel.org@...> wrote:
>
> > I tried out an RC of the 2.6.31 kernel recently, and discovered that under
> > that kernel the 'uid' option to mount.cifs has changed behaviour and the fix is
> > to add some new other option. At a guess, in almost all cases of giving a uid=
> > in fstab (different user UID locally and remotely) this change makes working
> > things stop.
> >
> > Based on discussion at http://bugzilla.kernel.org/show_bug.cgi?id=13861 
> > (initially, I thought this was a bug) it was suggested I bring the discussion
> > here to see what a wider set of people thought.
> >
> > Should we really not (at least) print a warning ('uid without forceuid, this
> > might be wrong as behaviour has changed [url]') ?
> > I spent a good while changing things on the server and client before I though
> > to roll back the kernel, and there's nothing obvious in Google (or the Samba
> > site) about this.
> >
> > Also, why don't we just default the new option (forceuid) such that mounts
> > keep working (i.e. the old behaviour), and then people who prefer the new
> > behaviour can enable it if they want ?
> >
> > That seems to make more sense to me, rather than changing long established
> > behaviour (it might well be for the better, but the side effects out weigh
> > this, I feel).
> > Swapping the default would create fstab 'spam' but that's still better than
> > 'printk spam', which in turn is better than having stuff break without warning
> > or notice.
> >
>
> Agreed. I'm afraid I was a bit too cavalier on changing the default
> behavior. Mea culpa...
>
> I think the right solution is to bring back the original default
> behavior for now. Here's the patch I have so far:
>
> http://git.samba.org/?p=jlayton/cifs.git;a=commit;h=1561a1a2c00faed82b6a65146ccaf908ea1a5552
>
> ...it's fairly straightforward. I've lightly tested it and it seems to
> work correctly. Once that patch or something like it is in place, we
> need to determine what we want the default to ultimately be.
>
> There are a couple of problems with keeping this as the default
> behavior:
>
> A) these options get added automatically by /bin/mount for
> unprivileged user mounts. Therefore it's not always a conscious
> decision to use uid=. This means that when a mount is done by root, we
> default to using the ownership info from the server. When the mount is
> done by an unprivileged user, the ownership info is lost by default.
> I'll argue that this behavior is unintuitive and that the clobbering of
> ownership information from the server ought to be conditional on new
> mount options for consistency's sake.
>
> B) when ownership information is clobbered, the mode is not. So you're
> taking a mode from the server, and blindly applying it to different
> owners. Changing ownership of a file changes the meaning of the mode
> bits. That's fine if you want it, but it should be a conscious decision
> IMO. It should take more than just adding the uid= option to enable it.
>
> IMO, uid=/gid= ought to be how we specify the default uid/gid for the
> mount. That's what the ownership of the file should be when the
> server doesn't provide one.
>
> That said, I don't want to change the default behavior w/o some buy-in
> by others here. Anyone have an argument to the contrary?
>

I've just sent a patch that should revert the change (and add options
to allow ownership info to be preserved for those who need it). It
would be helpful if you could verify that it works for you.

Thanks,
--
Jeff Layton <jlayton@...>
_______________________________________________
linux-cifs-client mailing list
linux-cifs-client@...
https://lists.samba.org/mailman/listinfo/linux-cifs-client

[Tom Chiverton-7]

On Thursday 30 July 2009 14:04:58 Jeff Layton wrote:
> I think the right solution is to bring back the original default
> behavior for now. Here's the patch I have so far:

Ace, look forward to the next RC kernel :-)
And now it's here, things are good again. Cheers Jeff.
--
Tom
Violating our airspace is an act of war, not friendship.
_______________________________________________
linux-cifs-client mailing list
linux-cifs-client@...
https://lists.samba.org/mailman/listinfo/linux-cifs-client