devtmpfs default permissions

devtmpfs default permissions

[Mark Rosenstand]

Hi,

I've been using the devtmpfs patch for 2.6.31 and recently mainline
2.6.32-rc5.  I've noticed that /dev now have default tmpfs permissions,
i.e. 1777.

CONFIG_DEVTMPFS=y
CONFIG_DEVTMPFS_MOUNT=y

One can chmod the directory, but would it not make sense to use a more
less permissive mode (e.g. 0755) by default?

Thanks for working out this feature, the "just works" of devfs together
with the configurability of udev - love it :)

Re: devtmpfs default permissions

[Greg KH]

On Thu, Oct 29, 2009 at 03:02:50PM +0100, Mark Rosenstand wrote:
> Hi,
> 
> I've been using the devtmpfs patch for 2.6.31 and recently mainline
> 2.6.32-rc5.  I've noticed that /dev now have default tmpfs permissions,
> i.e. 1777.
> 
> CONFIG_DEVTMPFS=y
> CONFIG_DEVTMPFS_MOUNT=y
> 
> One can chmod the directory, but would it not make sense to use a more
> less permissive mode (e.g. 0755) by default?

I think the patch sent by Kay to the linux-kernel mailing list yesterday
will solve this problem.  It uses the default umask instead.

Can you test it out to verify this?

> Thanks for working out this feature, the "just works" of devfs together
> with the configurability of udev - love it :)

Glad you like it.

thanks,

greg k-h

Re: devtmpfs default permissions

[Kay Sievers]

On Thursday, October 29, 2009, Greg KH <greg@kroah.com> wrote:
> On Thu, Oct 29, 2009 at 03:02:50PM +0100, Mark Rosenstand wrote:
>>
>> I've been using the devtmpfs patch for 2.6.31 and recently mainline
>> 2.6.32-rc5.  I've noticed that /dev now have default tmpfs permissions,
>> i.e. 1777.
>>
>> One can chmod the directory, but would it not make sense to use a more
>> less permissive mode (e.g. 0755) by default?

Yeah, sounds good.

> I think the patch sent by Kay to the linux-kernel mailing list yesterday
> will solve this problem.  It uses the default umask instead.
>
> Can you test it out to verify this?

I'll prepare a patch to fix that, we need to pass "mode\a55" to the
mount command, to set the tmpfs root directory mode.

Thanks,
Kay

Re: devtmpfs default permissions

[Mark Rosenstand]

On Thu, 2009-10-29 at 18:12 +0100, Kay Sievers wrote:
> On Thursday, October 29, 2009, Greg KH <greg@kroah.com> wrote:
> > On Thu, Oct 29, 2009 at 03:02:50PM +0100, Mark Rosenstand wrote:
> >>
> >> I've been using the devtmpfs patch for 2.6.31 and recently mainline
> >> 2.6.32-rc5.  I've noticed that /dev now have default tmpfs permissions,
> >> i.e. 1777.
> >>
> >> One can chmod the directory, but would it not make sense to use a more
> >> less permissive mode (e.g. 0755) by default?
> 
> Yeah, sounds good.
> 
> > I think the patch sent by Kay to the linux-kernel mailing list yesterday
> > will solve this problem.  It uses the default umask instead.
> >
> > Can you test it out to verify this?

Default umask would probably be even better than a hardcoded one. Just
tried latest git, seems to still get 1777 permissions.

> I'll prepare a patch to fix that, we need to pass "mode\a55" to the
> mount command, to set the tmpfs root directory mode.

Thanks guys, rock on! :)

Re: devtmpfs default permissions

[Kay Sievers]

On Fri, 2009-10-30 at 00:53 +0100, Mark Rosenstand wrote:
> On Thu, 2009-10-29 at 18:12 +0100, Kay Sievers wrote:
> > On Thursday, October 29, 2009, Greg KH <greg@kroah.com> wrote:
> > > On Thu, Oct 29, 2009 at 03:02:50PM +0100, Mark Rosenstand wrote:
> > >>
> > >> I've been using the devtmpfs patch for 2.6.31 and recently mainline
> > >> 2.6.32-rc5.  I've noticed that /dev now have default tmpfs permissions,
> > >> i.e. 1777.
> > >>
> > >> One can chmod the directory, but would it not make sense to use a more
> > >> less permissive mode (e.g. 0755) by default?
> > 
> > Yeah, sounds good.
> > 
> > > I think the patch sent by Kay to the linux-kernel mailing list yesterday
> > > will solve this problem.  It uses the default umask instead.
> > >
> > > Can you test it out to verify this?
> 
> Default umask would probably be even better than a hardcoded one. Just
> tried latest git, seems to still get 1777 permissions.
> 
> > I'll prepare a patch to fix that, we need to pass "mode\a55" to the
> > mount command, to set the tmpfs root directory mode.
> 
> Thanks guys, rock on! :)

