linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] configfs: initialize inode with owner
@ 2018-03-07  3:36 Gwendal Grignou
  2018-03-08  0:11 ` Gwendal Grignou
  0 siblings, 1 reply; 5+ messages in thread
From: Gwendal Grignou @ 2018-03-07  3:36 UTC (permalink / raw)
  To: jlbec; +Cc: linux-kernel, jorgelo

From: Sarthak Kukreti <sarthakkukreti@chromium.org>

Use standard helper to set inode owner when created from user space.
This is a noop when the owner is root.

Signed-off-by: Sarthak Kukreti <sarthakkukreti@chromium.org>
Signed-off-by: Gwendal Grignou <gwendal@chromium.org>
---
 fs/configfs/configfs_internal.h |  5 ++++-
 fs/configfs/inode.c             | 14 ++++++++------
 fs/configfs/mount.c             |  2 +-
 3 files changed, 13 insertions(+), 8 deletions(-)

diff --git a/fs/configfs/configfs_internal.h b/fs/configfs/configfs_internal.h
index b65d1ef532d5..772fce1fd222 100644
--- a/fs/configfs/configfs_internal.h
+++ b/fs/configfs/configfs_internal.h
@@ -68,7 +68,10 @@ extern struct kmem_cache *configfs_dir_cachep;
 
 extern int configfs_is_root(struct config_item *item);
 
-extern struct inode * configfs_new_inode(umode_t mode, struct configfs_dirent *, struct super_block *);
+extern struct inode *configfs_new_inode(umode_t mode,
+					struct configfs_dirent *sd,
+					struct inode *p_inode,
+					struct super_block *s);
 extern int configfs_create(struct dentry *, umode_t mode, void (*init)(struct inode *));
 
 extern int configfs_create_file(struct config_item *, const struct configfs_attribute *);
diff --git a/fs/configfs/inode.c b/fs/configfs/inode.c
index eae87575e681..93db05a2db1f 100644
--- a/fs/configfs/inode.c
+++ b/fs/configfs/inode.c
@@ -108,9 +108,11 @@ int configfs_setattr(struct dentry * dentry, struct iattr * iattr)
 	return error;
 }
 
-static inline void set_default_inode_attr(struct inode * inode, umode_t mode)
+static inline void set_default_inode_attr(struct inode *inode,
+					  struct inode *p_inode,
+					  umode_t mode)
 {
-	inode->i_mode = mode;
+	inode_init_owner(inode, p_inode, mode);
 	inode->i_atime = inode->i_mtime = inode->i_ctime = CURRENT_TIME;
 }
 
@@ -125,7 +127,7 @@ static inline void set_inode_attr(struct inode * inode, struct iattr * iattr)
 }
 
 struct inode *configfs_new_inode(umode_t mode, struct configfs_dirent *sd,
-				 struct super_block *s)
+				 struct inode *p_inode, struct super_block *s)
 {
 	struct inode * inode = new_inode(s);
 	if (inode) {
@@ -140,7 +142,7 @@ struct inode *configfs_new_inode(umode_t mode, struct configfs_dirent *sd,
 			 */
 			set_inode_attr(inode, sd->s_iattr);
 		} else
-			set_default_inode_attr(inode, mode);
+			set_default_inode_attr(inode, p_inode, mode);
 	}
 	return inode;
 }
@@ -190,11 +192,11 @@ int configfs_create(struct dentry * dentry, umode_t mode, void (*init)(struct in
 		return -EEXIST;
 
 	sd = dentry->d_fsdata;
-	inode = configfs_new_inode(mode, sd, dentry->d_sb);
+	p_inode = d_inode(dentry->d_parent);
+	inode = configfs_new_inode(mode, sd, p_inode, dentry->d_sb);
 	if (!inode)
 		return -ENOMEM;
 
-	p_inode = d_inode(dentry->d_parent);
 	p_inode->i_mtime = p_inode->i_ctime = CURRENT_TIME;
 	configfs_set_inode_lock_class(sd, inode);
 
diff --git a/fs/configfs/mount.c b/fs/configfs/mount.c
index a8f3b589a2df..23aede27c502 100644
--- a/fs/configfs/mount.c
+++ b/fs/configfs/mount.c
@@ -78,7 +78,7 @@ static int configfs_fill_super(struct super_block *sb, void *data, int silent)
 	sb->s_time_gran = 1;
 
 	inode = configfs_new_inode(S_IFDIR | S_IRWXU | S_IRUGO | S_IXUGO,
-				   &configfs_root, sb);
+				   &configfs_root, NULL, sb);
 	if (inode) {
 		inode->i_op = &configfs_root_inode_operations;
 		inode->i_fop = &configfs_dir_operations;
-- 
2.16.2.660.g709887971b-goog

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] configfs: initialize inode with owner
  2018-03-07  3:36 [PATCH] configfs: initialize inode with owner Gwendal Grignou
@ 2018-03-08  0:11 ` Gwendal Grignou
  2018-04-13 10:51   ` [PATCH] configfs: inherit file and directory owners Gwendal Grignou
  0 siblings, 1 reply; 5+ messages in thread
From: Gwendal Grignou @ 2018-03-08  0:11 UTC (permalink / raw)
  To: Joel Becker; +Cc: Linux Kernel, Jorge Lucangeli Obes, sarthakkukreti

This patch is not correct: When listed with ls, configfs attributes
have the uid/gid of the user who list them, but looks wrong.

 I will test it more and repost.

Gwendal.

On Tue, Mar 6, 2018 at 7:36 PM, Gwendal Grignou <gwendal@chromium.org> wrote:
> From: Sarthak Kukreti <sarthakkukreti@chromium.org>
>
> Use standard helper to set inode owner when created from user space.
> This is a noop when the owner is root.
>
> Signed-off-by: Sarthak Kukreti <sarthakkukreti@chromium.org>
> Signed-off-by: Gwendal Grignou <gwendal@chromium.org>
> ---
>  fs/configfs/configfs_internal.h |  5 ++++-
>  fs/configfs/inode.c             | 14 ++++++++------
>  fs/configfs/mount.c             |  2 +-
>  3 files changed, 13 insertions(+), 8 deletions(-)
>
> diff --git a/fs/configfs/configfs_internal.h b/fs/configfs/configfs_internal.h
> index b65d1ef532d5..772fce1fd222 100644
> --- a/fs/configfs/configfs_internal.h
> +++ b/fs/configfs/configfs_internal.h
> @@ -68,7 +68,10 @@ extern struct kmem_cache *configfs_dir_cachep;
>
>  extern int configfs_is_root(struct config_item *item);
>
> -extern struct inode * configfs_new_inode(umode_t mode, struct configfs_dirent *, struct super_block *);
> +extern struct inode *configfs_new_inode(umode_t mode,
> +                                       struct configfs_dirent *sd,
> +                                       struct inode *p_inode,
> +                                       struct super_block *s);
>  extern int configfs_create(struct dentry *, umode_t mode, void (*init)(struct inode *));
>
>  extern int configfs_create_file(struct config_item *, const struct configfs_attribute *);
> diff --git a/fs/configfs/inode.c b/fs/configfs/inode.c
> index eae87575e681..93db05a2db1f 100644
> --- a/fs/configfs/inode.c
> +++ b/fs/configfs/inode.c
> @@ -108,9 +108,11 @@ int configfs_setattr(struct dentry * dentry, struct iattr * iattr)
>         return error;
>  }
>
> -static inline void set_default_inode_attr(struct inode * inode, umode_t mode)
> +static inline void set_default_inode_attr(struct inode *inode,
> +                                         struct inode *p_inode,
> +                                         umode_t mode)
>  {
> -       inode->i_mode = mode;
> +       inode_init_owner(inode, p_inode, mode);
>         inode->i_atime = inode->i_mtime = inode->i_ctime = CURRENT_TIME;
>  }
>
> @@ -125,7 +127,7 @@ static inline void set_inode_attr(struct inode * inode, struct iattr * iattr)
>  }
>
>  struct inode *configfs_new_inode(umode_t mode, struct configfs_dirent *sd,
> -                                struct super_block *s)
> +                                struct inode *p_inode, struct super_block *s)
>  {
>         struct inode * inode = new_inode(s);
>         if (inode) {
> @@ -140,7 +142,7 @@ struct inode *configfs_new_inode(umode_t mode, struct configfs_dirent *sd,
>                          */
>                         set_inode_attr(inode, sd->s_iattr);
>                 } else
> -                       set_default_inode_attr(inode, mode);
> +                       set_default_inode_attr(inode, p_inode, mode);
>         }
>         return inode;
>  }
> @@ -190,11 +192,11 @@ int configfs_create(struct dentry * dentry, umode_t mode, void (*init)(struct in
>                 return -EEXIST;
>
>         sd = dentry->d_fsdata;
> -       inode = configfs_new_inode(mode, sd, dentry->d_sb);
> +       p_inode = d_inode(dentry->d_parent);
> +       inode = configfs_new_inode(mode, sd, p_inode, dentry->d_sb);
>         if (!inode)
>                 return -ENOMEM;
>
> -       p_inode = d_inode(dentry->d_parent);
>         p_inode->i_mtime = p_inode->i_ctime = CURRENT_TIME;
>         configfs_set_inode_lock_class(sd, inode);
>
> diff --git a/fs/configfs/mount.c b/fs/configfs/mount.c
> index a8f3b589a2df..23aede27c502 100644
> --- a/fs/configfs/mount.c
> +++ b/fs/configfs/mount.c
> @@ -78,7 +78,7 @@ static int configfs_fill_super(struct super_block *sb, void *data, int silent)
>         sb->s_time_gran = 1;
>
>         inode = configfs_new_inode(S_IFDIR | S_IRWXU | S_IRUGO | S_IXUGO,
> -                                  &configfs_root, sb);
> +                                  &configfs_root, NULL, sb);
>         if (inode) {
>                 inode->i_op = &configfs_root_inode_operations;
>                 inode->i_fop = &configfs_dir_operations;
> --
> 2.16.2.660.g709887971b-goog
>

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH] configfs: inherit file and directory owners
  2018-03-08  0:11 ` Gwendal Grignou
