integrity_kobj did not have a release function and with CONFIG_DEBUG_KOBJECT_RELEASE, a use-after-free would be triggered as its holding struct gendisk would be freed without relying on its refcount.
Thomas Weißschuh (3): blk-integrity: use sysfs_emit blk-integrity: convert to struct device_attribute blk-integrity: register sysfs attributes on struct device
block/blk-integrity.c | 175 ++++++++++++++--------------------------- block/blk.h | 10 +-- block/genhd.c | 12 +-- include/linux/blkdev.h | 3 - 4 files changed, 66 insertions(+), 134 deletions(-)
From: Thomas Weißschuh linux@weissschuh.net
Upstream commit 3315e169b446249c1b61ff988d157238f4b2c5a0.
The correct way to emit data into sysfs is via sysfs_emit(), use it.
Also perform some trivial syntactic cleanups.
Signed-off-by: Thomas Weißschuh linux@weissschuh.net Reviewed-by: Christoph Hellwig hch@lst.de Reviewed-by: Martin K. Petersen martin.petersen@oracle.com Link: https://lore.kernel.org/r/20230309-kobj_release-gendisk_integrity-v3-1-ceccb... Signed-off-by: Jens Axboe axboe@kernel.dk Signed-off-by: Thadeu Lima de Souza Cascardo cascardo@igalia.com --- block/blk-integrity.c | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-)
diff --git a/block/blk-integrity.c b/block/blk-integrity.c index e2d88611d5bf..7131f0d30f11 100644 --- a/block/blk-integrity.c +++ b/block/blk-integrity.c @@ -248,20 +248,19 @@ static ssize_t integrity_attr_store(struct kobject *kobj, static ssize_t integrity_format_show(struct blk_integrity *bi, char *page) { if (bi->profile && bi->profile->name) - return sprintf(page, "%s\n", bi->profile->name); - else - return sprintf(page, "none\n"); + return sysfs_emit(page, "%s\n", bi->profile->name); + return sysfs_emit(page, "none\n"); }
static ssize_t integrity_tag_size_show(struct blk_integrity *bi, char *page) { - return sprintf(page, "%u\n", bi->tag_size); + return sysfs_emit(page, "%u\n", bi->tag_size); }
static ssize_t integrity_interval_show(struct blk_integrity *bi, char *page) { - return sprintf(page, "%u\n", - bi->interval_exp ? 1 << bi->interval_exp : 0); + return sysfs_emit(page, "%u\n", + bi->interval_exp ? 1 << bi->interval_exp : 0); }
static ssize_t integrity_verify_store(struct blk_integrity *bi, @@ -280,7 +279,7 @@ static ssize_t integrity_verify_store(struct blk_integrity *bi,
static ssize_t integrity_verify_show(struct blk_integrity *bi, char *page) { - return sprintf(page, "%d\n", (bi->flags & BLK_INTEGRITY_VERIFY) != 0); + return sysfs_emit(page, "%d\n", !!(bi->flags & BLK_INTEGRITY_VERIFY)); }
static ssize_t integrity_generate_store(struct blk_integrity *bi, @@ -299,13 +298,13 @@ static ssize_t integrity_generate_store(struct blk_integrity *bi,
static ssize_t integrity_generate_show(struct blk_integrity *bi, char *page) { - return sprintf(page, "%d\n", (bi->flags & BLK_INTEGRITY_GENERATE) != 0); + return sysfs_emit(page, "%d\n", !!(bi->flags & BLK_INTEGRITY_GENERATE)); }
static ssize_t integrity_device_show(struct blk_integrity *bi, char *page) { - return sprintf(page, "%u\n", - (bi->flags & BLK_INTEGRITY_DEVICE_CAPABLE) != 0); + return sysfs_emit(page, "%u\n", + !!(bi->flags & BLK_INTEGRITY_DEVICE_CAPABLE)); }
static struct integrity_sysfs_entry integrity_format_entry = {
From: Thomas Weißschuh linux@weissschuh.net
Upstream commit 76b8c319f02715e14abdbbbdd6508e83a1059bcc.
An upcoming patch will register the integrity attributes directly with the struct device kobject. For this the attributes have to be implemented in terms of struct device_attribute.
Signed-off-by: Thomas Weißschuh linux@weissschuh.net Reviewed-by: Christoph Hellwig hch@lst.de Reviewed-by: Martin K. Petersen martin.petersen@oracle.com Link: https://lore.kernel.org/r/20230309-kobj_release-gendisk_integrity-v3-2-ceccb... Signed-off-by: Jens Axboe axboe@kernel.dk Signed-off-by: Thadeu Lima de Souza Cascardo cascardo@igalia.com --- block/blk-integrity.c | 127 +++++++++++++++++++++--------------------- 1 file changed, 62 insertions(+), 65 deletions(-)
diff --git a/block/blk-integrity.c b/block/blk-integrity.c index 7131f0d30f11..91a502b01fc7 100644 --- a/block/blk-integrity.c +++ b/block/blk-integrity.c @@ -212,21 +212,15 @@ bool blk_integrity_merge_bio(struct request_queue *q, struct request *req, return true; }
-struct integrity_sysfs_entry { - struct attribute attr; - ssize_t (*show)(struct blk_integrity *, char *); - ssize_t (*store)(struct blk_integrity *, const char *, size_t); -}; - 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 blk_integrity *bi = &disk->queue->integrity; - struct integrity_sysfs_entry *entry = - container_of(attr, struct integrity_sysfs_entry, attr); + struct device *dev = disk_to_dev(disk); + struct device_attribute *dev_attr = + container_of(attr, struct device_attribute, attr);
- return entry->show(bi, page); + return dev_attr->show(dev, dev_attr, page); }
static ssize_t integrity_attr_store(struct kobject *kobj, @@ -234,38 +228,53 @@ static ssize_t integrity_attr_store(struct kobject *kobj, size_t count) { struct gendisk *disk = container_of(kobj, struct gendisk, integrity_kobj); - struct blk_integrity *bi = &disk->queue->integrity; - struct integrity_sysfs_entry *entry = - container_of(attr, struct integrity_sysfs_entry, attr); - ssize_t ret = 0; + struct device *dev = disk_to_dev(disk); + struct device_attribute *dev_attr = + container_of(attr, struct device_attribute, attr);
- if (entry->store) - ret = entry->store(bi, page, count); + if (!dev_attr->store) + return 0; + return dev_attr->store(dev, dev_attr, page, count); +}
- return ret; +static inline struct blk_integrity *dev_to_bi(struct device *dev) +{ + return &dev_to_disk(dev)->queue->integrity; }
-static ssize_t integrity_format_show(struct blk_integrity *bi, char *page) +static ssize_t format_show(struct device *dev, struct device_attribute *attr, + char *page) { + struct blk_integrity *bi = dev_to_bi(dev); + if (bi->profile && bi->profile->name) return sysfs_emit(page, "%s\n", bi->profile->name); return sysfs_emit(page, "none\n"); }
-static ssize_t integrity_tag_size_show(struct blk_integrity *bi, char *page) +static ssize_t tag_size_show(struct device *dev, struct device_attribute *attr, + char *page) { + struct blk_integrity *bi = dev_to_bi(dev); + return sysfs_emit(page, "%u\n", bi->tag_size); }
-static ssize_t integrity_interval_show(struct blk_integrity *bi, char *page) +static ssize_t protection_interval_bytes_show(struct device *dev, + struct device_attribute *attr, + char *page) { + struct blk_integrity *bi = dev_to_bi(dev); + return sysfs_emit(page, "%u\n", bi->interval_exp ? 1 << bi->interval_exp : 0); }
-static ssize_t integrity_verify_store(struct blk_integrity *bi, - const char *page, size_t count) +static ssize_t read_verify_store(struct device *dev, + struct device_attribute *attr, + const char *page, size_t count) { + struct blk_integrity *bi = dev_to_bi(dev); char *p = (char *) page; unsigned long val = simple_strtoul(p, &p, 10);
@@ -277,14 +286,20 @@ static ssize_t integrity_verify_store(struct blk_integrity *bi, return count; }
-static ssize_t integrity_verify_show(struct blk_integrity *bi, char *page) +static ssize_t read_verify_show(struct device *dev, + struct device_attribute *attr, char *page) { + struct blk_integrity *bi = dev_to_bi(dev); + return sysfs_emit(page, "%d\n", !!(bi->flags & BLK_INTEGRITY_VERIFY)); }
-static ssize_t integrity_generate_store(struct blk_integrity *bi, - const char *page, size_t count) +static ssize_t write_generate_store(struct device *dev, + struct device_attribute *attr, + const char *page, size_t count) { + struct blk_integrity *bi = dev_to_bi(dev); + char *p = (char *) page; unsigned long val = simple_strtoul(p, &p, 10);
@@ -296,57 +311,39 @@ static ssize_t integrity_generate_store(struct blk_integrity *bi, return count; }
-static ssize_t integrity_generate_show(struct blk_integrity *bi, char *page) +static ssize_t write_generate_show(struct device *dev, + struct device_attribute *attr, char *page) { + struct blk_integrity *bi = dev_to_bi(dev); + return sysfs_emit(page, "%d\n", !!(bi->flags & BLK_INTEGRITY_GENERATE)); }
-static ssize_t integrity_device_show(struct blk_integrity *bi, char *page) +static ssize_t device_is_integrity_capable_show(struct device *dev, + struct device_attribute *attr, + char *page) { + struct blk_integrity *bi = dev_to_bi(dev); + return sysfs_emit(page, "%u\n", !!(bi->flags & BLK_INTEGRITY_DEVICE_CAPABLE)); }
-static struct integrity_sysfs_entry integrity_format_entry = { - .attr = { .name = "format", .mode = 0444 }, - .show = integrity_format_show, -}; - -static struct integrity_sysfs_entry integrity_tag_size_entry = { - .attr = { .name = "tag_size", .mode = 0444 }, - .show = integrity_tag_size_show, -}; - -static struct integrity_sysfs_entry integrity_interval_entry = { - .attr = { .name = "protection_interval_bytes", .mode = 0444 }, - .show = integrity_interval_show, -}; - -static struct integrity_sysfs_entry integrity_verify_entry = { - .attr = { .name = "read_verify", .mode = 0644 }, - .show = integrity_verify_show, - .store = integrity_verify_store, -}; - -static struct integrity_sysfs_entry integrity_generate_entry = { - .attr = { .name = "write_generate", .mode = 0644 }, - .show = integrity_generate_show, - .store = integrity_generate_store, -}; - -static struct integrity_sysfs_entry integrity_device_entry = { - .attr = { .name = "device_is_integrity_capable", .mode = 0444 }, - .show = integrity_device_show, -}; +static DEVICE_ATTR_RO(format); +static DEVICE_ATTR_RO(tag_size); +static DEVICE_ATTR_RO(protection_interval_bytes); +static DEVICE_ATTR_RW(read_verify); +static DEVICE_ATTR_RW(write_generate); +static DEVICE_ATTR_RO(device_is_integrity_capable);
static struct attribute *integrity_attrs[] = { - &integrity_format_entry.attr, - &integrity_tag_size_entry.attr, - &integrity_interval_entry.attr, - &integrity_verify_entry.attr, - &integrity_generate_entry.attr, - &integrity_device_entry.attr, - NULL, + &dev_attr_format.attr, + &dev_attr_tag_size.attr, + &dev_attr_protection_interval_bytes.attr, + &dev_attr_read_verify.attr, + &dev_attr_write_generate.attr, + &dev_attr_device_is_integrity_capable.attr, + NULL }; ATTRIBUTE_GROUPS(integrity);
From: Thomas Weißschuh linux@weissschuh.net
Upstream commit ff53cd52d9bdbf4074d2bbe9b591729997780bd3.
The "integrity" kobject only acted as a holder for static sysfs entries. It also was embedded into struct gendisk without managing it, violating assumptions of the driver core.
Instead register the sysfs entries directly onto the struct device.
Also drop the now unused member integrity_kobj from struct gendisk.
Suggested-by: Christoph Hellwig hch@infradead.org Signed-off-by: Thomas Weißschuh linux@weissschuh.net Reviewed-by: Christoph Hellwig hch@lst.de Reviewed-by: Martin K. Petersen martin.petersen@oracle.com Link: https://lore.kernel.org/r/20230309-kobj_release-gendisk_integrity-v3-3-ceccb... Signed-off-by: Jens Axboe axboe@kernel.dk [cascardo: conflict because of constification of integrity_ktype] [cascardo: struct gendisk is defined at include/linux/genhd.h] [cascardo: there is no blk_trace_attr_group] Signed-off-by: Thadeu Lima de Souza Cascardo cascardo@igalia.com --- block/blk-integrity.c | 55 +++---------------------------------------- block/blk.h | 10 +------- block/genhd.c | 12 ++++------ include/linux/genhd.h | 3 --- 4 files changed, 8 insertions(+), 72 deletions(-)
diff --git a/block/blk-integrity.c b/block/blk-integrity.c index 8868b1e01d58..fbbb38cc9e8a 100644 --- a/block/blk-integrity.c +++ b/block/blk-integrity.c @@ -212,31 +212,6 @@ bool blk_integrity_merge_bio(struct request_queue *q, struct request *req, return true; }
-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 device *dev = disk_to_dev(disk); - struct device_attribute *dev_attr = - container_of(attr, struct device_attribute, attr); - - return dev_attr->show(dev, dev_attr, page); -} - -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 device *dev = disk_to_dev(disk); - struct device_attribute *dev_attr = - container_of(attr, struct device_attribute, attr); - - if (!dev_attr->store) - return 0; - return dev_attr->store(dev, dev_attr, page, count); -} - static inline struct blk_integrity *dev_to_bi(struct device *dev) { return &dev_to_disk(dev)->queue->integrity; @@ -345,16 +320,10 @@ static struct attribute *integrity_attrs[] = { &dev_attr_device_is_integrity_capable.attr, NULL }; -ATTRIBUTE_GROUPS(integrity);
-static const struct sysfs_ops integrity_ops = { - .show = &integrity_attr_show, - .store = &integrity_attr_store, -}; - -static struct kobj_type integrity_ktype = { - .default_groups = integrity_groups, - .sysfs_ops = &integrity_ops, +const struct attribute_group blk_integrity_attr_group = { + .name = "integrity", + .attrs = integrity_attrs, };
static blk_status_t blk_integrity_nop_fn(struct blk_integrity_iter *iter) @@ -431,21 +400,3 @@ void blk_integrity_unregister(struct gendisk *disk) memset(bi, 0, sizeof(*bi)); } EXPORT_SYMBOL(blk_integrity_unregister); - -int blk_integrity_add(struct gendisk *disk) -{ - int ret; - - 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); - 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); -} diff --git a/block/blk.h b/block/blk.h index aab72194d226..e90a5e348512 100644 --- a/block/blk.h +++ b/block/blk.h @@ -130,8 +130,7 @@ static inline bool integrity_req_gap_front_merge(struct request *req, bip_next->bip_vec[0].bv_offset); }
-int blk_integrity_add(struct gendisk *disk); -void blk_integrity_del(struct gendisk *); +extern const struct attribute_group blk_integrity_attr_group; #else /* CONFIG_BLK_DEV_INTEGRITY */ static inline bool blk_integrity_merge_rq(struct request_queue *rq, struct request *r1, struct request *r2) @@ -164,13 +163,6 @@ static inline bool bio_integrity_endio(struct bio *bio) static inline void bio_integrity_free(struct bio *bio) { } -static inline int blk_integrity_add(struct gendisk *disk) -{ - return 0; -} -static inline void blk_integrity_del(struct gendisk *disk) -{ -} #endif /* CONFIG_BLK_DEV_INTEGRITY */
unsigned long blk_rq_timeout(unsigned long timeout); diff --git a/block/genhd.c b/block/genhd.c index 4d28f1d5f9b0..88d1a6385a24 100644 --- a/block/genhd.c +++ b/block/genhd.c @@ -467,15 +467,11 @@ int device_add_disk(struct device *parent, struct gendisk *disk, */ pm_runtime_set_memalloc_noio(ddev, true);
- ret = blk_integrity_add(disk); - if (ret) - goto out_del_block_link; - disk->part0->bd_holder_dir = kobject_create_and_add("holders", &ddev->kobj); if (!disk->part0->bd_holder_dir) { ret = -ENOMEM; - goto out_del_integrity; + goto out_del_block_link; } disk->slave_dir = kobject_create_and_add("slaves", &ddev->kobj); if (!disk->slave_dir) { @@ -535,8 +531,6 @@ int device_add_disk(struct device *parent, struct gendisk *disk, disk->slave_dir = NULL; out_put_holder_dir: kobject_put(disk->part0->bd_holder_dir); -out_del_integrity: - blk_integrity_del(disk); out_del_block_link: if (!sysfs_deprecated) sysfs_remove_link(block_depr, dev_name(ddev)); @@ -592,7 +586,6 @@ void del_gendisk(struct gendisk *disk) if (WARN_ON_ONCE(!disk_live(disk) && !(disk->flags & GENHD_FL_HIDDEN))) return;
- blk_integrity_del(disk); disk_del_events(disk);
mutex_lock(&disk->open_mutex); @@ -1084,6 +1077,9 @@ static struct attribute_group disk_attr_group = {
static const struct attribute_group *disk_attr_groups[] = { &disk_attr_group, +#ifdef CONFIG_BLK_DEV_INTEGRITY + &blk_integrity_attr_group, +#endif NULL };
diff --git a/include/linux/genhd.h b/include/linux/genhd.h index 690b7f7996d1..3f49a3a30e9b 100644 --- a/include/linux/genhd.h +++ b/include/linux/genhd.h @@ -144,9 +144,6 @@ struct gendisk { struct timer_rand_state *random; atomic_t sync_io; /* RAID */ struct disk_events *ev; -#ifdef CONFIG_BLK_DEV_INTEGRITY - struct kobject integrity_kobj; -#endif /* CONFIG_BLK_DEV_INTEGRITY */ #if IS_ENABLED(CONFIG_CDROM) struct cdrom_device_info *cdi; #endif
linux-stable-mirror@lists.linaro.org
-
Thadeu Lima de Souza Cascardo