2005-04-29 21:13:51

by Steve French

[permalink] [raw]
Subject: Re: [PATCH] cifs: handle termination of cifs oplockd kernel thread

> does and who revied that? Things like that don't have a business in the
> kernel, and certainly not as ioctl.

Other filesystems such as smbfs had an ioctl that returned the uid of
the mounter which they used (in the smbfs case in smbumount). This was
required by the unmount helper to determine if the unmount would allow a
user to unmount a particular mount that they mounted. Unlike in the
case of mount, for unmount you can not use the owner uid of the mount
target to tell who mounted that mount. I had not received any better
suggestions as to how to address it. I had proposed various
alternatives - exporting in in /proc/mounts e.g.

As we try to gradually obsolete smbfs, this came up with various users
(there was even a bugzilla bug opened for adding it) who said that they
need the ability to unmount their own mounts for network filesystems
without using /etc/fstab. Unfortunately for network filesytsems,
unlike local filesystems, it is impractical to put every possible mount
target in /etc/fstab since servers get renamed and the universe of
possible cifs mount targets for a user is large.

There seemed only three alternatives -
1) mimic the smbfs ioctl - as can be seen from smbfs and smbumount
source this has portability problems because apparently there is no
guarantee that uid_t is the same size in kernel and in userspace - smbfs
actually has two ioctls for different sizes of uid field - this seemed
like a bad idea
2) export the uid in /proc/mounts - same problem as above
3) call into the kernel to see if current matches the uid of the mounter
- this has no 16/32/64 bit uid portability issues since the check is
made in kernel

If there is a better way to achieve these goals I would like to know - I
had not gotten any feedback on a better way. Although I am not a fan
of ioctls, this is as simple as they get and I checked for overlaps in
the ioctl numbers and the utility checks to make sure it is only invoked
if the filesystem magic number matches CIFS's magic number and no parms
are passed or returned so it is quite safe.

Of course I would have preferred that this facility were built into the
kernel via a syscall so the same approach could be put in umount itself
instead of in a umount helper.


2005-04-29 21:37:08

by Christoph Hellwig

[permalink] [raw]
Subject: Re: [PATCH] cifs: handle termination of cifs oplockd kernel thread

On Fri, Apr 29, 2005 at 04:09:09PM -0500, Steve French wrote:
> > does and who revied that? Things like that don't have a business in the
> > kernel, and certainly not as ioctl.
>
> Other filesystems such as smbfs had an ioctl that returned the uid of
> the mounter which they used (in the smbfs case in smbumount). This was
> required by the unmount helper to determine if the unmount would allow a
> user to unmount a particular mount that they mounted. Unlike in the
> case of mount, for unmount you can not use the owner uid of the mount
> target to tell who mounted that mount. I had not received any better
> suggestions as to how to address it. I had proposed various
> alternatives - exporting in in /proc/mounts e.g.

exporting the uid using the show_options superblock methods sounds like
a much better option.

> As we try to gradually obsolete smbfs, this came up with various users
> (there was even a bugzilla bug opened for adding it) who said that they
> need the ability to unmount their own mounts for network filesystems
> without using /etc/fstab. Unfortunately for network filesytsems,
> unlike local filesystems, it is impractical to put every possible mount
> target in /etc/fstab since servers get renamed and the universe of
> possible cifs mount targets for a user is large.

Do you use the same suid wrapper hack for mounts as fuse? Maybe you
should chime in on that thread so we can find a proper solution.

>
> There seemed only three alternatives -
> 1) mimic the smbfs ioctl - as can be seen from smbfs and smbumount
> source this has portability problems because apparently there is no
> guarantee that uid_t is the same size in kernel and in userspace - smbfs
> actually has two ioctls for different sizes of uid field - this seemed
> like a bad idea
> 2) export the uid in /proc/mounts - same problem as above

No. /proc/self/mounts is an ASCII format, so there's no problem with
differemt sizes.

2005-04-29 22:21:00

by Steve French

[permalink] [raw]
Subject: Re: [PATCH] cifs: handle termination of cifs oplockd kernel thread

Christoph Hellwig wrote:

>On Fri, Apr 29, 2005 at 04:09:09PM -0500, Steve French wrote:
>
>
>>>does and who revied that? Things like that don't have a business in the
>>>kernel, and certainly not as ioctl.
>>>
>>>
>>Other filesystems such as smbfs had an ioctl that returned the uid of
>>the mounter which they used (in the smbfs case in smbumount). This was
>>required by the unmount helper to determine if the unmount would allow a
>>user to unmount a particular mount that they mounted. Unlike in the
>>case of mount, for unmount you can not use the owner uid of the mount
>>target to tell who mounted that mount. I had not received any better
>>suggestions as to how to address it. I had proposed various
>>alternatives - exporting in in /proc/mounts e.g.
>>
>>
>
>exporting the uid using the show_options superblock methods sounds like
>a much better option.
>
>
>