@ 2018-04-13 10:51   ` Gwendal Grignou
  2018-04-13 21:01     ` kbuild test robot
  2018-04-13 22:35     ` kbuild test robot
  0 siblings, 2 replies; 5+ messages in thread
From: Gwendal Grignou @ 2018-04-13 10:51 UTC (permalink / raw)
  To: jlbec; +Cc: drosen, linux-kernel, jorgelo

All entries in configfs are currently owned by root,
regardless of context. Instead, this preserves the
current ownership, allowing userspace to choose who
has permissions to configure the system through
any particular configfs subsystem.

This means anyone who can create a group will now
have the ability to create any groups inside of that
group.

Signed-off-by: Daniel Rosenberg <drosen@google.com>
Signed-off-by: Gwendal Grignou <gwendal@chromium.org>
---
 fs/configfs/inode.c | 38 +++++++++++++++++++++++++++++++-------
 1 file changed, 31 insertions(+), 7 deletions(-)

diff --git a/fs/configfs/inode.c b/fs/configfs/inode.c
index ad718e5e37bb..1b55374de953 100644
--- a/fs/configfs/inode.c
+++ b/fs/configfs/inode.c
@@ -54,6 +54,28 @@ static const struct inode_operations configfs_inode_operations ={
 	.setattr	= configfs_setattr,
 };
 
+static struct iattr *alloc_iattr(struct configfs_dirent *sd_parent,
+						struct configfs_dirent *sd)
+{
+	struct iattr *sd_iattr;
+
+	sd_iattr = kzalloc(sizeof(struct iattr), GFP_KERNEL);
+	if (!sd_iattr)
+		return NULL;
+	/* assign default attributes */
+	sd_iattr->ia_mode = sd->s_mode;
+	if (sd_parent && sd_parent->s_iattr) {
+		sd_iattr->ia_uid = sd_parent->s_iattr->ia_uid;
+		sd_iattr->ia_gid = sd_parent->s_iattr->ia_gid;
+	} else {
+		sd_iattr->ia_uid = GLOBAL_ROOT_UID;
+		sd_iattr->ia_gid = GLOBAL_ROOT_GID;
+	}
+	sd_iattr->ia_atime = sd_iattr->ia_mtime =
+			sd_iattr->ia_ctime = CURRENT_TIME;
+	return sd_iattr;
+}
+
 int configfs_setattr(struct dentry * dentry, struct iattr * iattr)
 {
 	struct inode * inode = d_inode(dentry);
@@ -68,15 +90,9 @@ int configfs_setattr(struct dentry * dentry, struct iattr * iattr)
 	sd_iattr = sd->s_iattr;
 	if (!sd_iattr) {
 		/* setting attributes for the first time, allocate now */
-		sd_iattr = kzalloc(sizeof(struct iattr), GFP_KERNEL);
+		sd_iattr = alloc_iattr(NULL, sd);
 		if (!sd_iattr)
 			return -ENOMEM;
-		/* assign default attributes */
-		sd_iattr->ia_mode = sd->s_mode;
-		sd_iattr->ia_uid = GLOBAL_ROOT_UID;
-		sd_iattr->ia_gid = GLOBAL_ROOT_GID;
-		sd_iattr->ia_atime = sd_iattr->ia_mtime =
-			sd_iattr->ia_ctime = current_time(inode);
 		sd->s_iattr = sd_iattr;
 	}
 	/* attributes were changed atleast once in past */
@@ -184,6 +200,7 @@ int configfs_create(struct dentry * dentry, umode_t mode, void (*init)(struct in
 	struct inode *inode = NULL;
 	struct configfs_dirent *sd;
 	struct inode *p_inode;
+	struct dentry *parent;
 
 	if (!dentry)
 		return -ENOENT;
@@ -192,6 +209,13 @@ int configfs_create(struct dentry * dentry, umode_t mode, void (*init)(struct in
 		return -EEXIST;
 
 	sd = dentry->d_fsdata;
+	parent = dget_parent(dentry);
+	if (parent && !sd->s_iattr) {
+		sd->s_iattr = alloc_iattr(parent->d_fsdata, sd);
+		if (!sd->s_iattr)
+			return -ENOMEM;
+	}
+	dput(parent);
 	inode = configfs_new_inode(mode, sd, dentry->d_sb);
 	if (!inode)
 		return -ENOMEM;
-- 
2.17.0.484.g0c8726318c-goog

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] configfs: inherit file and directory owners
  2018-04-13 10:51   ` [PATCH] configfs: inherit file and directory owners Gwendal Grignou
