[PATCH] umask in POSIX message queues

[PATCH] umask in POSIX message queues

[Krzysztof Benedyczak]

Hi

All kernels (form 2.6.6) ignore umask when creating new queues via
mq_open (when creating with open() on mqueue fs it is ok of course).
According to specification this a bug. The following trivial patch fixes
this. It should apply cleanly to any current kernel. Please apply.


Signed-off-by: Krzysztof Benedyczak <golbi@mat.uni.torun.pl>


--- linux-2.6.14-rc2/ipc/mqueue.c.orig  2005-09-25 18:52:29.000000000 +0200
+++ linux-2.6.14-rc2/ipc/mqueue.c       2005-09-26 11:44:41.000000000 +0200
@@ -278,6 +278,8 @@ static int mqueue_create(struct inode *d
        queues_count++;
        spin_unlock(&mq_lock);

+       mode &= ~current->fs->umask;
+
        inode = mqueue_get_inode(dir->i_sb, mode, attr);
        if (!inode) {
                error = -ENOMEM;

Re: [PATCH] umask in POSIX message queues

[Linus Torvalds]

On Mon, 26 Sep 2005, Krzysztof Benedyczak wrote:
> 
> All kernels (form 2.6.6) ignore umask when creating new queues via
> mq_open (when creating with open() on mqueue fs it is ok of course).
> According to specification this a bug. The following trivial patch fixes
> this. It should apply cleanly to any current kernel. Please apply.

As far as I can tell, the VFS layer should have done this for us already, 
with code like

		...
                if (!IS_POSIXACL(dir->d_inode))
                        mode &= ~current->fs->umask;
                error = vfs_create(dir->d_inode, path.dentry, mode, nd);
		...

in fs/namei.c (open_namei()).

Which path did you come through that didn't do this? That would be the
real bug, I suspect..

		Linus

Re: [PATCH] umask in POSIX message queues

[Krzysztof Benedyczak]

On Mon, 26 Sep 2005, Linus Torvalds wrote:

> As far as I can tell, the VFS layer should have done this for us already,
> with code like
>
> 		...
>                 if (!IS_POSIXACL(dir->d_inode))
>                         mode &= ~current->fs->umask;
>                 error = vfs_create(dir->d_inode, path.dentry, mode, nd);
> 		...
>
> in fs/namei.c (open_namei()).
>
> Which path did you come through that didn't do this? That would be the
> real bug, I suspect..

As I noted when creating mqueues with sys_open() the umask is set
correctly just by the code you pointed out. But sys_mq_open() doesn't use
open_namei() nor filp_open(); the reason is extra data - mq_attr - that
must be passed to real mqueue creating code. So the invocation path is:
sys_mq_open() -> do_create() -> vfs_create() -> (vfs create handler)
mqueue_create().

After rereading it I think that the better place for the line setting
umask is do_create() function as it will be on the same level as
open_namei(). I hope this change will clarify things.

If this make sense I'll send a patch.

Best regards
Krzysiek

Re: [PATCH] umask in POSIX message queues

[Linus Torvalds]

On Tue, 27 Sep 2005, Krzysztof Benedyczak wrote:
> 
> After rereading it I think that the better place for the line setting
> umask is do_create() function as it will be on the same level as
> open_namei(). I hope this change will clarify things.
> 
> If this make sense I'll send a patch.

Yes, that makes more sense.

Please do send a tested patch,

		Linus

Re: [PATCH] umask in POSIX message queues

[Krzysztof Benedyczak]

On Tue, 27 Sep 2005, Linus Torvalds wrote:

> On Tue, 27 Sep 2005, Krzysztof Benedyczak wrote:
> >
> > After rereading it I think that the better place for the line setting
> > umask is do_create() function as it will be on the same level as
> > open_namei(). I hope this change will clarify things.
> >
> > If this make sense I'll send a patch.
>
> Yes, that makes more sense.
>
> Please do send a tested patch,

Setting umask moved to do_create and tested once again.
Krzysiek

Signed-off-by: Krzysztof Benedyczak <golbi@mat.uni.torun.pl>

--- linux-2.6.14-rc2/ipc/mqueue.c.orig  2005-09-25 18:52:29.000000000 +0200
+++ linux-2.6.14-rc2/ipc/mqueue.c       2005-09-27 22:12:41.692903624 +0200
@@ -611,6 +611,7 @@ static struct file *do_create(struct den
                dentry->d_fsdata = &attr;
        }

+       mode &= ~current->fs->umask;
        ret = vfs_create(dir->d_inode, dentry, mode, NULL);
        dentry->d_fsdata = NULL;
        if (ret)