linux-kernel - [PATCH] block: don't embed integrity

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-Id: <20230309-kobj_release-gendisk_integrity-v1-1-a240f54eac60@weissschuh.net>
Date:   Thu, 09 Mar 2023 20:23:41 +0000
From:   Thomas Weißschuh <linux@...ssschuh.net>
To:     Jens Axboe <axboe@...nel.dk>
Cc:     linux-block@...r.kernel.org, linux-kernel@...r.kernel.org,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Mirsad Todorovac <mirsad.todorovac@....unizg.hr>,
        Thomas Weißschuh <linux@...ssschuh.net>
Subject: [PATCH] block: don't embed integrity_kobj into gendisk

A struct kobject is only supposed to be embedded into objects which
lifetime it will manage.
Objects of type struct gendisk however are refcounted by their part0
block_device.
Therefore the integrity_kobj should not be embedded but split into its
own independently managed object.

This will also provide a proper .release function for the ktype which
avoid warnings like the following:
kobject: 'integrity' (000000005198bea8): does not have a release() function, it is broken and must be fixed.

While modifying blk_integrity_del() also drop the explicit call to
kobject_uevent(KOBJ_REMOVE) as the driver care will do this
automatically.

Reported-by: Mirsad Todorovac <mirsad.todorovac@....unizg.hr>
Link: https://lore.kernel.org/lkml/60b2b66c-22c9-1d38-ed1c-7b7d95e32720@alu.unizg.hr/
Signed-off-by: Thomas Weißschuh <linux@...ssschuh.net>
---
 block/blk-integrity.c  | 32 ++++++++++++++++++++++++--------
 include/linux/blkdev.h |  2 +-
 2 files changed, 25 insertions(+), 9 deletions(-)

diff --git a/block/blk-integrity.c b/block/blk-integrity.c
index 8f01d786f5cb..40adf33f5535 100644
--- a/block/blk-integrity.c
+++ b/block/blk-integrity.c
@@ -218,10 +218,15 @@ struct integrity_sysfs_entry {
 	ssize_t (*store)(struct blk_integrity *, const char *, size_t);
 };
 
+static inline struct gendisk *integrity_kobj_to_disk(struct kobject *kobj)
+{
+	return dev_to_disk(kobj_to_dev(kobj->parent));
+}
+
 static ssize_t integrity_attr_show(struct kobject *kobj, struct attribute *attr,
 				   char *page)
 {
-	struct gendisk *disk = container_of(kobj, struct gendisk, integrity_kobj);
+	struct gendisk *disk = integrity_kobj_to_disk(kobj);
 	struct blk_integrity *bi = &disk->queue->integrity;
 	struct integrity_sysfs_entry *entry =
 		container_of(attr, struct integrity_sysfs_entry, attr);
@@ -233,7 +238,7 @@ static ssize_t integrity_attr_store(struct kobject *kobj,
 				    struct attribute *attr, const char *page,
 				    size_t count)
 {
-	struct gendisk *disk = container_of(kobj, struct gendisk, integrity_kobj);
+	struct gendisk *disk = integrity_kobj_to_disk(kobj);
 	struct blk_integrity *bi = &disk->queue->integrity;
 	struct integrity_sysfs_entry *entry =
 		container_of(attr, struct integrity_sysfs_entry, attr);
@@ -356,9 +361,15 @@ static const struct sysfs_ops integrity_ops = {
 	.store	= &integrity_attr_store,
 };
 
+static void integrity_release(struct kobject *kobj)
+{
+	kfree(kobj);
+}
+
 static const struct kobj_type integrity_ktype = {
 	.default_groups = integrity_groups,
 	.sysfs_ops	= &integrity_ops,
+	.release        = integrity_release,
 };
 
 static blk_status_t blk_integrity_nop_fn(struct blk_integrity_iter *iter)
@@ -442,16 +453,21 @@ int blk_integrity_add(struct gendisk *disk)
 {
 	int ret;
 
-	ret = kobject_init_and_add(&disk->integrity_kobj, &integrity_ktype,
+	disk->integrity_kobj = kmalloc(sizeof(*disk->integrity_kobj), GFP_KERNEL);
+	if (!disk->integrity_kobj)
+		return -ENOMEM;
+
+	ret = kobject_init_and_add(disk->integrity_kobj, &integrity_ktype,
 				   &disk_to_dev(disk)->kobj, "%s", "integrity");
-	if (!ret)
-		kobject_uevent(&disk->integrity_kobj, KOBJ_ADD);
+	if (ret)
+		kobject_put(disk->integrity_kobj);
+	else
+		kobject_uevent(disk->integrity_kobj, KOBJ_ADD);
+
 	return ret;
 }
 
 void blk_integrity_del(struct gendisk *disk)
 {
-	kobject_uevent(&disk->integrity_kobj, KOBJ_REMOVE);
-	kobject_del(&disk->integrity_kobj);
-	kobject_put(&disk->integrity_kobj);
+	kobject_put(disk->integrity_kobj);
 }
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index d1aee08f8c18..2fbfb3277a2b 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -164,7 +164,7 @@ struct gendisk {
 	atomic_t sync_io;		/* RAID */
 	struct disk_events *ev;
 #ifdef  CONFIG_BLK_DEV_INTEGRITY
-	struct kobject integrity_kobj;
+	struct kobject *integrity_kobj;
 #endif	/* CONFIG_BLK_DEV_INTEGRITY */
 
 #ifdef CONFIG_BLK_DEV_ZONED

---
base-commit: 44889ba56cbb3d51154660ccd15818bc77276696
change-id: 20230309-kobj_release-gendisk_integrity-e26c0bc126aa

Best regards,
-- 
Thomas Weißschuh <linux@...ssschuh.net>