@ 2018-04-13 21:01     ` kbuild test robot
  2018-04-13 22:35     ` kbuild test robot
  1 sibling, 0 replies; 5+ messages in thread
From: kbuild test robot @ 2018-04-13 21:01 UTC (permalink / raw)
  To: Gwendal Grignou; +Cc: kbuild-all, jlbec, drosen, linux-kernel, jorgelo

[-- Attachment #1: Type: text/plain, Size: 1980 bytes --]

Hi Gwendal,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on linus/master]
[also build test ERROR on v4.16 next-20180413]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Gwendal-Grignou/configfs-inherit-file-and-directory-owners/20180414-041333
config: i386-randconfig-x014-201814 (attached as .config)
compiler: gcc-7 (Debian 7.3.0-1) 7.3.0
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

All errors (new ones prefixed by >>):

   fs/configfs/inode.c: In function 'alloc_iattr':
>> fs/configfs/inode.c:75:25: error: 'CURRENT_TIME' undeclared (first use in this function); did you mean 'MS_RELATIME'?
       sd_iattr->ia_ctime = CURRENT_TIME;
                            ^~~~~~~~~~~~
                            MS_RELATIME
   fs/configfs/inode.c:75:25: note: each undeclared identifier is reported only once for each function it appears in

vim +75 fs/configfs/inode.c

    56	
    57	static struct iattr *alloc_iattr(struct configfs_dirent *sd_parent,
    58							struct configfs_dirent *sd)
    59	{
    60		struct iattr *sd_iattr;
    61	
    62		sd_iattr = kzalloc(sizeof(struct iattr), GFP_KERNEL);
    63		if (!sd_iattr)
    64			return NULL;
    65		/* assign default attributes */
    66		sd_iattr->ia_mode = sd->s_mode;
    67		if (sd_parent && sd_parent->s_iattr) {
    68			sd_iattr->ia_uid = sd_parent->s_iattr->ia_uid;
    69			sd_iattr->ia_gid = sd_parent->s_iattr->ia_gid;
    70		} else {
    71			sd_iattr->ia_uid = GLOBAL_ROOT_UID;
    72			sd_iattr->ia_gid = GLOBAL_ROOT_GID;
    73		}
    74		sd_iattr->ia_atime = sd_iattr->ia_mtime =
  > 75				sd_iattr->ia_ctime = CURRENT_TIME;
    76		return sd_iattr;
    77	}
    78	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 27320 bytes --]

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] configfs: inherit file and directory owners
  2018-04-13 10:51   ` [PATCH] configfs: inherit file and directory owners Gwendal Grignou
  2018-04-13 21:01     ` kbuild test robot
