The fbdev sysfs attributes are registered after sending the uevent for
the device creation, leaving a race window where e.g. udev rules may
not be able to access the sysfs attributes because the registration is
not done yet.

Fix this by switching to device_create_with_groups(). This also results in
a nice cleanup. After switching to device_create_with_groups() all that
is left of fb_init_device() is setting the drvdata and that can be passed
to device_create[_with_groups]() too. After which fb_init_device() can
be completely removed.

Dropping fb_init_device() + fb_cleanup_device() in turn allows removing
fb_info.class_flag as they were the only user of this field.

Fixes: 5fc830d6aca1 ("fbdev: Register sysfs groups through device_add_group")
Cc: [email protected]
Cc: Shixiong Ou <[email protected]>
Signed-off-by: Hans de Goede <[email protected]>
---
Note the fixes tag is technically wrong. This race has existed forever.
The commit I picked for the fixes tag is a dependency of this change not
the commit introducing the race. I don't believe that backporting this
back any further is useful which is why I went with this commit.
---
 drivers/video/fbdev/core/fbsysfs.c | 36 +++---------------------------
 include/linux/fb.h                 |  1 -
 2 files changed, 3 insertions(+), 34 deletions(-)

diff --git a/drivers/video/fbdev/core/fbsysfs.c 
b/drivers/video/fbdev/core/fbsysfs.c
index b8344c40073b..baa2bae0fb5b 100644
--- a/drivers/video/fbdev/core/fbsysfs.c
+++ b/drivers/video/fbdev/core/fbsysfs.c
@@ -12,8 +12,6 @@
 
 #include "fb_internal.h"
 
-#define FB_SYSFS_FLAG_ATTR 1
-
 static int activate(struct fb_info *fb_info, struct fb_var_screeninfo *var)
 {
        int err;
@@ -451,33 +449,7 @@ static struct attribute *fb_device_attrs[] = {
        NULL,
 };
 
-static const struct attribute_group fb_device_attr_group = {
-       .attrs          = fb_device_attrs,
-};
-
-static int fb_init_device(struct fb_info *fb_info)
-{
-       int ret;
-
-       dev_set_drvdata(fb_info->dev, fb_info);
-
-       fb_info->class_flag |= FB_SYSFS_FLAG_ATTR;
-
-       ret = device_add_group(fb_info->dev, &fb_device_attr_group);
-       if (ret)
-               fb_info->class_flag &= ~FB_SYSFS_FLAG_ATTR;
-
-       return 0;
-}
-
-static void fb_cleanup_device(struct fb_info *fb_info)
-{
-       if (fb_info->class_flag & FB_SYSFS_FLAG_ATTR) {
-               device_remove_group(fb_info->dev, &fb_device_attr_group);
-
-               fb_info->class_flag &= ~FB_SYSFS_FLAG_ATTR;
-       }
-}
+ATTRIBUTE_GROUPS(fb_device);
 
 int fb_device_create(struct fb_info *fb_info)
 {
@@ -485,14 +457,13 @@ int fb_device_create(struct fb_info *fb_info)
        dev_t devt = MKDEV(FB_MAJOR, node);
        int ret;
 
-       fb_info->dev = device_create(fb_class, fb_info->device, devt, NULL, 
"fb%d", node);
+       fb_info->dev = device_create_with_groups(fb_class, fb_info->device, 
devt, fb_info,
+                                                fb_device_groups, "fb%d", 
node);
        if (IS_ERR(fb_info->dev)) {
                /* Not fatal */
                ret = PTR_ERR(fb_info->dev);
                pr_warn("Unable to create device for framebuffer %d; error 
%d\n", node, ret);
                fb_info->dev = NULL;
-       } else {
-               fb_init_device(fb_info);
        }
 
        return 0;
@@ -505,7 +476,6 @@ void fb_device_destroy(struct fb_info *fb_info)
        if (!fb_info->dev)
                return;
 
-       fb_cleanup_device(fb_info);
        device_destroy(fb_class, devt);
        fb_info->dev = NULL;
 }
diff --git a/include/linux/fb.h b/include/linux/fb.h
index 05cc251035da..c3302d513546 100644
--- a/include/linux/fb.h
+++ b/include/linux/fb.h
@@ -497,7 +497,6 @@ struct fb_info {
 #if defined(CONFIG_FB_DEVICE)
        struct device *dev;             /* This is this fb device */
 #endif
-       int class_flag;                    /* private sysfs flags */
 #ifdef CONFIG_FB_TILEBLITTING
        struct fb_tile_ops *tileops;    /* Tile Blitting */
 #endif
-- 
2.52.0

Reply via email to