Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: - [High] sysfs: Use-After-Free in kobject delayed release via devm/memset - [Low] sysfs: incorrect documentation for dss_overlay_kobj_uninit() cleanup --
commit 25c0a8c68e5a115989252d41a0d4731867415d79 Author: Ian Rogers <[email protected]> Subject: kobject: Require explicit initialization of struct kobj_type::release Annotate struct kobj_type::release with __require_explicit_initialization so the compiler diagnoses any struct kobj_type initializer that omits .release. For the remaining legacy struct kobj_type definitions where the kobject is static, per-CPU, or embedded inside a devm- or parent-managed structure rather than freed by kobject_put(), explicitly initialize .release = NULL. Link: https://lpc.events/event/20/contributions/2392/ > diff --git a/drivers/dma/ioat/sysfs.c b/drivers/dma/ioat/sysfs.c > index e796ddb5383f8..7fdd0ca36b98a 100644 > --- a/drivers/dma/ioat/sysfs.c > +++ b/drivers/dma/ioat/sysfs.c > @@ -167,6 +167,8 @@ static const struct attribute *const ioat_attrs[] = { > ATTRIBUTE_GROUPS(ioat); > > const struct kobj_type ioat_ktype = { > + /* ioatdma_chan is allocated via devm_kzalloc() rather than freed by > kobject_put(). */ > + .release = NULL, [Severity: High] Does this setup cause a use-after-free when CONFIG_DEBUG_KOBJECT_RELEASE is enabled? When kobject_put() is called, it queues a delayed_work to execute kobject_cleanup() later. Because the struct is managed by devm_kzalloc(), the memory is freed immediately after unbind, leaving an active delayed_work pointing to the freed memory. [ ... ] > diff --git a/drivers/soc/hisilicon/kunpeng_hccs.c > b/drivers/soc/hisilicon/kunpeng_hccs.c > index 0cc2953d59a54..5d083d2b72caa 100644 > --- a/drivers/soc/hisilicon/kunpeng_hccs.c > +++ b/drivers/soc/hisilicon/kunpeng_hccs.c > @@ -1038,6 +1038,8 @@ static struct attribute *hccs_port_default_attrs[] = { > ATTRIBUTE_GROUPS(hccs_port_default); > > static const struct kobj_type hccs_port_type = { > + /* hccs_port_info is allocated via devm_kcalloc() rather than freed by > kobject_put(). */ > + .release = NULL, [Severity: High] Can this devm allocation cause a use-after-free for the delayed_work? Similar to the ioat_ktype case, the devm_kcalloc allocation means the struct containing the kobject is freed upon driver unbind immediately after kobject_put() executes, leaving a pending delayed_work on freed memory. [ ... ] > diff --git a/drivers/video/fbdev/omap2/omapfb/dss/display-sysfs.c > b/drivers/video/fbdev/omap2/omapfb/dss/display-sysfs.c > index ae937854403b1..3d787245f5b51 100644 > --- a/drivers/video/fbdev/omap2/omapfb/dss/display-sysfs.c > +++ b/drivers/video/fbdev/omap2/omapfb/dss/display-sysfs.c > @@ -304,6 +304,8 @@ static const struct sysfs_ops display_sysfs_ops = { > }; > > static struct kobj_type display_ktype = { > + /* dssdev->kobj is embedded in struct omap_dss_device and zeroed in > display_uninit_sysfs(). */ > + .release = NULL, [Severity: High] Does this zeroing pattern corrupt the active workqueue when CONFIG_DEBUG_KOBJECT_RELEASE is enabled? Looking at display_uninit_sysfs(), the code calls kobject_put() followed immediately by memset: void display_uninit_sysfs(struct omap_dss_device *dssdev) { ... kobject_del(&dssdev->kobj); kobject_put(&dssdev->kobj); memset(&dssdev->kobj, 0, sizeof(dssdev->kobj)); } Since kobject_put() schedules a delayed_work, wouldn't the memset synchronously destroy the active work struct before it can run? [ ... ] > diff --git a/drivers/video/fbdev/omap2/omapfb/dss/overlay-sysfs.c > b/drivers/video/fbdev/omap2/omapfb/dss/overlay-sysfs.c > index 1da4fb1c77b4a..9aa16b3dee5ef 100644 > --- a/drivers/video/fbdev/omap2/omapfb/dss/overlay-sysfs.c > +++ b/drivers/video/fbdev/omap2/omapfb/dss/overlay-sysfs.c > @@ -429,6 +429,8 @@ static const struct sysfs_ops overlay_sysfs_ops = { > }; > > static struct kobj_type overlay_ktype = { > + /* ovl->kobj is embedded in static overlays[]; > dss_overlay_kobj_uninit() only calls kobject_del(). */ > + .release = NULL, [Severity: Low] Is this comment accurate? Looking at dss_overlay_kobj_uninit() in the same file: void dss_overlay_kobj_uninit(struct omap_overlay *ovl) { kobject_del(&ovl->kobj); kobject_put(&ovl->kobj); } The implementation contradicts the comment by explicitly calling kobject_put(). -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=11