@ 2018-04-13 22:35     ` kbuild test robot
  1 sibling, 0 replies; 5+ messages in thread
From: kbuild test robot @ 2018-04-13 22:35 UTC (permalink / raw)
  To: Gwendal Grignou; +Cc: kbuild-all, jlbec, drosen, linux-kernel, jorgelo

[-- Attachment #1: Type: text/plain, Size: 1986 bytes --]

Hi Gwendal,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on linus/master]
[also build test ERROR on v4.16 next-20180413]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Gwendal-Grignou/configfs-inherit-file-and-directory-owners/20180414-041333
config: x86_64-randconfig-x013-201814 (attached as .config)
compiler: gcc-7 (Debian 7.3.0-1) 7.3.0
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All errors (new ones prefixed by >>):

   fs/configfs/inode.c: In function 'alloc_iattr':
>> fs/configfs/inode.c:75:25: error: 'CURRENT_TIME' undeclared (first use in this function); did you mean 'CURRENT_MASK'?
       sd_iattr->ia_ctime = CURRENT_TIME;
                            ^~~~~~~~~~~~
                            CURRENT_MASK
   fs/configfs/inode.c:75:25: note: each undeclared identifier is reported only once for each function it appears in

vim +75 fs/configfs/inode.c

    56	
    57	static struct iattr *alloc_iattr(struct configfs_dirent *sd_parent,
    58							struct configfs_dirent *sd)
    59	{
    60		struct iattr *sd_iattr;
    61	
    62		sd_iattr = kzalloc(sizeof(struct iattr), GFP_KERNEL);
    63		if (!sd_iattr)
    64			return NULL;
    65		/* assign default attributes */
    66		sd_iattr->ia_mode = sd->s_mode;
    67		if (sd_parent && sd_parent->s_iattr) {
    68			sd_iattr->ia_uid = sd_parent->s_iattr->ia_uid;
    69			sd_iattr->ia_gid = sd_parent->s_iattr->ia_gid;
    70		} else {
    71			sd_iattr->ia_uid = GLOBAL_ROOT_UID;
    72			sd_iattr->ia_gid = GLOBAL_ROOT_GID;
    73		}
    74		sd_iattr->ia_atime = sd_iattr->ia_mtime =
  > 75				sd_iattr->ia_ctime = CURRENT_TIME;
    76		return sd_iattr;
    77	}
    78	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 29326 bytes --]

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2018-04-13 22:36 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-03-07  3:36 [PATCH] configfs: initialize inode with owner Gwendal Grignou
2018-03-08  0:11 ` Gwendal Grignou
2018-04-13 10:51   ` [PATCH] configfs: inherit file and directory owners Gwendal Grignou
2018-04-13 21:01     ` kbuild test robot
2018-04-13 22:35     ` kbuild test robot

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).