Re: [PATCH] Restrict mtab umask

[Karel Zak <kzak@xxxxxxxxxx>]

 Hi Alexey,

On Thu, Apr 17, 2008 at 03:23:29PM +0400, Alexey Gladkov wrote:
> This is openwall linux patch to restrict /etc/mtab permissions.

 Why do you need any extra restriction?

> Signed-off-by: Alexey Gladkov <legion@xxxxxxxxxxxx>
> ---
> mount/mount.c        |    2 +-
> mount/mount_mntent.c |    2 +-
> mount/umount.c       |    2 +-
> 3 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/mount/mount.c b/mount/mount.c
> index bed792d..297a8ad 100644
> --- a/mount/mount.c
> +++ b/mount/mount.c
> @@ -1826,7 +1826,7 @@ main(int argc, char *argv[]) {
> 	if ((p = strrchr(progname, '/')) != NULL)
> 		progname = p+1;
>
> -	umask(022);
> +	umask(077);

 Please, don't touch umask in the mount(8) at all.. there is a
 significant side effect:

 without your patch:

         # mount -t vfat  /dev/loop0 /mnt/test
         # ls -lad /mnt/test
         drwxr-xr-x 2 root root 16384 1970-01-01 01:00 /mnt/test

 with your patch:

        # mount -t vfat /dev/loop0 /mnt/test
        # ls -lad /mnt/test
        drwx------ 2 root root 16384 1970-01-01 01:00 /mnt/test

 ... because the default umask for some filesystems is the umask of the
 mount process.

> mntFILE *
> my_setmntent (const char *file, char *mode) {
> 	mntFILE *mfp = xmalloc(sizeof(*mfp));
> -	mode_t old_umask = umask(077);
> +	mode_t old_umask = umask(033);
>
> 	mfp->mntent_fp = fopen(file, mode);
> 	umask(old_umask);

 This change has almost zero effect because update_mtab() uses
 fchmod(S_IRUSR|S_IWUSR|S_IRGRP|S_IROTH).

 I really don't want to change /etc/mtab permissions. From my point
 of view this change is regression. Many other programs or scripts
 read this file.

    Karel

-- 
 Karel Zak  <kzak@xxxxxxxxxx>
--
To unsubscribe from this list: send the line "unsubscribe util-linux-ng" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html