Thanks! This works for me.


From: Kay Sievers <kay.sievers@vrfy.org>
Subject: Driver-Core: devtmpfs - set root directory mode to 0755

Signed-off-by: Kay Sievers <kay.sievers@vrfy.org>
---
 drivers/base/devtmpfs.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

--- a/drivers/base/devtmpfs.c
+++ b/drivers/base/devtmpfs.c
@@ -359,7 +359,7 @@ int __init devtmpfs_init(void)
 		return err;
 	}
 
-	mnt = kern_mount(&dev_fs_type);
+	mnt = kern_mount_data(&dev_fs_type, "mode\a55");
 	if (IS_ERR(mnt)) {
 		err = PTR_ERR(mnt);
 		printk(KERN_ERR "devtmpfs: unable to create devtmpfs %i\n", err);

Re: devtmpfs default permissions

[Mark Rosenstand]

On Fri, 2009-10-30 at 12:48 +0100, Kay Sievers wrote:
> On Fri, 2009-10-30 at 00:53 +0100, Mark Rosenstand wrote:
> > On Thu, 2009-10-29 at 18:12 +0100, Kay Sievers wrote:
> > > On Thursday, October 29, 2009, Greg KH <greg@kroah.com> wrote:
> > > > On Thu, Oct 29, 2009 at 03:02:50PM +0100, Mark Rosenstand wrote:
> > > >>
> > > >> I've been using the devtmpfs patch for 2.6.31 and recently mainline
> > > >> 2.6.32-rc5.  I've noticed that /dev now have default tmpfs permissions,
> > > >> i.e. 1777.
> > > >>
> > > >> One can chmod the directory, but would it not make sense to use a more
> > > >> less permissive mode (e.g. 0755) by default?
> > > 
> > > Yeah, sounds good.
> > > 
> > > > I think the patch sent by Kay to the linux-kernel mailing list yesterday
> > > > will solve this problem.  It uses the default umask instead.
> > > >
> > > > Can you test it out to verify this?
> > 
> > Default umask would probably be even better than a hardcoded one. Just
> > tried latest git, seems to still get 1777 permissions.
> > 
> > > I'll prepare a patch to fix that, we need to pass "mode\a55" to the
> > > mount command, to set the tmpfs root directory mode.
> > 
> > Thanks guys, rock on! :)
> 
> Thanks! This works for me.
> 
> 
> From: Kay Sievers <kay.sievers@vrfy.org>
> Subject: Driver-Core: devtmpfs - set root directory mode to 0755
> 
> Signed-off-by: Kay Sievers <kay.sievers@vrfy.org>
> ---
>  drivers/base/devtmpfs.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> --- a/drivers/base/devtmpfs.c
> +++ b/drivers/base/devtmpfs.c
> @@ -359,7 +359,7 @@ int __init devtmpfs_init(void)
>  		return err;
>  	}
>  
> -	mnt = kern_mount(&dev_fs_type);
> +	mnt = kern_mount_data(&dev_fs_type, "mode\a55");
>  	if (IS_ERR(mnt)) {
>  		err = PTR_ERR(mnt);
>  		printk(KERN_ERR "devtmpfs: unable to create devtmpfs %i\n", err);

Works here too. Getting this on the console, though:

tmpfs: No value for mount option 'mode'

But hey, it works. Thanks :)

Re: devtmpfs default permissions

[Kay Sievers]

On Tue, Nov 3, 2009 at 02:24, Mark Rosenstand <rosenstand@gmail.com> wrote:
> Works here too. Getting this on the console, though:
>
> tmpfs: No value for mount option 'mode'
>
> But hey, it works. Thanks :)

Yeah, the tmpfs code mangles *data, and as we get a single superblock,
remount is called during mount, and the tmpfs code tries to parse
*data again which it destroyed in the first run. It's harmless and
needs to be fixed, but I'm not sure which piece to fix at the moment.
:)

Thanks,
Kay

Re: devtmpfs default permissions

[Kay Sievers]

