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: stable@vger.kernel.org Cc: Shixiong Ou oushixiong@kylinos.cn Signed-off-by: Hans de Goede johannes.goede@oss.qualcomm.com --- 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