From 5cf3bb0d3a2d0de94f3f551f0e4211068818aabf Mon Sep 17 00:00:00 2001 From: Christoph Hellwig Date: Mon, 13 Sep 2021 07:41:12 +0200 Subject: sysfs: split out binary attribute handling from sysfs_add_file_mode_ns Split adding binary attributes into a separate handler instead of overloading sysfs_add_file_mode_ns. Acked-by: Christian Brauner Acked-by: Tejun Heo Signed-off-by: Christoph Hellwig Link: https://lore.kernel.org/r/20210913054121.616001-5-hch@lst.de Signed-off-by: Greg Kroah-Hartman --- fs/sysfs/file.c | 120 ++++++++++++++++++++++++++++++------------------------- fs/sysfs/group.c | 15 ++++--- fs/sysfs/sysfs.h | 8 ++-- 3 files changed, 78 insertions(+), 65 deletions(-) (limited to 'fs') diff --git a/fs/sysfs/file.c b/fs/sysfs/file.c index d019d6ac6ad0..f737bd61f71b 100644 --- a/fs/sysfs/file.c +++ b/fs/sysfs/file.c @@ -255,59 +255,73 @@ static const struct kernfs_ops sysfs_bin_kfops_mmap = { }; int sysfs_add_file_mode_ns(struct kernfs_node *parent, - const struct attribute *attr, bool is_bin, - umode_t mode, kuid_t uid, kgid_t gid, const void *ns) + const struct attribute *attr, umode_t mode, kuid_t uid, + kgid_t gid, const void *ns) { + struct kobject *kobj = parent->priv; + const struct sysfs_ops *sysfs_ops = kobj->ktype->sysfs_ops; struct lock_class_key *key = NULL; const struct kernfs_ops *ops; struct kernfs_node *kn; - loff_t size; - - if (!is_bin) { - struct kobject *kobj = parent->priv; - const struct sysfs_ops *sysfs_ops = kobj->ktype->sysfs_ops; - - /* every kobject with an attribute needs a ktype assigned */ - if (WARN(!sysfs_ops, KERN_ERR - "missing sysfs attribute operations for kobject: %s\n", - kobject_name(kobj))) - return -EINVAL; - - if (sysfs_ops->show && sysfs_ops->store) { - if (mode & SYSFS_PREALLOC) - ops = &sysfs_prealloc_kfops_rw; - else - ops = &sysfs_file_kfops_rw; - } else if (sysfs_ops->show) { - if (mode & SYSFS_PREALLOC) - ops = &sysfs_prealloc_kfops_ro; - else - ops = &sysfs_file_kfops_ro; - } else if (sysfs_ops->store) { - if (mode & SYSFS_PREALLOC) - ops = &sysfs_prealloc_kfops_wo; - else - ops = &sysfs_file_kfops_wo; - } else - ops = &sysfs_file_kfops_empty; - - size = PAGE_SIZE; - } else { - struct bin_attribute *battr = (void *)attr; - - if (battr->mmap) - ops = &sysfs_bin_kfops_mmap; - else if (battr->read && battr->write) - ops = &sysfs_bin_kfops_rw; - else if (battr->read) - ops = &sysfs_bin_kfops_ro; - else if (battr->write) - ops = &sysfs_bin_kfops_wo; + + /* every kobject with an attribute needs a ktype assigned */ + if (WARN(!sysfs_ops, KERN_ERR + "missing sysfs attribute operations for kobject: %s\n", + kobject_name(kobj))) + return -EINVAL; + + if (sysfs_ops->show && sysfs_ops->store) { + if (mode & SYSFS_PREALLOC) + ops = &sysfs_prealloc_kfops_rw; else - ops = &sysfs_file_kfops_empty; + ops = &sysfs_file_kfops_rw; + } else if (sysfs_ops->show) { + if (mode & SYSFS_PREALLOC) + ops = &sysfs_prealloc_kfops_ro; + else + ops = &sysfs_file_kfops_ro; + } else if (sysfs_ops->store) { + if (mode & SYSFS_PREALLOC) + ops = &sysfs_prealloc_kfops_wo; + else + ops = &sysfs_file_kfops_wo; + } else + ops = &sysfs_file_kfops_empty; - size = battr->size; +#ifdef CONFIG_DEBUG_LOCK_ALLOC + if (!attr->ignore_lockdep) + key = attr->key ?: (struct lock_class_key *)&attr->skey; +#endif + + kn = __kernfs_create_file(parent, attr->name, mode & 0777, uid, gid, + PAGE_SIZE, ops, (void *)attr, ns, key); + if (IS_ERR(kn)) { + if (PTR_ERR(kn) == -EEXIST) + sysfs_warn_dup(parent, attr->name); + return PTR_ERR(kn); } + return 0; +} + +int sysfs_add_bin_file_mode_ns(struct kernfs_node *parent, + const struct bin_attribute *battr, umode_t mode, + kuid_t uid, kgid_t gid, const void *ns) +{ + const struct attribute *attr = &battr->attr; + struct lock_class_key *key = NULL; + const struct kernfs_ops *ops; + struct kernfs_node *kn; + + if (battr->mmap) + ops = &sysfs_bin_kfops_mmap; + else if (battr->read && battr->write) + ops = &sysfs_bin_kfops_rw; + else if (battr->read) + ops = &sysfs_bin_kfops_ro; + else if (battr->write) + ops = &sysfs_bin_kfops_wo; + else + ops = &sysfs_file_kfops_empty; #ifdef CONFIG_DEBUG_LOCK_ALLOC if (!attr->ignore_lockdep) @@ -315,7 +329,7 @@ int sysfs_add_file_mode_ns(struct kernfs_node *parent, #endif kn = __kernfs_create_file(parent, attr->name, mode & 0777, uid, gid, - size, ops, (void *)attr, ns, key); + battr->size, ops, (void *)attr, ns, key); if (IS_ERR(kn)) { if (PTR_ERR(kn) == -EEXIST) sysfs_warn_dup(parent, attr->name); @@ -340,9 +354,7 @@ int sysfs_create_file_ns(struct kobject *kobj, const struct attribute *attr, return -EINVAL; kobject_get_ownership(kobj, &uid, &gid); - return sysfs_add_file_mode_ns(kobj->sd, attr, false, attr->mode, - uid, gid, ns); - + return sysfs_add_file_mode_ns(kobj->sd, attr, attr->mode, uid, gid, ns); } EXPORT_SYMBOL_GPL(sysfs_create_file_ns); @@ -385,8 +397,8 @@ int sysfs_add_file_to_group(struct kobject *kobj, return -ENOENT; kobject_get_ownership(kobj, &uid, &gid); - error = sysfs_add_file_mode_ns(parent, attr, false, - attr->mode, uid, gid, NULL); + error = sysfs_add_file_mode_ns(parent, attr, attr->mode, uid, gid, + NULL); kernfs_put(parent); return error; @@ -555,8 +567,8 @@ int sysfs_create_bin_file(struct kobject *kobj, return -EINVAL; kobject_get_ownership(kobj, &uid, &gid); - return sysfs_add_file_mode_ns(kobj->sd, &attr->attr, true, - attr->attr.mode, uid, gid, NULL); + return sysfs_add_bin_file_mode_ns(kobj->sd, attr, attr->attr.mode, uid, + gid, NULL); } EXPORT_SYMBOL_GPL(sysfs_create_bin_file); diff --git a/fs/sysfs/group.c b/fs/sysfs/group.c index f29d62004527..eeb0e3099421 100644 --- a/fs/sysfs/group.c +++ b/fs/sysfs/group.c @@ -61,8 +61,8 @@ static int create_files(struct kernfs_node *parent, struct kobject *kobj, (*attr)->name, mode); mode &= SYSFS_PREALLOC | 0664; - error = sysfs_add_file_mode_ns(parent, *attr, false, - mode, uid, gid, NULL); + error = sysfs_add_file_mode_ns(parent, *attr, mode, uid, + gid, NULL); if (unlikely(error)) break; } @@ -90,10 +90,9 @@ static int create_files(struct kernfs_node *parent, struct kobject *kobj, (*bin_attr)->attr.name, mode); mode &= SYSFS_PREALLOC | 0664; - error = sysfs_add_file_mode_ns(parent, - &(*bin_attr)->attr, true, - mode, - uid, gid, NULL); + error = sysfs_add_bin_file_mode_ns(parent, *bin_attr, + mode, uid, gid, + NULL); if (error) break; } @@ -340,8 +339,8 @@ int sysfs_merge_group(struct kobject *kobj, kobject_get_ownership(kobj, &uid, &gid); for ((i = 0, attr = grp->attrs); *attr && !error; (++i, ++attr)) - error = sysfs_add_file_mode_ns(parent, *attr, false, - (*attr)->mode, uid, gid, NULL); + error = sysfs_add_file_mode_ns(parent, *attr, (*attr)->mode, + uid, gid, NULL); if (error) { while (--i >= 0) kernfs_remove_by_name(parent, (*--attr)->name); diff --git a/fs/sysfs/sysfs.h b/fs/sysfs/sysfs.h index 0050cc0c0236..3f28c9af5756 100644 --- a/fs/sysfs/sysfs.h +++ b/fs/sysfs/sysfs.h @@ -28,9 +28,11 @@ void sysfs_warn_dup(struct kernfs_node *parent, const char *name); * file.c */ int sysfs_add_file_mode_ns(struct kernfs_node *parent, - const struct attribute *attr, bool is_bin, - umode_t amode, kuid_t uid, kgid_t gid, - const void *ns); + const struct attribute *attr, umode_t amode, kuid_t uid, + kgid_t gid, const void *ns); +int sysfs_add_bin_file_mode_ns(struct kernfs_node *parent, + const struct bin_attribute *battr, umode_t mode, + kuid_t uid, kgid_t gid, const void *ns); /* * symlink.c -- cgit From d1a1a9606e080c9767e229742f5b331b3b551c0c Mon Sep 17 00:00:00 2001 From: Christoph Hellwig Date: Mon, 13 Sep 2021 07:41:13 +0200 Subject: sysfs: refactor sysfs_add_file_mode_ns Regroup the code so that preallocated attributes and normal attributes are handled in clearly separate blocks. Acked-by: Christian Brauner Acked-by: Tejun Heo Signed-off-by: Christoph Hellwig Link: https://lore.kernel.org/r/20210913054121.616001-6-hch@lst.de Signed-off-by: Greg Kroah-Hartman --- fs/sysfs/file.c | 27 ++++++++++++++------------- 1 file changed, 14 insertions(+), 13 deletions(-) (limited to 'fs') diff --git a/fs/sysfs/file.c b/fs/sysfs/file.c index f737bd61f71b..74a2a8021c8b 100644 --- a/fs/sysfs/file.c +++ b/fs/sysfs/file.c @@ -261,7 +261,7 @@ int sysfs_add_file_mode_ns(struct kernfs_node *parent, struct kobject *kobj = parent->priv; const struct sysfs_ops *sysfs_ops = kobj->ktype->sysfs_ops; struct lock_class_key *key = NULL; - const struct kernfs_ops *ops; + const struct kernfs_ops *ops = NULL; struct kernfs_node *kn; /* every kobject with an attribute needs a ktype assigned */ @@ -270,22 +270,23 @@ int sysfs_add_file_mode_ns(struct kernfs_node *parent, kobject_name(kobj))) return -EINVAL; - if (sysfs_ops->show && sysfs_ops->store) { - if (mode & SYSFS_PREALLOC) + if (mode & SYSFS_PREALLOC) { + if (sysfs_ops->show && sysfs_ops->store) ops = &sysfs_prealloc_kfops_rw; - else - ops = &sysfs_file_kfops_rw; - } else if (sysfs_ops->show) { - if (mode & SYSFS_PREALLOC) + else if (sysfs_ops->show) ops = &sysfs_prealloc_kfops_ro; - else - ops = &sysfs_file_kfops_ro; - } else if (sysfs_ops->store) { - if (mode & SYSFS_PREALLOC) + else if (sysfs_ops->store) ops = &sysfs_prealloc_kfops_wo; - else + } else { + if (sysfs_ops->show && sysfs_ops->store) + ops = &sysfs_file_kfops_rw; + else if (sysfs_ops->show) + ops = &sysfs_file_kfops_ro; + else if (sysfs_ops->store) ops = &sysfs_file_kfops_wo; - } else + } + + if (!ops) ops = &sysfs_file_kfops_empty; #ifdef CONFIG_DEBUG_LOCK_ALLOC -- cgit From 820879ee1865f7010ec3a949322f643f008c9feb Mon Sep 17 00:00:00 2001 From: Christoph Hellwig Date: Mon, 13 Sep 2021 07:41:14 +0200 Subject: sysfs: simplify sysfs_kf_seq_show Contrary to the comment ->show is never called from lseek for sysfs, given that sysfs does not use seq_lseek. So remove the NULL ->show case and just WARN and return an error if some future code path ends up here. Acked-by: Tejun Heo Signed-off-by: Christoph Hellwig Link: https://lore.kernel.org/r/20210913054121.616001-7-hch@lst.de Signed-off-by: Greg Kroah-Hartman --- fs/sysfs/file.c | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) (limited to 'fs') diff --git a/fs/sysfs/file.c b/fs/sysfs/file.c index 74a2a8021c8b..42dcf96881b6 100644 --- a/fs/sysfs/file.c +++ b/fs/sysfs/file.c @@ -45,6 +45,9 @@ static int sysfs_kf_seq_show(struct seq_file *sf, void *v) ssize_t count; char *buf; + if (WARN_ON_ONCE(!ops->show)) + return -EINVAL; + /* acquire buffer and ensure that it's >= PAGE_SIZE and clear */ count = seq_get_buf(sf, &buf); if (count < PAGE_SIZE) { @@ -53,15 +56,9 @@ static int sysfs_kf_seq_show(struct seq_file *sf, void *v) } memset(buf, 0, PAGE_SIZE); - /* - * Invoke show(). Control may reach here via seq file lseek even - * if @ops->show() isn't implemented. - */ - if (ops->show) { - count = ops->show(kobj, of->kn->priv, buf); - if (count < 0) - return count; - } + count = ops->show(kobj, of->kn->priv, buf); + if (count < 0) + return count; /* * The code works fine with PAGE_SIZE return but it's likely to -- cgit From 8f5cfb3b5a1cb887d625b040e7260bd592ca57e0 Mon Sep 17 00:00:00 2001 From: Luis Chamberlain Date: Mon, 27 Sep 2021 09:38:00 -0700 Subject: fs/kernfs/symlink.c: replace S_IRWXUGO with 0777 on kernfs_create_link() If one ends up extending this line checkpatch will complain about the use of S_IRWXUGO suggesting it is not preferred and that 0777 should be used instead. Take the tip from checkpatch and do that change before we do our subsequent changes. This makes no functional changes. Signed-off-by: Luis Chamberlain Link: https://lore.kernel.org/r/20210927163805.808907-8-mcgrof@kernel.org Signed-off-by: Greg Kroah-Hartman --- fs/kernfs/symlink.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) (limited to 'fs') diff --git a/fs/kernfs/symlink.c b/fs/kernfs/symlink.c index c8f8e41b8411..19a6c71c6ff5 100644 --- a/fs/kernfs/symlink.c +++ b/fs/kernfs/symlink.c @@ -36,8 +36,7 @@ struct kernfs_node *kernfs_create_link(struct kernfs_node *parent, gid = target->iattr->ia_gid; } - kn = kernfs_new_node(parent, name, S_IFLNK|S_IRWXUGO, uid, gid, - KERNFS_LINK); + kn = kernfs_new_node(parent, name, S_IFLNK|0777, uid, gid, KERNFS_LINK); if (!kn) return ERR_PTR(-ENOMEM); -- cgit From d7c5bf94475b8b8fb960c7cf90682086076934df Mon Sep 17 00:00:00 2001 From: Luis Chamberlain Date: Mon, 27 Sep 2021 09:38:01 -0700 Subject: fs/sysfs/dir.c: replace S_IRWXU|S_IRUGO|S_IXUGO with 0755 sysfs_create_dir_ns() If one ends up expanding on this line checkpatch will complain that the combination S_IRWXU|S_IRUGO|S_IXUGO should just be replaced with the octal 0755. Do that. This makes no functional changes. Signed-off-by: Luis Chamberlain Link: https://lore.kernel.org/r/20210927163805.808907-9-mcgrof@kernel.org Signed-off-by: Greg Kroah-Hartman --- fs/sysfs/dir.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) (limited to 'fs') diff --git a/fs/sysfs/dir.c b/fs/sysfs/dir.c index 59dffd5ca517..b6b6796e1616 100644 --- a/fs/sysfs/dir.c +++ b/fs/sysfs/dir.c @@ -56,8 +56,7 @@ int sysfs_create_dir_ns(struct kobject *kobj, const void *ns) kobject_get_ownership(kobj, &uid, &gid); - kn = kernfs_create_dir_ns(parent, kobject_name(kobj), - S_IRWXU | S_IRUGO | S_IXUGO, uid, gid, + kn = kernfs_create_dir_ns(parent, kobject_name(kobj), 0755, uid, gid, kobj, ns); if (IS_ERR(kn)) { if (PTR_ERR(kn) == -EEXIST) -- cgit