From e2dfa1d5223cdfa7b832a582de0b7504eaf7ae22 Mon Sep 17 00:00:00 2001 From: Zhen Lei Date: Wed, 26 Jul 2023 14:25:08 +0800 Subject: kobject: Add helper kobj_ns_type_is_valid() There are too many "(type > KOBJ_NS_TYPE_NONE) && (type < KOBJ_NS_TYPES)" and "(type <= KOBJ_NS_TYPE_NONE) || (type >= KOBJ_NS_TYPES)", add helper kobj_ns_type_is_valid() to eliminate duplicate code and improve readability. Signed-off-by: Zhen Lei Link: https://lore.kernel.org/r/20230726062508.950-1-thunder.leizhen@huaweicloud.com Signed-off-by: Greg Kroah-Hartman --- lib/kobject.c | 33 ++++++++++++++++----------------- 1 file changed, 16 insertions(+), 17 deletions(-) (limited to 'lib') diff --git a/lib/kobject.c b/lib/kobject.c index 16d530f9c174..14e845209226 100644 --- a/lib/kobject.c +++ b/lib/kobject.c @@ -56,6 +56,14 @@ void kobject_get_ownership(const struct kobject *kobj, kuid_t *uid, kgid_t *gid) kobj->ktype->get_ownership(kobj, uid, gid); } +static bool kobj_ns_type_is_valid(enum kobj_ns_type type) +{ + if ((type <= KOBJ_NS_TYPE_NONE) || (type >= KOBJ_NS_TYPES)) + return false; + + return true; +} + static int create_dir(struct kobject *kobj) { const struct kobj_type *ktype = get_ktype(kobj); @@ -86,8 +94,7 @@ static int create_dir(struct kobject *kobj) */ ops = kobj_child_ns_ops(kobj); if (ops) { - BUG_ON(ops->type <= KOBJ_NS_TYPE_NONE); - BUG_ON(ops->type >= KOBJ_NS_TYPES); + BUG_ON(!kobj_ns_type_is_valid(ops->type)); BUG_ON(!kobj_ns_type_registered(ops->type)); sysfs_enable_ns(kobj->sd); @@ -1017,11 +1024,7 @@ int kobj_ns_type_register(const struct kobj_ns_type_operations *ops) spin_lock(&kobj_ns_type_lock); error = -EINVAL; - if (type >= KOBJ_NS_TYPES) - goto out; - - error = -EINVAL; - if (type <= KOBJ_NS_TYPE_NONE) + if (!kobj_ns_type_is_valid(type)) goto out; error = -EBUSY; @@ -1041,7 +1044,7 @@ int kobj_ns_type_registered(enum kobj_ns_type type) int registered = 0; spin_lock(&kobj_ns_type_lock); - if ((type > KOBJ_NS_TYPE_NONE) && (type < KOBJ_NS_TYPES)) + if (kobj_ns_type_is_valid(type)) registered = kobj_ns_ops_tbl[type] != NULL; spin_unlock(&kobj_ns_type_lock); @@ -1068,8 +1071,7 @@ bool kobj_ns_current_may_mount(enum kobj_ns_type type) bool may_mount = true; spin_lock(&kobj_ns_type_lock); - if ((type > KOBJ_NS_TYPE_NONE) && (type < KOBJ_NS_TYPES) && - kobj_ns_ops_tbl[type]) + if (kobj_ns_type_is_valid(type) && kobj_ns_ops_tbl[type]) may_mount = kobj_ns_ops_tbl[type]->current_may_mount(); spin_unlock(&kobj_ns_type_lock); @@ -1081,8 +1083,7 @@ void *kobj_ns_grab_current(enum kobj_ns_type type) void *ns = NULL; spin_lock(&kobj_ns_type_lock); - if ((type > KOBJ_NS_TYPE_NONE) && (type < KOBJ_NS_TYPES) && - kobj_ns_ops_tbl[type]) + if (kobj_ns_type_is_valid(type) && kobj_ns_ops_tbl[type]) ns = kobj_ns_ops_tbl[type]->grab_current_ns(); spin_unlock(&kobj_ns_type_lock); @@ -1095,8 +1096,7 @@ const void *kobj_ns_netlink(enum kobj_ns_type type, struct sock *sk) const void *ns = NULL; spin_lock(&kobj_ns_type_lock); - if ((type > KOBJ_NS_TYPE_NONE) && (type < KOBJ_NS_TYPES) && - kobj_ns_ops_tbl[type]) + if (kobj_ns_type_is_valid(type) && kobj_ns_ops_tbl[type]) ns = kobj_ns_ops_tbl[type]->netlink_ns(sk); spin_unlock(&kobj_ns_type_lock); @@ -1108,8 +1108,7 @@ const void *kobj_ns_initial(enum kobj_ns_type type) const void *ns = NULL; spin_lock(&kobj_ns_type_lock); - if ((type > KOBJ_NS_TYPE_NONE) && (type < KOBJ_NS_TYPES) && - kobj_ns_ops_tbl[type]) + if (kobj_ns_type_is_valid(type) && kobj_ns_ops_tbl[type]) ns = kobj_ns_ops_tbl[type]->initial_ns(); spin_unlock(&kobj_ns_type_lock); @@ -1119,7 +1118,7 @@ const void *kobj_ns_initial(enum kobj_ns_type type) void kobj_ns_drop(enum kobj_ns_type type, void *ns) { spin_lock(&kobj_ns_type_lock); - if ((type > KOBJ_NS_TYPE_NONE) && (type < KOBJ_NS_TYPES) && + if (kobj_ns_type_is_valid(type) && kobj_ns_ops_tbl[type] && kobj_ns_ops_tbl[type]->drop_ns) kobj_ns_ops_tbl[type]->drop_ns(ns); spin_unlock(&kobj_ns_type_lock); -- cgit v1.2.3-58-ga151 From 4d0fe8c52bb3029d83e323c961221156ab98680b Mon Sep 17 00:00:00 2001 From: Zhen Lei Date: Sat, 5 Aug 2023 16:41:13 +0800 Subject: kobject: Add sanity check for kset->kobj.ktype in kset_register() When I register a kset in the following way: static struct kset my_kset; kobject_set_name(&my_kset.kobj, "my_kset"); ret = kset_register(&my_kset); A null pointer dereference exception is occurred: [ 4453.568337] Unable to handle kernel NULL pointer dereference at \ virtual address 0000000000000028 ... ... [ 4453.810361] Call trace: [ 4453.813062] kobject_get_ownership+0xc/0x34 [ 4453.817493] kobject_add_internal+0x98/0x274 [ 4453.822005] kset_register+0x5c/0xb4 [ 4453.825820] my_kobj_init+0x44/0x1000 [my_kset] ... ... Because I didn't initialize my_kset.kobj.ktype. According to the description in Documentation/core-api/kobject.rst: - A ktype is the type of object that embeds a kobject. Every structure that embeds a kobject needs a corresponding ktype. So add sanity check to make sure kset->kobj.ktype is not NULL. Signed-off-by: Zhen Lei Link: https://lore.kernel.org/r/20230805084114.1298-2-thunder.leizhen@huaweicloud.com Signed-off-by: Greg Kroah-Hartman --- lib/kobject.c | 5 +++++ 1 file changed, 5 insertions(+) (limited to 'lib') diff --git a/lib/kobject.c b/lib/kobject.c index 14e845209226..72fa20f405f1 100644 --- a/lib/kobject.c +++ b/lib/kobject.c @@ -861,6 +861,11 @@ int kset_register(struct kset *k) if (!k) return -EINVAL; + if (!k->kobj.ktype) { + pr_err("must have a ktype to be initialized properly!\n"); + return -EINVAL; + } + kset_init(k); err = kobject_add_internal(&k->kobj); if (err) { -- cgit v1.2.3-58-ga151 From 1b28cb81dab7c1eedc6034206f4e8d644046ad31 Mon Sep 17 00:00:00 2001 From: Zhen Lei Date: Sat, 5 Aug 2023 16:41:14 +0800 Subject: kobject: Remove redundant checks for whether ktype is NULL When adding koject or kset, we have made sure that ktype cannot be NULL. Therefore, after adding koject or kset, there is no need to worry about ktype being NULL. Clear all ktype-related redundancy checks. Signed-off-by: Zhen Lei Link: https://lore.kernel.org/r/20230805084114.1298-3-thunder.leizhen@huaweicloud.com Signed-off-by: Greg Kroah-Hartman --- lib/kobject.c | 24 ++++++++++-------------- 1 file changed, 10 insertions(+), 14 deletions(-) (limited to 'lib') diff --git a/lib/kobject.c b/lib/kobject.c index 72fa20f405f1..59dbcbdb1c91 100644 --- a/lib/kobject.c +++ b/lib/kobject.c @@ -74,12 +74,10 @@ static int create_dir(struct kobject *kobj) if (error) return error; - if (ktype) { - error = sysfs_create_groups(kobj, ktype->default_groups); - if (error) { - sysfs_remove_dir(kobj); - return error; - } + error = sysfs_create_groups(kobj, ktype->default_groups); + if (error) { + sysfs_remove_dir(kobj); + return error; } /* @@ -591,8 +589,7 @@ static void __kobject_del(struct kobject *kobj) sd = kobj->sd; ktype = get_ktype(kobj); - if (ktype) - sysfs_remove_groups(kobj, ktype->default_groups); + sysfs_remove_groups(kobj, ktype->default_groups); /* send "remove" if the caller did not do it but sent "add" */ if (kobj->state_add_uevent_sent && !kobj->state_remove_uevent_sent) { @@ -669,10 +666,6 @@ static void kobject_cleanup(struct kobject *kobj) pr_debug("'%s' (%p): %s, parent %p\n", kobject_name(kobj), kobj, __func__, kobj->parent); - if (t && !t->release) - pr_debug("'%s' (%p): does not have a release() function, it is broken and must be fixed. See Documentation/core-api/kobject.rst.\n", - kobject_name(kobj), kobj); - /* remove from sysfs if the caller did not do it */ if (kobj->state_in_sysfs) { pr_debug("'%s' (%p): auto cleanup kobject_del\n", @@ -683,10 +676,13 @@ static void kobject_cleanup(struct kobject *kobj) parent = NULL; } - if (t && t->release) { + if (t->release) { pr_debug("'%s' (%p): calling ktype release\n", kobject_name(kobj), kobj); t->release(kobj); + } else { + pr_debug("'%s' (%p): does not have a release() function, it is broken and must be fixed. See Documentation/core-api/kobject.rst.\n", + kobject_name(kobj), kobj); } /* free name if we allocated it */ @@ -1060,7 +1056,7 @@ const struct kobj_ns_type_operations *kobj_child_ns_ops(const struct kobject *pa { const struct kobj_ns_type_operations *ops = NULL; - if (parent && parent->ktype && parent->ktype->child_ns_type) + if (parent && parent->ktype->child_ns_type) ops = parent->ktype->child_ns_type(parent); return ops; -- cgit v1.2.3-58-ga151