>No. /proc/self/mounts is an ASCII format, so there's no problem with
>differemt sizes.
>
>
>
>

I agree that it would work for most cases [today, in 2.6 Linux] - but I
really feel uncomfortable introducing a user space / kernel space
dependency on the size of a field where none is needed - I am especially
nervous because I can see from the Samba change logs that:
1) historically the size of this field changed
2) other operating systems also have either increased the size of uid
(as MacOS e.g.) or mapped it (as Windows does) - to accomodate the
needed concept of UUID (in large networks the current uid is too small)

Ideally I would have liked a general kernel call to be able to answer
the question "Does the current security context match that of the
mounter?" because with SELinux, and the need to increase the size of
uid or somehow work around it for distributed systems, it is hard for
user space code to take something opaque (the thing that showmounts
returns) and map it to what libc returns as the uid for current -
otherwise it would be impossible for user space code to guarantee that
it will match the kernel code, but this is so trivial, and has no
sideeffects so in the interim this seems safer.




2005-05-11 09:00:02

by Christoph Hellwig

[permalink] [raw]
Subject: Re: [PATCH] cifs: handle termination of cifs oplockd kernel thread

On Fri, Apr 29, 2005 at 05:20:37PM -0500, Steve French wrote:
> I agree that it would work for most cases [today, in 2.6 Linux] - but I
> really feel uncomfortable introducing a user space / kernel space
> dependency on the size of a field where none is needed - I am especially
> nervous because I can see from the Samba change logs that:

Please listen, I said you should export it in /proc/<pid>/mounts, which is
an ASCII interface and any half-sane parser does not depend on the width
of the field in the kernel.

Can we please get rid of the broken ioctl now so it doesn't become part
of the ABI and you'll add the trivial output to /proc/<pid>/mounts?

2005-05-11 18:20:44

by Steve French

[permalink] [raw]
Subject: Re: [PATCH] cifs: handle termination of cifs oplockd kernel thread

Christoph Hellwig wrote:

>
>should export it in /proc/<pid>/mounts, which is
>an ASCII interface and any half-sane parser does not depend on the width
>of the field in the kernel.
>
>Can we please get rid of the broken ioctl now so it doesn't become part
>of the ABI and you'll add the trivial output to /proc/<pid>/mounts?
>
>
>
>
OK - why don't we just add this (ie the ioctl removal) to the patch

[PATCH] unprivileged mount/umount

of Miklos et al, since that removes the need to modify showmounts (and
avoids any name collision/confusion
with the existing meaning of the mount option "uid" ie as the default
uid to use for files on the system when
mounting to servers which can not return inode owners as uids).

On another topic relating to ioctls, various people have suggested
adding an ioctl to add a table to optionally map file owner (uid / gid
mapping tables) on remote filesystems. Although this is easy enough to
do for the case of CIFS, this seems like a function (loading the table)
that could be done via /proc or perhaps even sysfs. Is there are
precedent for doing this on Linux? Googling I see various examples where
NFS client on other platforms (not Linux) have done something vaguely
similar. NFSv4 uses an upcall for this (although they are mapping
slightly differently since they now receive a fully qualified username
and have to map this to a loca uid, rather than getting a remote uid to
local uid as earlier nfs did). The general issue is that when mounting
to multiple Unix/Linux servers (especially in different domains), unlike
in Windows (or perhaps MacOS), similar users are defined with different
uids, and there are cases where mapping uids/gids or ranges of uids/gids
from that returned from the server would be helpful. The mapping table
would have to hang off the tree connection or the SMB session for the
case of CIFS but I would rather not use an ioctl to load it, yet if the
table ever got big, I would prefer not to use /proc either. Is there a
recommended approach.

2005-05-16 09:34:29

by Christoph Hellwig

[permalink] [raw]
Subject: Re: [PATCH] cifs: handle termination of cifs oplockd kernel thread

On Wed, May 11, 2005 at 01:19:19PM -0500, Steve French wrote:
> OK - why don't we just add this (ie the ioctl removal) to the patch
>
> [PATCH] unprivileged mount/umount
>
> of Miklos et al, since that removes the need to modify showmounts (and
> avoids any name collision/confusion
> with the existing meaning of the mount option "uid" ie as the default
> uid to use for files on the system when
> mounting to servers which can not return inode owners as uids).

I think that would be best. It still needs a little work first.

> On another topic relating to ioctls, various people have suggested
> adding an ioctl to add a table to optionally map file owner (uid / gid
> mapping tables) on remote filesystems. Although this is easy enough to
> do for the case of CIFS, this seems like a function (loading the table)
> that could be done via /proc or perhaps even sysfs. Is there are
> precedent for doing this on Linux?

I don't think that mapping should happen in kernelspace. It would
be nice if you could share that with nfs, maybe even generalizing the
nfsv4 one.