leds: fix attribute-creation races
Subsystems (4)
cover leds: fix attribute-creation races
LED-class device attributes should be created using the attribute groups of struct device, rather than be added manually after the device has been registered in order to avoid racing with userspace.
The first patch adds an attribute groups field to struct led_classdev, which is used during registration to add device attributes in a race-free manner.
The remaining patches convert the current racy LED-device attribute creations to use the new facility. Hopefully my grep-patterns have caught most instances. Note that this also tends to simplify probe error paths.
The lp55xx-common failed to remove the attribute group it is currently creating, something which is fixed separately.
Note that this series include one driver from drivers/input/keyboard.
The individual led-driver patches have been compile tested only.
Johan
Johan Hovold (13):
leds: add led-class attribute-group support
leds: lm3550: fix attribute-creation race
leds: lm3533: fix attribute-creation race
leds: lm355x: fix attribute-creation race
leds: lm3642: fix attribute-creation race
leds: max8997: fix attribute-creation race
leds: netxbig: fix attribute-creation race
leds: ns2: fix attribute-creation race
leds: ss4200: fix attribute-creation race
leds: wm831x-status: fix attribute-creation race
input: lm8323: fix attribute-creation race
leds: lp55xx-common: fix sysfs entry leak
leds: lp55xx-common: fix attribute-creation race
drivers/input/keyboard/lm8323.c | 22 +++++++++------------- drivers/leds/led-class.c | 5 +++-- drivers/leds/leds-lm3530.c | 20 +++++++------------- drivers/leds/leds-lm3533.c | 20 ++++++++------------ drivers/leds/leds-lm355x.c | 21 +++++++++------------ drivers/leds/leds-lm3642.c | 30 ++++++++++++++---------------- drivers/leds/leds-lp55xx-common.c | 20 +++----------------- drivers/leds/leds-max8997.c | 16 +++++++--------- drivers/leds/leds-netxbig.c | 26 ++++++++++++-------------- drivers/leds/leds-ns2.c | 16 +++++++--------- drivers/leds/leds-ss4200.c | 14 +++++++++----- drivers/leds/leds-wm831x-status.c | 23 +++++++++-------------- include/linux/leds.h | 2 ++ 13 files changed, 99 insertions(+), 136 deletions(-)
Signature · 1 line
1.8.5.5
PATCH 1/13 leds: add led-class attribute-group support
Allow led-class devices to be created with optional attribute groups.
This is needed in order to allow led drivers to create custom device attributes in a race-free manner.
--- drivers/leds/led-class.c | 5 +++-- include/linux/leds.h | 2 ++ 2 files changed, 5 insertions(+), 2 deletions(-)
drivers/leds/led-class.c +3 -2
@@ -210,8 +210,9 @@ static const struct dev_pm_ops leds_class_dev_pm_ops = {*/ int led_classdev_register (struct device *parent,struct led_classdev *led_cdev) { - led_cdev->dev =device_create (leds_class, parent,0 , led_cdev, -"%s" , led_cdev->name); + led_cdev->dev =device_create_with_groups (leds_class, parent,0 , + led_cdev, led_cdev->groups, +"%s" , led_cdev->name);if (IS_ERR (led_cdev->dev))return PTR_ERR (led_cdev->dev);
include/linux/leds.h +2 -0
@@ -63,6 +63,8 @@ struct led_classdev { unsigned long *delay_off);struct device *dev; +const struct attribute_group **groups; +struct list_head node;/* LED Device list */ const char *default_trigger;/* Trigger to use */
Signature · 1 line
1.8.5.5
PATCH 2/13 leds: lm3550: fix attribute-creation race
In reply to Johan HovoldUse the attribute groups of the led-class to create the mode attribute during probe in order to avoid racing with userspace.
--- drivers/leds/leds-lm3530.c | 20 +++++++------------- 1 file changed, 7 insertions(+), 13 deletions(-)
drivers/leds/leds-lm3530.c +7 -13
@@ -400,6 +400,12 @@ static ssize_t lm3530_mode_set(struct device *dev, struct device_attribute } static DEVICE_ATTR(mode, 0644, lm3530_mode_get, lm3530_mode_set); +static struct attribute *lm3530_attrs[] = { + &dev_attr_mode.attr, +NULL +}; +ATTRIBUTE_GROUPS (lm3530); +static int lm3530_probe (struct i2c_client *client,const struct i2c_device_id *id) {
@@ -436,6 +442,7 @@ static int lm3530_probe(struct i2c_client *client, drvdata->led_dev.name =LM3530_LED_DEV ; drvdata->led_dev.brightness_set = lm3530_brightness_set; drvdata->led_dev.max_brightness =MAX_BRIGHTNESS ; + drvdata->led_dev.groups = lm3530_groups;i2c_set_clientdata (client, drvdata);
@@ -461,26 +468,13 @@ static int lm3530_probe(struct i2c_client *client,return err; } - err =device_create_file (drvdata->led_dev.dev, &dev_attr_mode); -if (err <0 ) { -dev_err (&client->dev,"File device creation failed: %d\n" , err); - err = -ENODEV ; - goto err_create_file; - } -return 0 ; - -err_create_file: -led_classdev_unregister (&drvdata->led_dev); -return err; }static int lm3530_remove (struct i2c_client *client) {struct lm3530_data *drvdata =i2c_get_clientdata (client); -device_remove_file (drvdata->led_dev.dev, &dev_attr_mode); -lm3530_led_disable (drvdata);led_classdev_unregister (&drvdata->led_dev);return 0 ;
Signature · 1 line
1.8.5.5
PATCH 3/13 leds: lm3533: fix attribute-creation race
In reply to Johan HovoldUse the attribute groups of the led-class to create the attributes during probe in order to avoid racing with userspace.
--- drivers/leds/leds-lm3533.c | 20 ++++++++------------ 1 file changed, 8 insertions(+), 12 deletions(-)
drivers/leds/leds-lm3533.c +8 -12
@@ -645,6 +645,11 @@ static struct attribute_group lm3533_led_attribute_group = { .attrs = lm3533_led_attributes }; +static const struct attribute_group *lm3533_led_attribute_groups[] = { + &lm3533_led_attribute_group, +NULL +}; +static int lm3533_led_setup (struct lm3533_led *led,struct lm3533_led_platform_data *pdata) {
@@ -692,6 +697,7 @@ static int lm3533_led_probe(struct platform_device *pdev) led->cdev.brightness_get = lm3533_led_get; led->cdev.blink_set = lm3533_led_blink_set; led->cdev.brightness =LED_OFF ; + led->cdev.groups = lm3533_led_attribute_groups, led->id = pdev->id;mutex_init (&led->mutex);
@@ -715,25 +721,16 @@ static int lm3533_led_probe(struct platform_device *pdev) led->cb.dev = led->cdev.dev; - ret =sysfs_create_group (&led->cdev.dev->kobj, - &lm3533_led_attribute_group); -if (ret <0 ) { -dev_err (&pdev->dev,"failed to create sysfs attributes\n" ); - goto err_unregister; - } - ret =lm3533_led_setup (led, pdata);if (ret) - goto err_sysfs_remove; + goto err_unregister; ret =lm3533_ctrlbank_enable (&led->cb);if (ret) - goto err_sysfs_remove; + goto err_unregister;return 0 ; -err_sysfs_remove: -sysfs_remove_group (&led->cdev.dev->kobj, &lm3533_led_attribute_group); err_unregister:led_classdev_unregister (&led->cdev);flush_work (&led->work);
@@ -748,7 +745,6 @@ static int lm3533_led_remove(struct platform_device *pdev)dev_dbg (&pdev->dev,"%s\n" , __func__);lm3533_ctrlbank_disable (&led->cb); -sysfs_remove_group (&led->cdev.dev->kobj, &lm3533_led_attribute_group);led_classdev_unregister (&led->cdev);flush_work (&led->work);
Signature · 1 line
1.8.5.5
PATCH 4/13 leds: lm355x: fix attribute-creation race
In reply to Johan HovoldUse the attribute groups of the led-class to create the indicator-LED attributes during probe in order to avoid racing with userspace.
Note that the comment about the pattern attribute only being for LM3554 was incorrect and did not match the code (the original leds-lm3556 driver had the attribute before LM3554 support was added).
--- drivers/leds/leds-lm355x.c | 21 +++++++++------------ 1 file changed, 9 insertions(+), 12 deletions(-)
drivers/leds/leds-lm355x.c +9 -12
@@ -413,6 +413,12 @@ out: static DEVICE_ATTR(pattern, S_IWUSR, NULL, lm3556_indicator_pattern_store); +static struct attribute *lm355x_indicator_attrs[] = { + &dev_attr_pattern.attr, +NULL +}; +ATTRIBUTE_GROUPS (lm355x_indicator); +static const struct regmap_config lm355x_regmap = { .reg_bits =8 , .val_bits =8 ,
@@ -501,25 +507,18 @@ static int lm355x_probe(struct i2c_client *client,else chip->cdev_indicator.max_brightness =8 ; chip->cdev_indicator.brightness_set = lm355x_indicator_brightness_set; +/* indicator pattern control only for LM3556 */ +if (id->driver_data ==CHIP_LM3556 ) + chip->cdev_indicator.groups = lm355x_indicator_groups; err =led_classdev_register ((struct device *) &client->dev, &chip->cdev_indicator);if (err <0 ) goto err_create_indicator_file; -/* indicator pattern control only for LM3554 */ -if (id->driver_data ==CHIP_LM3556 ) { - err = -device_create_file (chip->cdev_indicator.dev, - &dev_attr_pattern); -if (err <0 ) - goto err_create_pattern_file; - }dev_info (&client->dev,"%s is initialized\n" , lm355x_name[id->driver_data]);return 0 ; -err_create_pattern_file: -led_classdev_unregister (&chip->cdev_indicator); err_create_indicator_file:led_classdev_unregister (&chip->cdev_torch); err_create_torch_file:
@@ -534,8 +533,6 @@ static int lm355x_remove(struct i2c_client *client)struct lm355x_reg_data *preg = chip->regs;regmap_write (chip->regmap, preg[REG_OPMODE ].regno,0 ); -if (chip->type ==CHIP_LM3556 ) -device_remove_file (chip->cdev_indicator.dev, &dev_attr_pattern);led_classdev_unregister (&chip->cdev_indicator);flush_work (&chip->work_indicator);led_classdev_unregister (&chip->cdev_torch);
Signature · 1 line
1.8.5.5
PATCH 5/13 leds: lm3642: fix attribute-creation race
In reply to Johan HovoldUse the attribute groups of the led-class to create the flash and torch-LED attributes during probe in order to avoid racing with userspace.
--- drivers/leds/leds-lm3642.c | 30 ++++++++++++++---------------- 1 file changed, 14 insertions(+), 16 deletions(-)
drivers/leds/leds-lm3642.c +14 -16
@@ -313,6 +313,18 @@ static const struct regmap_config lm3642_regmap = { .max_register = REG_MAX, }; +static struct attribute *lm3642_flash_attrs[] = { + &dev_attr_strobe_pin.attr, +NULL +}; +ATTRIBUTE_GROUPS (lm3642_flash); + +static struct attribute *lm3642_torch_attrs[] = { + &dev_attr_torch_pin.attr, +NULL +}; +ATTRIBUTE_GROUPS (lm3642_torch); +static int lm3642_probe (struct i2c_client *client,const struct i2c_device_id *id) {
@@ -364,17 +376,13 @@ static int lm3642_probe(struct i2c_client *client, chip->cdev_flash.max_brightness =16 ; chip->cdev_flash.brightness_set = lm3642_strobe_brightness_set; chip->cdev_flash.default_trigger ="flash" ; + chip->cdev_flash.groups = lm3642_flash_groups, err =led_classdev_register ((struct device *) &client->dev, &chip->cdev_flash);if (err <0 ) {dev_err (chip->dev,"failed to register flash\n" ); goto err_out; } - err =device_create_file (chip->cdev_flash.dev, &dev_attr_strobe_pin); -if (err <0 ) { -dev_err (chip->dev,"failed to create strobe-pin file\n" ); - goto err_create_flash_pin_file; - }/* torch */ INIT_WORK (&chip->work_torch, lm3642_deferred_torch_brightness_set);
@@ -382,17 +390,13 @@ static int lm3642_probe(struct i2c_client *client, chip->cdev_torch.max_brightness =8 ; chip->cdev_torch.brightness_set = lm3642_torch_brightness_set; chip->cdev_torch.default_trigger ="torch" ; + chip->cdev_torch.groups = lm3642_torch_groups, err =led_classdev_register ((struct device *) &client->dev, &chip->cdev_torch);if (err <0 ) {dev_err (chip->dev,"failed to register torch\n" ); goto err_create_torch_file; } - err =device_create_file (chip->cdev_torch.dev, &dev_attr_torch_pin); -if (err <0 ) { -dev_err (chip->dev,"failed to create torch-pin file\n" ); - goto err_create_torch_pin_file; - }/* indicator */ INIT_WORK(&chip->work_indicator,
@@ -411,12 +415,8 @@ static int lm3642_probe(struct i2c_client *client,return 0 ; err_create_indicator_file: -device_remove_file (chip->cdev_torch.dev, &dev_attr_torch_pin); -err_create_torch_pin_file:led_classdev_unregister (&chip->cdev_torch); err_create_torch_file: -device_remove_file (chip->cdev_flash.dev, &dev_attr_strobe_pin); -err_create_flash_pin_file:led_classdev_unregister (&chip->cdev_flash); err_out:return err;
@@ -428,10 +428,8 @@ static int lm3642_remove(struct i2c_client *client)led_classdev_unregister (&chip->cdev_indicator);flush_work (&chip->work_indicator); -device_remove_file (chip->cdev_torch.dev, &dev_attr_torch_pin);led_classdev_unregister (&chip->cdev_torch);flush_work (&chip->work_torch); -device_remove_file (chip->cdev_flash.dev, &dev_attr_strobe_pin);led_classdev_unregister (&chip->cdev_flash);flush_work (&chip->work_flash);regmap_write (chip->regmap,REG_ENABLE ,0 );
Signature · 1 line
1.8.5.5
PATCH 6/13 leds: max8997: fix attribute-creation race
In reply to Johan HovoldUse the attribute groups of the led-class to create the mode attribute during probe in order to avoid racing with userspace.
--- drivers/leds/leds-max8997.c | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-)
drivers/leds/leds-max8997.c +7 -9
@@ -229,6 +229,12 @@ static ssize_t max8997_led_store_mode(struct device *dev, static DEVICE_ATTR(mode, 0644, max8997_led_show_mode, max8997_led_store_mode); +static struct attribute *max8997_attrs[] = { + &dev_attr_mode.attr, +NULL +}; +ATTRIBUTE_GROUPS (max8997); +static int max8997_led_probe (struct platform_device *pdev) {struct max8997_dev *iodev =dev_get_drvdata (pdev->dev.parent);
@@ -253,6 +259,7 @@ static int max8997_led_probe(struct platform_device *pdev) led->cdev.brightness_set = max8997_led_brightness_set; led->cdev.flags |=LED_CORE_SUSPENDRESUME ; led->cdev.brightness =0 ; + led->cdev.groups = max8997_groups; led->iodev = iodev;/* initialize mode and brightness according to platform_data */
@@ -281,14 +288,6 @@ static int max8997_led_probe(struct platform_device *pdev)if (ret <0 )return ret; - ret =device_create_file (led->cdev.dev, &dev_attr_mode); -if (ret !=0 ) { -dev_err (&pdev->dev, -"failed to create file: %d\n" , ret); -led_classdev_unregister (&led->cdev); -return ret; - } -return 0 ; }
@@ -296,7 +295,6 @@ static int max8997_led_remove(struct platform_device *pdev) {struct max8997_led *led =platform_get_drvdata (pdev); -device_remove_file (led->cdev.dev, &dev_attr_mode);led_classdev_unregister (&led->cdev);return 0 ;
Signature · 1 line
1.8.5.5
PATCH 7/13 leds: netxbig: fix attribute-creation race
In reply to Johan HovoldUse the attribute groups of the led-class to create the sata attribute during probe in order to avoid racing with userspace.
--- drivers/leds/leds-netxbig.c | 26 ++++++++++++-------------- 1 file changed, 12 insertions(+), 14 deletions(-)
drivers/leds/leds-netxbig.c +12 -14
@@ -293,10 +293,14 @@ static ssize_t netxbig_led_sata_show(struct device *dev, static DEVICE_ATTR(sata, 0644, netxbig_led_sata_show, netxbig_led_sata_store); +static struct attribute *netxbig_led_attrs[] = { + &dev_attr_sata.attr, +NULL +}; +ATTRIBUTE_GROUPS (netxbig_led); +static void delete_netxbig_led (struct netxbig_led_data *led_dat) { -if (led_dat->mode_val[NETXBIG_LED_SATA ] !=NETXBIG_LED_INVALID_MODE ) -device_remove_file (led_dat->cdev.dev, &dev_attr_sata);led_classdev_unregister (&led_dat->cdev); }
@@ -327,6 +331,12 @@ create_netxbig_led(struct platform_device *pdev, led_dat->sata =0 ; led_dat->cdev.brightness =LED_OFF ; led_dat->cdev.flags |=LED_CORE_SUSPENDRESUME ; +/* +* If available, expose the SATA activity blink capability through +* a "sata" sysfs attribute. +*/ +if (led_dat->mode_val[NETXBIG_LED_SATA ] !=NETXBIG_LED_INVALID_MODE ) + led_dat->cdev.groups = netxbig_led_groups; led_dat->mode_addr = template->mode_addr; led_dat->mode_val = template->mode_val; led_dat->bright_addr = template->bright_addr;
@@ -335,18 +345,6 @@ create_netxbig_led(struct platform_device *pdev, led_dat->num_timer = pdata->num_timer; ret =led_classdev_register (&pdev->dev, &led_dat->cdev); -if (ret <0 ) -return ret; - -/* -* If available, expose the SATA activity blink capability through -* a "sata" sysfs attribute. -*/ -if (led_dat->mode_val[NETXBIG_LED_SATA ] !=NETXBIG_LED_INVALID_MODE ) { - ret =device_create_file (led_dat->cdev.dev, &dev_attr_sata); -if (ret) -led_classdev_unregister (&led_dat->cdev); - }return ret; }
Signature · 1 line
1.8.5.5
Re: [PATCH 07/13] leds: netxbig: fix attribute-creation race
Quoted 43 lines · Johan Hovold
Use the attribute groups of the led-class to create the sata attribute during probe in order to avoid racing with userspace.
Signed-off-by Johan Hovold <[email protected]>--- drivers/leds/leds-netxbig.c | 26 ++++++++++++-------------- 1 file changed, 12 insertions(+), 14 deletions(-)drivers/leds/leds-netxbig.c +12
-2@@ -293,10 +293,14 @@ static ssize_t netxbig_led_sata_show(struct device *dev, static DEVICE_ATTR(sata, 0644, netxbig_led_sata_show, netxbig_led_sata_store); +static struct attribute *netxbig_led_attrs[] = { + &dev_attr_sata.attr, +NULL +}; +ATTRIBUTE_GROUPS (netxbig_led); +static void delete_netxbig_led (struct netxbig_led_data *led_dat) { -if (led_dat->mode_val[NETXBIG_LED_SATA ] !=NETXBIG_LED_INVALID_MODE ) -device_remove_file (led_dat->cdev.dev, &dev_attr_sata);led_classdev_unregister (&led_dat->cdev); }@@ -327,6 +331,12 @@ create_netxbig_led(struct platform_device *pdev, led_dat->sata =0 ; led_dat->cdev.brightness =LED_OFF ; led_dat->cdev.flags |=LED_CORE_SUSPENDRESUME ; +/* +* If available, expose the SATA activity blink capability through +* a "sata" sysfs attribute. +*/ +if (led_dat->mode_val[NETXBIG_LED_SATA ] !=NETXBIG_LED_INVALID_MODE ) + led_dat->cdev.groups = netxbig_led_groups; led_dat->mode_addr = template->mode_addr; led_dat->mode_val = template->mode_val; led_dat->bright_addr = template->bright_addr;
drivers/leds/leds-netxbig.c create_netxbig_led(struct platform_device *pdev, · line 347
Quoted 3 lines
drivers/leds/leds-netxbig.c +0
-0@@ -335,18 +345,6 @@ create_netxbig_led(struct platform_device *pdev, led_dat->num_timer = pdata->num_timer; ret =led_classdev_register (&pdev->dev, &led_dat->cdev);
Let's just "return led_classdev_register(&pdev->dev, &led_dat->cdev);" and remove "ret"
I will handle this trivial change.
-Bryan
Quoted 18 lines
drivers/leds/leds-netxbig.c +0
-13-if (ret <0 ) -return ret; - -/* -* If available, expose the SATA activity blink capability through -* a "sata" sysfs attribute. -*/ -if (led_dat->mode_val[NETXBIG_LED_SATA ] !=NETXBIG_LED_INVALID_MODE ) { - ret =device_create_file (led_dat->cdev.dev, &dev_attr_sata); -if (ret) -led_classdev_unregister (&led_dat->cdev); - }return ret; } --1.8.5.5
PATCH 8/13 leds: ns2: fix attribute-creation race
In reply to Johan HovoldUse the attribute groups of the led-class to create the sata attribute during probe in order to avoid racing with userspace.
--- drivers/leds/leds-ns2.c | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-)
drivers/leds/leds-ns2.c +7 -9
@@ -185,6 +185,12 @@ static ssize_t ns2_led_sata_show(struct device *dev, static DEVICE_ATTR(sata, 0644, ns2_led_sata_show, ns2_led_sata_store); +static struct attribute *ns2_led_attrs[] = { + &dev_attr_sata.attr, +NULL +}; +ATTRIBUTE_GROUPS (ns2_led); +static int create_ns2_led (struct platform_device *pdev,struct ns2_led_data *led_dat,const struct ns2_led *template)
@@ -219,6 +225,7 @@ create_ns2_led(struct platform_device *pdev, struct ns2_led_data *led_dat, led_dat->cdev.blink_set =NULL ; led_dat->cdev.brightness_set = ns2_led_set; led_dat->cdev.flags |=LED_CORE_SUSPENDRESUME ; + led_dat->cdev.groups = ns2_led_groups; led_dat->cmd = template->cmd; led_dat->slow = template->slow;
@@ -235,20 +242,11 @@ create_ns2_led(struct platform_device *pdev, struct ns2_led_data *led_dat,if (ret <0 )return ret; - ret =device_create_file (led_dat->cdev.dev, &dev_attr_sata); -if (ret <0 ) - goto err_free_cdev; -return 0 ; - -err_free_cdev: -led_classdev_unregister (&led_dat->cdev); -return ret; }static void delete_ns2_led (struct ns2_led_data *led_dat) { -device_remove_file (led_dat->cdev.dev, &dev_attr_sata);led_classdev_unregister (&led_dat->cdev); }
Signature · 1 line
1.8.5.5
PATCH 9/13 leds: ss4200: fix attribute-creation race
In reply to Johan HovoldUse the attribute groups of the led-class to create the blink attribute during probe in order to avoid racing with userspace.
--- drivers/leds/leds-ss4200.c | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-)
drivers/leds/leds-ss4200.c +9 -5
@@ -469,6 +469,12 @@ static ssize_t nas_led_blink_store(struct device *dev, static DEVICE_ATTR(blink, 0644, nas_led_blink_show, nas_led_blink_store); +static struct attribute *nasgpio_led_attrs[] = { + &dev_attr_blink.attr, +NULL +}; +ATTRIBUTE_GROUPS (nasgpio_led); +static int register_nasgpio_led (int led_nr) {int ret;
@@ -481,20 +487,18 @@ static int register_nasgpio_led(int led_nr) led->brightness =LED_FULL ; led->brightness_set = nasgpio_led_set_brightness; led->blink_set = nasgpio_led_set_blink; + led->groups = nasgpio_led_groups; ret =led_classdev_register (&nas_gpio_pci_dev->dev, led);if (ret)return ret; - ret =device_create_file (led->dev, &dev_attr_blink); -if (ret) -led_classdev_unregister (led); -return ret; + +return 0 ; }static void unregister_nasgpio_led (int led_nr) {struct led_classdev *led =get_classdev_for_led_nr (led_nr);led_classdev_unregister (led); -device_remove_file (led->dev, &dev_attr_blink); }/* * module load/initialization
Signature · 1 line
1.8.5.5
PATCH 10/13 leds: wm831x-status: fix attribute-creation race
In reply to Johan HovoldUse the attribute groups of the led-class to create the src attribute during probe in order to avoid racing with userspace.
--- drivers/leds/leds-wm831x-status.c | 23 +++++++++-------------- 1 file changed, 9 insertions(+), 14 deletions(-)
drivers/leds/leds-wm831x-status.c +9 -14
@@ -219,6 +219,12 @@ static ssize_t wm831x_status_src_store(struct device *dev, static DEVICE_ATTR(src, 0644, wm831x_status_src_show, wm831x_status_src_store); +static struct attribute *wm831x_status_attrs[] = { + &dev_attr_src.attr, +NULL +}; +ATTRIBUTE_GROUPS (wm831x_status); +static int wm831x_status_probe (struct platform_device *pdev) {struct wm831x *wm831x =dev_get_drvdata (pdev->dev.parent);
@@ -232,8 +238,7 @@ static int wm831x_status_probe(struct platform_device *pdev) res =platform_get_resource (pdev,IORESOURCE_REG ,0 );if (res ==NULL ) {dev_err (&pdev->dev,"No register resource\n" ); - ret = -EINVAL ; - goto err; +return -EINVAL ; } drvdata = devm_kzalloc(&pdev->dev, sizeof(struct wm831x_status),
@@ -284,31 +289,21 @@ static int wm831x_status_probe(struct platform_device *pdev) drvdata->cdev.default_trigger = pdata.default_trigger; drvdata->cdev.brightness_set = wm831x_status_set; drvdata->cdev.blink_set = wm831x_status_blink_set; + drvdata->cdev.groups = wm831x_status_groups; ret =led_classdev_register (wm831x->dev, &drvdata->cdev);if (ret <0 ) {dev_err (&pdev->dev,"Failed to register LED: %d\n" , ret); - goto err_led; +return ret; } - ret =device_create_file (drvdata->cdev.dev, &dev_attr_src); -if (ret !=0 ) -dev_err (&pdev->dev, -"No source control for LED: %d\n" , ret); -return 0 ; - -err_led: -led_classdev_unregister (&drvdata->cdev); -err: -return ret; }static int wm831x_status_remove (struct platform_device *pdev) {struct wm831x_status *drvdata =platform_get_drvdata (pdev); -device_remove_file (drvdata->cdev.dev, &dev_attr_src);led_classdev_unregister (&drvdata->cdev);return 0 ;
Signature · 1 line
1.8.5.5
PATCH 11/13 input: lm8323: fix attribute-creation race
In reply to Johan HovoldUse the attribute groups of the led-class to create the time attribute during probe in order to avoid racing with userspace.
--- drivers/input/keyboard/lm8323.c | 22 +++++++++------------- 1 file changed, 9 insertions(+), 13 deletions(-)
drivers/input/keyboard/lm8323.c +9 -13
@@ -558,6 +558,12 @@ static ssize_t lm8323_pwm_store_time(struct device *dev, } static DEVICE_ATTR(time, 0644, lm8323_pwm_show_time, lm8323_pwm_store_time); +static struct attribute *lm8323_pwm_attrs[] = { + &dev_attr_time.attr, +NULL +}; +ATTRIBUTE_GROUPS (lm8323_pwm); +static int init_pwm (struct lm8323_chip *lm,int id,struct device *dev,const char *name) {
@@ -580,16 +586,11 @@ static int init_pwm(struct lm8323_chip *lm, int id, struct device *dev,if (name) { pwm->cdev.name = name; pwm->cdev.brightness_set = lm8323_pwm_set_brightness; + pwm->cdev.groups = lm8323_pwm_groups;if (led_classdev_register (dev, &pwm->cdev) <0 ) {dev_err (dev,"couldn't register PWM %d\n" , id);return -1 ; } -if (device_create_file (pwm->cdev.dev, - &dev_attr_time) <0 ) { -dev_err (dev,"couldn't register time attribute\n" ); -led_classdev_unregister (&pwm->cdev); -return -1 ; - } pwm->enabled = true; }
@@ -753,11 +754,8 @@ fail3:device_remove_file (&client->dev, &dev_attr_disable_kp); fail2:while (--pwm >=0 ) -if (lm->pwm[pwm].enabled) { -device_remove_file (lm->pwm[pwm].cdev.dev, - &dev_attr_time); +if (lm->pwm[pwm].enabled)led_classdev_unregister (&lm->pwm[pwm].cdev); - } fail1:input_free_device (idev);kfree (lm);
@@ -777,10 +775,8 @@ static int lm8323_remove(struct i2c_client *client)device_remove_file (&lm->client->dev, &dev_attr_disable_kp);for (i =0 ; i <3 ; i++) -if (lm->pwm[i].enabled) { -device_remove_file (lm->pwm[i].cdev.dev, &dev_attr_time); +if (lm->pwm[i].enabled)led_classdev_unregister (&lm->pwm[i].cdev); - }kfree (lm);
Signature · 1 line
1.8.5.5
Re: [PATCH 11/13] input: lm8323: fix attribute-creation race
Quoted 3 lines · Johan Hovold
Use the attribute groups of the led-class to create the time attribute during probe in order to avoid racing with userspace.
I'm about to merge this, but need an Ack from input subsystem maintainers. Dmitry or Jiri, are you OK with this patch?
-Bryan
Quoted 68 lines
Signed-off-by Johan Hovold <[email protected]>--- drivers/input/keyboard/lm8323.c | 22 +++++++++------------- 1 file changed, 9 insertions(+), 13 deletions(-)drivers/input/keyboard/lm8323.c +9
-14@@ -558,6 +558,12 @@ static ssize_t lm8323_pwm_store_time(struct device *dev, } static DEVICE_ATTR(time, 0644, lm8323_pwm_show_time, lm8323_pwm_store_time); +static struct attribute *lm8323_pwm_attrs[] = { + &dev_attr_time.attr, +NULL +}; +ATTRIBUTE_GROUPS (lm8323_pwm); +static int init_pwm (struct lm8323_chip *lm,int id,struct device *dev,const char *name) {@@ -580,16 +586,11 @@ static int init_pwm(struct lm8323_chip *lm, int id, struct device *dev,if (name) { pwm->cdev.name = name; pwm->cdev.brightness_set = lm8323_pwm_set_brightness; + pwm->cdev.groups = lm8323_pwm_groups;if (led_classdev_register (dev, &pwm->cdev) <0 ) {dev_err (dev,"couldn't register PWM %d\n" , id);return -1 ; } -if (device_create_file (pwm->cdev.dev, - &dev_attr_time) <0 ) { -dev_err (dev,"couldn't register time attribute\n" ); -led_classdev_unregister (&pwm->cdev); -return -1 ; - } pwm->enabled = true; }@@ -753,11 +754,8 @@ fail3:device_remove_file (&client->dev, &dev_attr_disable_kp); fail2:while (--pwm >=0 ) -if (lm->pwm[pwm].enabled) { -device_remove_file (lm->pwm[pwm].cdev.dev, - &dev_attr_time); +if (lm->pwm[pwm].enabled)led_classdev_unregister (&lm->pwm[pwm].cdev); - } fail1:input_free_device (idev);kfree (lm);@@ -777,10 +775,8 @@ static int lm8323_remove(struct i2c_client *client)device_remove_file (&lm->client->dev, &dev_attr_disable_kp);for (i =0 ; i <3 ; i++) -if (lm->pwm[i].enabled) { -device_remove_file (lm->pwm[i].cdev.dev, &dev_attr_time); +if (lm->pwm[i].enabled)led_classdev_unregister (&lm->pwm[i].cdev); - }kfree (lm); --1.8.5.5
Re: [PATCH 11/13] input: lm8323: fix attribute-creation race
In reply to Johan HovoldQuoted 4 lines · Johan Hovold
Use the attribute groups of the led-class to create the time attribute during probe in order to avoid racing with userspace.
Signed-off-by Johan Hovold <[email protected]>
Quoted 65 lines
--- drivers/input/keyboard/lm8323.c | 22 +++++++++------------- 1 file changed, 9 insertions(+), 13 deletions(-)drivers/input/keyboard/lm8323.c +9
-14@@ -558,6 +558,12 @@ static ssize_t lm8323_pwm_store_time(struct device *dev, } static DEVICE_ATTR(time, 0644, lm8323_pwm_show_time, lm8323_pwm_store_time); +static struct attribute *lm8323_pwm_attrs[] = { + &dev_attr_time.attr, +NULL +}; +ATTRIBUTE_GROUPS (lm8323_pwm); +static int init_pwm (struct lm8323_chip *lm,int id,struct device *dev,const char *name) {@@ -580,16 +586,11 @@ static int init_pwm(struct lm8323_chip *lm, int id, struct device *dev,if (name) { pwm->cdev.name = name; pwm->cdev.brightness_set = lm8323_pwm_set_brightness; + pwm->cdev.groups = lm8323_pwm_groups;if (led_classdev_register (dev, &pwm->cdev) <0 ) {dev_err (dev,"couldn't register PWM %d\n" , id);return -1 ; } -if (device_create_file (pwm->cdev.dev, - &dev_attr_time) <0 ) { -dev_err (dev,"couldn't register time attribute\n" ); -led_classdev_unregister (&pwm->cdev); -return -1 ; - } pwm->enabled = true; }@@ -753,11 +754,8 @@ fail3:device_remove_file (&client->dev, &dev_attr_disable_kp); fail2:while (--pwm >=0 ) -if (lm->pwm[pwm].enabled) { -device_remove_file (lm->pwm[pwm].cdev.dev, - &dev_attr_time); +if (lm->pwm[pwm].enabled)led_classdev_unregister (&lm->pwm[pwm].cdev); - } fail1:input_free_device (idev);kfree (lm);@@ -777,10 +775,8 @@ static int lm8323_remove(struct i2c_client *client)device_remove_file (&lm->client->dev, &dev_attr_disable_kp);for (i =0 ; i <3 ; i++) -if (lm->pwm[i].enabled) { -device_remove_file (lm->pwm[i].cdev.dev, &dev_attr_time); +if (lm->pwm[i].enabled)led_classdev_unregister (&lm->pwm[i].cdev); - }kfree (lm); --
Quoted 2 lines
1.8.5.5
Thanks.
Signature · 1 line
Dmitry
Re: [PATCH 11/13] input: lm8323: fix attribute-creation race
Quoted 8 lines · Dmitry Torokhov
Quoted 4 lines · Johan Hovold
Use the attribute groups of the led-class to create the time attribute during probe in order to avoid racing with userspace.
Signed-off-by Johan Hovold <[email protected]>Acked-by Dmitry Torokhov <[email protected]>
Good, I merged this patch with others into my for-next branch.
-Bryan
Quoted 72 lines
Quoted 65 lines
--- drivers/input/keyboard/lm8323.c | 22 +++++++++------------- 1 file changed, 9 insertions(+), 13 deletions(-)drivers/input/keyboard/lm8323.c +9
-14@@ -558,6 +558,12 @@ static ssize_t lm8323_pwm_store_time(struct device *dev, } static DEVICE_ATTR(time, 0644, lm8323_pwm_show_time, lm8323_pwm_store_time); +static struct attribute *lm8323_pwm_attrs[] = { + &dev_attr_time.attr, +NULL +}; +ATTRIBUTE_GROUPS (lm8323_pwm); +static int init_pwm (struct lm8323_chip *lm,int id,struct device *dev,const char *name) {@@ -580,16 +586,11 @@ static int init_pwm(struct lm8323_chip *lm, int id, struct device *dev,if (name) { pwm->cdev.name = name; pwm->cdev.brightness_set = lm8323_pwm_set_brightness; + pwm->cdev.groups = lm8323_pwm_groups;if (led_classdev_register (dev, &pwm->cdev) <0 ) {dev_err (dev,"couldn't register PWM %d\n" , id);return -1 ; } -if (device_create_file (pwm->cdev.dev, - &dev_attr_time) <0 ) { -dev_err (dev,"couldn't register time attribute\n" ); -led_classdev_unregister (&pwm->cdev); -return -1 ; - } pwm->enabled = true; }@@ -753,11 +754,8 @@ fail3:device_remove_file (&client->dev, &dev_attr_disable_kp); fail2:while (--pwm >=0 ) -if (lm->pwm[pwm].enabled) { -device_remove_file (lm->pwm[pwm].cdev.dev, - &dev_attr_time); +if (lm->pwm[pwm].enabled)led_classdev_unregister (&lm->pwm[pwm].cdev); - } fail1:input_free_device (idev);kfree (lm);@@ -777,10 +775,8 @@ static int lm8323_remove(struct i2c_client *client)device_remove_file (&lm->client->dev, &dev_attr_disable_kp);for (i =0 ; i <3 ; i++) -if (lm->pwm[i].enabled) { -device_remove_file (lm->pwm[i].cdev.dev, &dev_attr_time); +if (lm->pwm[i].enabled)led_classdev_unregister (&lm->pwm[i].cdev); - }kfree (lm); --Quoted 2 lines
1.8.5.5
Thanks.
--
Dmitry
PATCH 12/13 leds: lp55xx-common: fix sysfs entry leak
In reply to Johan HovoldMake sure the sysfs group is removed when the LEDs are unregistered.
--- drivers/leds/leds-lp55xx-common.c | 2 ++ 1 file changed, 2 insertions(+)
drivers/leds/leds-lp55xx-common.c +2 -0
@@ -520,6 +520,8 @@ void lp55xx_unregister_leds(struct lp55xx_led *led, struct lp55xx_chip *chip)for (i =0 ; i < chip->num_leds; i++) { each = led + i; +sysfs_remove_group (&each->cdev.dev->kobj, + &lp55xx_led_attr_group);led_classdev_unregister (&each->cdev);flush_work (&each->brightness_work); }
Signature · 1 line
1.8.5.5
PATCH 13/13 leds: lp55xx-common: fix attribute-creation race
In reply to Johan HovoldUse the attribute groups of the led-class to create the LED attributes during probe in order to avoid racing with userspace.
--- drivers/leds/leds-lp55xx-common.c | 22 +++------------------- 1 file changed, 3 insertions(+), 19 deletions(-)
drivers/leds/leds-lp55xx-common.c +3 -19
@@ -127,15 +127,12 @@ static DEVICE_ATTR(led_current, S_IRUGO | S_IWUSR, lp55xx_show_current, lp55xx_store_current); static DEVICE_ATTR(max_current, S_IRUGO , lp55xx_show_max_current, NULL); -static struct attribute *lp55xx_led_attributes[] = { +static struct attribute *lp55xx_led_attrs[] = { &dev_attr_led_current.attr, &dev_attr_max_current.attr,NULL , }; - -static struct attribute_group lp55xx_led_attr_group = { - .attrs = lp55xx_led_attributes -}; +ATTRIBUTE_GROUPS (lp55xx_led);static void lp55xx_set_brightness (struct led_classdev *cdev,enum led_brightness brightness)
@@ -176,6 +173,7 @@ static int lp55xx_init_led(struct lp55xx_led *led, } led->cdev.brightness_set = lp55xx_set_brightness; + led->cdev.groups = lp55xx_led_groups;if (pdata->led_config[chan].name) { led->cdev.name = pdata->led_config[chan].name;
@@ -185,24 +183,12 @@ static int lp55xx_init_led(struct lp55xx_led *led, led->cdev.name = name; } -/* -* register led class device for each channel and -* add device attributes -*/ - ret =led_classdev_register (dev, &led->cdev);if (ret) {dev_err (dev,"led register err: %d\n" , ret);return ret; } - ret =sysfs_create_group (&led->cdev.dev->kobj, &lp55xx_led_attr_group); -if (ret) { -dev_err (dev,"led sysfs err: %d\n" , ret); -led_classdev_unregister (&led->cdev); -return ret; - } -return 0 ; }
@@ -520,8 +506,6 @@ void lp55xx_unregister_leds(struct lp55xx_led *led, struct lp55xx_chip *chip)for (i =0 ; i < chip->num_leds; i++) { each = led + i; -sysfs_remove_group (&each->cdev.dev->kobj, - &lp55xx_led_attr_group);led_classdev_unregister (&each->cdev);flush_work (&each->brightness_work); }
Signature · 1 line
1.8.5.5
Quoted 13 lines · Johan Hovold
LED-class device attributes should be created using the attribute groups of struct device, rather than be added manually after the device has been registered in order to avoid racing with userspace.
The first patch adds an attribute groups field to struct
led_classdev, which is used during registration to add device attributes in a race-free manner.The remaining patches convert the current racy LED-device attribute creations to use the new facility. Hopefully my grep-patterns have caught most instances. Note that this also tends to simplify probe error paths.
Quoted 7 lines
The lp55xx-common failed to remove the attribute group it is currently creating, something which is fixed separately.
Note that this series include one driver from
drivers/input/keyboard.The individual led-driver patches have been compile tested only.
Thanks a lot for driving this. I will applied this patchset into my -devel branch. After I got the Ack from Input guys, I will apply it to my for-next branch then.
-Bryan
Quoted 36 lines
Johan
Johan Hovold (13):
leds: add led-class attribute-group support
leds: lm3550: fix attribute-creation race
leds: lm3533: fix attribute-creation race
leds: lm355x: fix attribute-creation race
leds: lm3642: fix attribute-creation race
leds: max8997: fix attribute-creation race
leds: netxbig: fix attribute-creation race
leds: ns2: fix attribute-creation race
leds: ss4200: fix attribute-creation race
leds: wm831x-status: fix attribute-creation race
input: lm8323: fix attribute-creation race
leds: lp55xx-common: fix sysfs entry leak
leds: lp55xx-common: fix attribute-creation racedrivers/input/keyboard/lm8323.c | 22 +++++++++------------- drivers/leds/led-class.c | 5 +++-- drivers/leds/leds-lm3530.c | 20 +++++++------------- drivers/leds/leds-lm3533.c | 20 ++++++++------------ drivers/leds/leds-lm355x.c | 21 +++++++++------------ drivers/leds/leds-lm3642.c | 30 ++++++++++++++---------------- drivers/leds/leds-lp55xx-common.c | 20 +++----------------- drivers/leds/leds-max8997.c | 16 +++++++--------- drivers/leds/leds-netxbig.c | 26 ++++++++++++-------------- drivers/leds/leds-ns2.c | 16 +++++++--------- drivers/leds/leds-ss4200.c | 14 +++++++++----- drivers/leds/leds-wm831x-status.c | 23 +++++++++-------------- include/linux/leds.h | 2 ++ 13 files changed, 99 insertions(+), 136 deletions(-)--
1.8.5.5
Quoted 17 lines · Bryan Wu
Quoted 16 lines · Johan Hovold
LED-class device attributes should be created using the attribute groups of struct device, rather than be added manually after the device has been registered in order to avoid racing with userspace.
The first patch adds an attribute groups field to struct
led_classdev, which is used during registration to add device attributes in a race-free manner.The remaining patches convert the current racy LED-device attribute creations to use the new facility. Hopefully my grep-patterns have caught most instances. Note that this also tends to simplify probe error paths.
The lp55xx-common failed to remove the attribute group it is currently creating, something which is fixed separately.
Quoted 8 lines
Quoted 4 lines
Note that this series include one driver from
drivers/input/keyboard.The individual led-driver patches have been compile tested only.
Thanks a lot for driving this. I will applied this patchset into my -devel branch. After I got the Ack from Input guys, I will apply it to my for-next branch then.
Series looks good to me, Johan, thanks for doing this work.
greg k-h
Quoted 5 lines · Greg Kroah-Hartman
Quoted 3 lines
Thanks a lot for driving this. I will applied this patchset into my -devel branch. After I got the Ack from Input guys, I will apply it to my for-next branch then.
Series looks good to me, Johan, thanks for doing this work.
Yeah, looks good to me as well, thanks. The lm8323 change should be Acked by Dmitry though, as it's in his area.
Signature · 2 lines
Jiri Kosina SUSE Labs