| Message ID | 20240827-efifb-sysfs-v1-2-c9cc3e052180@weissschuh.net (mailing list archive) |
|---|---|
| State | Accepted, archived |
| Headers | show |
| Series | fbdev: devm_register_framebuffer() and some fixes for efifb | expand |
On 8/27/24 17:25, Thomas Weißschuh wrote: > The driver core can register and cleanup sysfs groups already. > Make use of that functionality to simplify the error handling and > cleanup. > > Also avoid a UAF race during unregistering where the sysctl attributes > were usable after the info struct was freed. > > Signed-off-by: Thomas Weißschuh <linux@weissschuh.net> I've added your patches #2, #3, #4 and #5 of this series to the fbdev git tree. I tend to not take patch #1 as already explained there... Thanks! Helge > --- > drivers/video/fbdev/efifb.c | 11 ++--------- > 1 file changed, 2 insertions(+), 9 deletions(-) > > diff --git a/drivers/video/fbdev/efifb.c b/drivers/video/fbdev/efifb.c > index 8bfe0ccbc67a..d36b95856dd0 100644 > --- a/drivers/video/fbdev/efifb.c > +++ b/drivers/video/fbdev/efifb.c > @@ -561,15 +561,10 @@ static int efifb_probe(struct platform_device *dev) > break; > } > > - err = sysfs_create_groups(&dev->dev.kobj, efifb_groups); > - if (err) { > - pr_err("efifb: cannot add sysfs attrs\n"); > - goto err_unmap; > - } > err = fb_alloc_cmap(&info->cmap, 256, 0); > if (err < 0) { > pr_err("efifb: cannot allocate colormap\n"); > - goto err_groups; > + goto err_unmap; > } > > err = devm_aperture_acquire_for_platform_device(dev, par->base, par->size); > @@ -587,8 +582,6 @@ static int efifb_probe(struct platform_device *dev) > > err_fb_dealloc_cmap: > fb_dealloc_cmap(&info->cmap); > -err_groups: > - sysfs_remove_groups(&dev->dev.kobj, efifb_groups); > err_unmap: > if (mem_flags & (EFI_MEMORY_UC | EFI_MEMORY_WC)) > iounmap(info->screen_base); > @@ -608,12 +601,12 @@ static void efifb_remove(struct platform_device *pdev) > > /* efifb_destroy takes care of info cleanup */ > unregister_framebuffer(info); > - sysfs_remove_groups(&pdev->dev.kobj, efifb_groups); > } > > static struct platform_driver efifb_driver = { > .driver = { > .name = "efi-framebuffer", > + .dev_groups = efifb_groups, > }, > .probe = efifb_probe, > .remove_new = efifb_remove, >
diff --git a/drivers/video/fbdev/efifb.c b/drivers/video/fbdev/efifb.c index 8bfe0ccbc67a..d36b95856dd0 100644 --- a/drivers/video/fbdev/efifb.c +++ b/drivers/video/fbdev/efifb.c @@ -561,15 +561,10 @@ static int efifb_probe(struct platform_device *dev) break; } - err = sysfs_create_groups(&dev->dev.kobj, efifb_groups); - if (err) { - pr_err("efifb: cannot add sysfs attrs\n"); - goto err_unmap; - } err = fb_alloc_cmap(&info->cmap, 256, 0); if (err < 0) { pr_err("efifb: cannot allocate colormap\n"); - goto err_groups; + goto err_unmap; } err = devm_aperture_acquire_for_platform_device(dev, par->base, par->size); @@ -587,8 +582,6 @@ static int efifb_probe(struct platform_device *dev) err_fb_dealloc_cmap: fb_dealloc_cmap(&info->cmap); -err_groups: - sysfs_remove_groups(&dev->dev.kobj, efifb_groups); err_unmap: if (mem_flags & (EFI_MEMORY_UC | EFI_MEMORY_WC)) iounmap(info->screen_base); @@ -608,12 +601,12 @@ static void efifb_remove(struct platform_device *pdev) /* efifb_destroy takes care of info cleanup */ unregister_framebuffer(info); - sysfs_remove_groups(&pdev->dev.kobj, efifb_groups); } static struct platform_driver efifb_driver = { .driver = { .name = "efi-framebuffer", + .dev_groups = efifb_groups, }, .probe = efifb_probe, .remove_new = efifb_remove,
The driver core can register and cleanup sysfs groups already. Make use of that functionality to simplify the error handling and cleanup. Also avoid a UAF race during unregistering where the sysctl attributes were usable after the info struct was freed. Signed-off-by: Thomas Weißschuh <linux@weissschuh.net> --- drivers/video/fbdev/efifb.c | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-)