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(-)
--
1.8.5.5
Use the attribute groups of the led-class to create the mode attribute
during probe in order to avoid racing with userspace.
Signed-off-by: Johan Hovold <[email protected]>
---
drivers/leds/leds-lm3530.c | 20 +++++++-------------
1 file changed, 7 insertions(+), 13 deletions(-)
diff --git a/drivers/leds/leds-lm3530.c b/drivers/leds/leds-lm3530.c
index 652368c2ea9a..91325de3cd33 100644
--- a/drivers/leds/leds-lm3530.c
+++ b/drivers/leds/leds-lm3530.c
@@ -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;
--
1.8.5.5
Use the attribute groups of the led-class to create the mode attribute
during probe in order to avoid racing with userspace.
Signed-off-by: Johan Hovold <[email protected]>
---
drivers/leds/leds-max8997.c | 16 +++++++---------
1 file changed, 7 insertions(+), 9 deletions(-)
diff --git a/drivers/leds/leds-max8997.c b/drivers/leds/leds-max8997.c
index f449a8bdddc7..607bc2755aba 100644
--- a/drivers/leds/leds-max8997.c
+++ b/drivers/leds/leds-max8997.c
@@ -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;
--
1.8.5.5
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.
Signed-off-by: Johan Hovold <[email protected]>
---
drivers/leds/led-class.c | 5 +++--
include/linux/leds.h | 2 ++
2 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/leds/led-class.c b/drivers/leds/led-class.c
index f37d63cf726b..aa29198fca3e 100644
--- a/drivers/leds/led-class.c
+++ b/drivers/leds/led-class.c
@@ -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);
diff --git a/include/linux/leds.h b/include/linux/leds.h
index 0287ab296689..e43686472197 100644
--- a/include/linux/leds.h
+++ b/include/linux/leds.h
@@ -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 */
--
1.8.5.5
Use the attribute groups of the led-class to create the blink attribute
during probe in order to avoid racing with userspace.
Signed-off-by: Johan Hovold <[email protected]>
---
drivers/leds/leds-ss4200.c | 14 +++++++++-----
1 file changed, 9 insertions(+), 5 deletions(-)
diff --git a/drivers/leds/leds-ss4200.c b/drivers/leds/leds-ss4200.c
index 2eb3ef62962b..046cb7008745 100644
--- a/drivers/leds/leds-ss4200.c
+++ b/drivers/leds/leds-ss4200.c
@@ -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
--
1.8.5.5
Use the attribute groups of the led-class to create the src attribute
during probe in order to avoid racing with userspace.
Signed-off-by: Johan Hovold <[email protected]>
---
drivers/leds/leds-wm831x-status.c | 23 +++++++++--------------
1 file changed, 9 insertions(+), 14 deletions(-)
diff --git a/drivers/leds/leds-wm831x-status.c b/drivers/leds/leds-wm831x-status.c
index e72c974142d0..1b71e0701002 100644
--- a/drivers/leds/leds-wm831x-status.c
+++ b/drivers/leds/leds-wm831x-status.c
@@ -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;
--
1.8.5.5
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]>
---
drivers/input/keyboard/lm8323.c | 22 +++++++++-------------
1 file changed, 9 insertions(+), 13 deletions(-)
diff --git a/drivers/input/keyboard/lm8323.c b/drivers/input/keyboard/lm8323.c
index 0b42118cbf8f..cb32e2b506b7 100644
--- a/drivers/input/keyboard/lm8323.c
+++ b/drivers/input/keyboard/lm8323.c
@@ -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
Use the attribute groups of the led-class to create the LED attributes
during probe in order to avoid racing with userspace.
Signed-off-by: Johan Hovold <[email protected]>
---
drivers/leds/leds-lp55xx-common.c | 22 +++-------------------
1 file changed, 3 insertions(+), 19 deletions(-)
diff --git a/drivers/leds/leds-lp55xx-common.c b/drivers/leds/leds-lp55xx-common.c
index 3fbfb31602c7..77c26bc32eed 100644
--- a/drivers/leds/leds-lp55xx-common.c
+++ b/drivers/leds/leds-lp55xx-common.c
@@ -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);
}
--
1.8.5.5
Make sure the sysfs group is removed when the LEDs are unregistered.
Signed-off-by: Johan Hovold <[email protected]>
---
drivers/leds/leds-lp55xx-common.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/leds/leds-lp55xx-common.c b/drivers/leds/leds-lp55xx-common.c
index 88317b4f7bf3..3fbfb31602c7 100644
--- a/drivers/leds/leds-lp55xx-common.c
+++ b/drivers/leds/leds-lp55xx-common.c
@@ -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);
}
--
1.8.5.5
Use the attribute groups of the led-class to create the attributes
during probe in order to avoid racing with userspace.
Signed-off-by: Johan Hovold <[email protected]>
---
drivers/leds/leds-lm3533.c | 20 ++++++++------------
1 file changed, 8 insertions(+), 12 deletions(-)
diff --git a/drivers/leds/leds-lm3533.c b/drivers/leds/leds-lm3533.c
index e2c642c1169b..cbf61a40137d 100644
--- a/drivers/leds/leds-lm3533.c
+++ b/drivers/leds/leds-lm3533.c
@@ -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);
--
1.8.5.5
Use the attribute groups of the led-class to create the flash and
torch-LED attributes during probe in order to avoid racing with
userspace.
Signed-off-by: Johan Hovold <[email protected]>
---
drivers/leds/leds-lm3642.c | 30 ++++++++++++++----------------
1 file changed, 14 insertions(+), 16 deletions(-)
diff --git a/drivers/leds/leds-lm3642.c b/drivers/leds/leds-lm3642.c
index ceb6b3cde6fe..d3dec0132769 100644
--- a/drivers/leds/leds-lm3642.c
+++ b/drivers/leds/leds-lm3642.c
@@ -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);
--
1.8.5.5
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(-)
diff --git a/drivers/leds/leds-netxbig.c b/drivers/leds/leds-netxbig.c
index e97f443a6e07..8ab931b8fcd3 100644
--- a/drivers/leds/leds-netxbig.c
+++ b/drivers/leds/leds-netxbig.c
@@ -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;
}
--
1.8.5.5
Use 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).
Signed-off-by: Johan Hovold <[email protected]>
---
drivers/leds/leds-lm355x.c | 21 +++++++++------------
1 file changed, 9 insertions(+), 12 deletions(-)
diff --git a/drivers/leds/leds-lm355x.c b/drivers/leds/leds-lm355x.c
index 591eb5e58ae3..f5112cb2d991 100644
--- a/drivers/leds/leds-lm355x.c
+++ b/drivers/leds/leds-lm355x.c
@@ -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);
--
1.8.5.5
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-ns2.c | 16 +++++++---------
1 file changed, 7 insertions(+), 9 deletions(-)
diff --git a/drivers/leds/leds-ns2.c b/drivers/leds/leds-ns2.c
index efa625883c83..231993d1fe21 100644
--- a/drivers/leds/leds-ns2.c
+++ b/drivers/leds/leds-ns2.c
@@ -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);
}
--
1.8.5.5
On Wed, Jun 25, 2014 at 10:08 AM, Johan Hovold <[email protected]> wrote:
> 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(-)
>
> diff --git a/drivers/leds/leds-netxbig.c b/drivers/leds/leds-netxbig.c
> index e97f443a6e07..8ab931b8fcd3 100644
> --- a/drivers/leds/leds-netxbig.c
> +++ b/drivers/leds/leds-netxbig.c
> @@ -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);
Let's just "return led_classdev_register(&pdev->dev, &led_dat->cdev);"
and remove "ret"
I will handle this trivial change.
-Bryan
> - 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
>
On Wed, Jun 25, 2014 at 10:08 AM, Johan Hovold <[email protected]> wrote:
> 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
> Signed-off-by: Johan Hovold <[email protected]>
> ---
> drivers/input/keyboard/lm8323.c | 22 +++++++++-------------
> 1 file changed, 9 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/input/keyboard/lm8323.c b/drivers/input/keyboard/lm8323.c
> index 0b42118cbf8f..cb32e2b506b7 100644
> --- a/drivers/input/keyboard/lm8323.c
> +++ b/drivers/input/keyboard/lm8323.c
> @@ -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
>
On Wed, Jun 25, 2014 at 10:08 AM, Johan Hovold <[email protected]> wrote:
> 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.
>
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
> 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(-)
>
> --
> 1.8.5.5
>
On Wed, Jun 25, 2014 at 03:46:19PM -0700, Bryan Wu wrote:
> On Wed, Jun 25, 2014 at 10:08 AM, Johan Hovold <[email protected]> wrote:
> > 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.
> >
>
> 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
On Thu, 26 Jun 2014, Greg Kroah-Hartman wrote:
> > 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.
--
Jiri Kosina
SUSE Labs
On Wed, Jun 25, 2014 at 10:08 AM, Johan Hovold <[email protected]> wrote:
> 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]>
> ---
> drivers/input/keyboard/lm8323.c | 22 +++++++++-------------
> 1 file changed, 9 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/input/keyboard/lm8323.c b/drivers/input/keyboard/lm8323.c
> index 0b42118cbf8f..cb32e2b506b7 100644
> --- a/drivers/input/keyboard/lm8323.c
> +++ b/drivers/input/keyboard/lm8323.c
> @@ -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
>
Thanks.
--
Dmitry
On Fri, Jun 27, 2014 at 11:50 AM, Dmitry Torokhov
<[email protected]> wrote:
> On Wed, Jun 25, 2014 at 10:08 AM, Johan Hovold <[email protected]> wrote:
>> 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
>> ---
>> drivers/input/keyboard/lm8323.c | 22 +++++++++-------------
>> 1 file changed, 9 insertions(+), 13 deletions(-)
>>
>> diff --git a/drivers/input/keyboard/lm8323.c b/drivers/input/keyboard/lm8323.c
>> index 0b42118cbf8f..cb32e2b506b7 100644
>> --- a/drivers/input/keyboard/lm8323.c
>> +++ b/drivers/input/keyboard/lm8323.c
>> @@ -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
>>
>
> Thanks.
>
> --
> Dmitry