On Wed, 2009-11-04 at 16:25 +0100, Kay Sievers wrote:
> On Tue, Nov 3, 2009 at 02:24, Mark Rosenstand <rosenstand@gmail.com> wrote:
> > Works here too. Getting this on the console, though:
> >
> > tmpfs: No value for mount option 'mode'
> >
> > But hey, it works. Thanks :)
> 
> Yeah, the tmpfs code mangles *data, and as we get a single superblock,
> remount is called during mount, and the tmpfs code tries to parse
> *data again which it destroyed in the first run. It's harmless and
> needs to be fixed, but I'm not sure which piece to fix at the moment.
> :)

This seems to work here.

Thanks,
Kay


From: Kay Sievers <kay.sievers@vrfy.org>
Subject: tmpfs: zap options after mangling them during parsing

Zap options list we have destroyed during parsing, to prevent
errors on repeated calls, like in the case we do kern_mount()
which might do an implicit remount() call.

Cc: Hugh Dickins <hugh.dickins@tiscali.co.uk>
Signed-off-by: Kay Sievers <kay.sievers@vrfy.org>
---
 mm/shmem.c |   26 ++++++++++++++++++--------
 1 file changed, 18 insertions(+), 8 deletions(-)

--- a/mm/shmem.c
+++ b/mm/shmem.c
@@ -2147,22 +2147,26 @@ static const struct export_operations sh
 static int shmem_parse_options(char *options, struct shmem_sb_info *sbinfo,
 			       bool remount)
 {
-	char *this_char, *value, *rest;
+	char *option, *value, *this_char, *rest;
 
-	while (options != NULL) {
-		this_char = options;
+	if (!options || !options[0])
+		return 0;
+
+	option = options;
+	while (option != NULL) {
+		this_char = option;
 		for (;;) {
 			/*
 			 * NUL-terminate this option: unfortunately,
 			 * mount options form a comma-separated list,
 			 * but mpol's nodelist may also contain commas.
 			 */
-			options = strchr(options, ',');
-			if (options = NULL)
+			option = strchr(option, ',');
+			if (option = NULL)
 				break;
-			options++;
-			if (!isdigit(*options)) {
-				options[-1] = '\0';
+			option++;
+			if (!isdigit(*option)) {
+				option[-1] = '\0';
 				break;
 			}
 		}
@@ -2225,6 +2229,12 @@ static int shmem_parse_options(char *opt
 			return 1;
 		}
 	}
+	/*
+	 * Zap options list we have destroyed during parsing, to prevent
+	 * errors on repeated calls, like in the case we do kern_mount()
+	 * which might do an implicit remount() call.
+	 */
+	options[0] = '\0';
 	return 0;
 
 bad_val:

Re: devtmpfs default permissions

[Greg KH]

On Thu, Nov 05, 2009 at 08:42:03PM +0100, Kay Sievers wrote:
> On Wed, 2009-11-04 at 16:25 +0100, Kay Sievers wrote:
> > On Tue, Nov 3, 2009 at 02:24, Mark Rosenstand <rosenstand@gmail.com> wrote:
> > > Works here too. Getting this on the console, though:
> > >
> > > tmpfs: No value for mount option 'mode'
> > >
> > > But hey, it works. Thanks :)
> > 
> > Yeah, the tmpfs code mangles *data, and as we get a single superblock,
> > remount is called during mount, and the tmpfs code tries to parse
> > *data again which it destroyed in the first run. It's harmless and
> > needs to be fixed, but I'm not sure which piece to fix at the moment.
> > :)
> 
> This seems to work here.

Care to run it by Hugh?

thanks,

greg k-h

Re: devtmpfs default permissions

[Kay Sievers]

On Thu, Nov 5, 2009 at 22:01, Greg KH <greg@kroah.com> wrote:
> On Thu, Nov 05, 2009 at 08:42:03PM +0100, Kay Sievers wrote:
>> On Wed, 2009-11-04 at 16:25 +0100, Kay Sievers wrote:
>> > On Tue, Nov 3, 2009 at 02:24, Mark Rosenstand <rosenstand@gmail.com> wrote:
>> > > Works here too. Getting this on the console, though:
>> > >
>> > > tmpfs: No value for mount option 'mode'
>> > >
>> > > But hey, it works. Thanks :)
>> >
>> > Yeah, the tmpfs code mangles *data, and as we get a single superblock,
>> > remount is called during mount, and the tmpfs code tries to parse
>> > *data again which it destroyed in the first run. It's harmless and
>> > needs to be fixed, but I'm not sure which piece to fix at the moment.
>> > :)
>>
>> This seems to work here.
>
> Care to run it by Hugh?

Oh, I sent another possible, simpler fix to fsdevel:
  http://article.gmane.org/gmane.linux.file-systems/36535

Let's wait for a response from them.

Thanks,
Kay