[PATCH 0/3] auxdisplay: add devm_linedisp_register() and fix work initialization races

[PATCH 0/3] auxdisplay: add devm_linedisp_register() and fix work initialization races

[kr494167]

From: Surendra Singh Chouhan <kr494167@gmail.com>

Per maintainer review feedback from Andy Shevchenko and Chris Packham,
this patch series introduces a devm-managed devm_linedisp_register() helper
for the line-display core to avoid use-after-free teardown races across
auxdisplay drivers.

Patch 1 adds devm_linedisp_register() to line-display core.

Patch 2 fixes delayed work initialization races in seg-led-gpio.c, moves
INIT_DELAYED_WORK() to probe(), uses devm_add_action_or_reset() for work
cancellation, and converts to devm_linedisp_register(), removing seg_led_remove().

Patch 3 fixes delayed work initialization races in max6959.c, moves
INIT_DELAYED_WORK() to probe(), uses devm_add_action_or_reset() for work
cancellation, and converts to devm_linedisp_register(), removing max6959_i2c_remove().

Surendra Singh Chouhan (3):
  auxdisplay: line-display: add devm_linedisp_register()
  auxdisplay: seg-led-gpio: fix work initialization race and convert to
    devm_linedisp_register()
  auxdisplay: max6959: fix work initialization race and convert to
    devm_linedisp_register()

 drivers/auxdisplay/line-display.c | 32 +++++++++++++++++++++++++++++++
 drivers/auxdisplay/line-display.h |  2 ++
 drivers/auxdisplay/max6959.c      | 26 ++++++++++++-------------
 drivers/auxdisplay/seg-led-gpio.c | 27 +++++++++++++-------------
 4 files changed, 61 insertions(+), 26 deletions(-)

-- 
2.55.0

[PATCH 0/3 1/3] auxdisplay: line-display: add devm_linedisp_register()

[kr494167]

From: Surendra Singh Chouhan <kr494167@gmail.com>

Add devm_linedisp_register() to manage character line display registration
via devres. This simplifies driver cleanup and prevents use-after-free
bugs when unregistering line displays on driver detach.

Signed-off-by: Surendra Singh Chouhan <kr494167@gmail.com>
---
 drivers/auxdisplay/line-display.c | 32 +++++++++++++++++++++++++++++++
 drivers/auxdisplay/line-display.h |  2 ++
 2 files changed, 34 insertions(+)

diff --git a/drivers/auxdisplay/line-display.c b/drivers/auxdisplay/line-display.c
index 915eb5cd96b2..6fd0b870e16a 100644
--- a/drivers/auxdisplay/line-display.c
+++ b/drivers/auxdisplay/line-display.c
@@ -592,5 +592,37 @@ void linedisp_unregister(struct linedisp *linedisp)
 }
 EXPORT_SYMBOL_NS_GPL(linedisp_unregister, "LINEDISP");
 
+static void devm_linedisp_unregister(void *data)
+{
+	struct linedisp *linedisp = data;
+
+	linedisp_unregister(linedisp);
+}
+
+/**
+ * devm_linedisp_register - register a character line display
+ * @dev: device being registered
+ * @linedisp: pointer to character line display structure
+ * @num_chars: the number of characters that can be displayed
+ * @ops: character line display operations
+ *
+ * Managed linedisp_register(). Line display registered with this function will
+ * automatically be unregistered on driver detach.
+ *
+ * Return: zero on success, else a negative error code.
+ */
+int devm_linedisp_register(struct device *dev, struct linedisp *linedisp,
+			   unsigned int num_chars, const struct linedisp_ops *ops)
+{
+	int err;
+
+	err = linedisp_register(linedisp, dev, num_chars, ops);
+	if (err)
+		return err;
+
+	return devm_add_action_or_reset(dev, devm_linedisp_unregister, linedisp);
+}
+EXPORT_SYMBOL_NS_GPL(devm_linedisp_register, "LINEDISP");
+
 MODULE_DESCRIPTION("Character line display core support");
 MODULE_LICENSE("GPL");
diff --git a/drivers/auxdisplay/line-display.h b/drivers/auxdisplay/line-display.h
index 36853b639711..cd1b94829f72 100644
--- a/drivers/auxdisplay/line-display.h
+++ b/drivers/auxdisplay/line-display.h
@@ -88,5 +88,7 @@ void linedisp_detach(struct device *dev);
 int linedisp_register(struct linedisp *linedisp, struct device *parent,
 		      unsigned int num_chars, const struct linedisp_ops *ops);
 void linedisp_unregister(struct linedisp *linedisp);
+int devm_linedisp_register(struct device *dev, struct linedisp *linedisp,
+			   unsigned int num_chars, const struct linedisp_ops *ops);
 
 #endif /* LINEDISP_H */
-- 
2.55.0

[PATCH 0/3 2/3] auxdisplay: seg-led-gpio: fix work initialization race and convert to devm_linedisp_register()

[kr494167]

From: Surendra Singh Chouhan <kr494167@gmail.com>

INIT_DELAYED_WORK(&priv->work, seg_led_update) was previously called inside
seg_led_linedisp_get_map_type(), which is invoked during/after
linedisp_register(). Initializing a delayed_work structure inside a map
query callback can re-initialize an active work item or race with
seg_led_linedisp_update().

In addition, seg_led_remove() called cancel_delayed_work_sync(&priv->work)
before linedisp_unregister(&priv->linedisp), allowing sysfs updates to
reschedule work after cancel_delayed_work_sync() completed.

Fix these by moving INIT_DELAYED_WORK() to probe(), using
devm_add_action_or_reset() for work cancellation, and converting to
devm_linedisp_register(). Registering devm_linedisp_register() after
work cancellation action ensures proper LIFO teardown order (sysfs
interface unregistered first, followed by work cancellation), allowing
seg_led_remove() to be removed entirely.

Fixes: 899383f9ecf5 ("auxdisplay: Add 7-segment LED display driver")
Reviewed-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
Tested-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
Signed-off-by: Surendra Singh Chouhan <kr494167@gmail.com>
---
 drivers/auxdisplay/seg-led-gpio.c | 27 ++++++++++++++-------------
 1 file changed, 14 insertions(+), 13 deletions(-)

diff --git a/drivers/auxdisplay/seg-led-gpio.c b/drivers/auxdisplay/seg-led-gpio.c
index bc463118fe51..ca39c9071688 100644
--- a/drivers/auxdisplay/seg-led-gpio.c
+++ b/drivers/auxdisplay/seg-led-gpio.c
@@ -38,11 +38,15 @@ static void seg_led_update(struct work_struct *work)
 	gpiod_multi_set_value_cansleep(priv->segment_gpios, values);
 }
 
-static int seg_led_linedisp_get_map_type(struct linedisp *linedisp)
+static void seg_led_cancel_work(void *data)
 {
-	struct seg_led_priv *priv = container_of(linedisp, struct seg_led_priv, linedisp);
+	struct delayed_work *work = data;
 
-	INIT_DELAYED_WORK(&priv->work, seg_led_update);
+	cancel_delayed_work_sync(work);
+}
+
+static int seg_led_linedisp_get_map_type(struct linedisp *linedisp)
+{
 	return LINEDISP_MAP_SEG7;
 }
 
@@ -62,11 +66,17 @@ static int seg_led_probe(struct platform_device *pdev)
 {
 	struct seg_led_priv *priv;
 	struct device *dev = &pdev->dev;
+	int ret;
 
 	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
 	if (!priv)
 		return -ENOMEM;
 
+	INIT_DELAYED_WORK(&priv->work, seg_led_update);
+	ret = devm_add_action_or_reset(dev, seg_led_cancel_work, &priv->work);
+	if (ret)
+		return ret;
+
 	platform_set_drvdata(pdev, priv);
 
 	priv->segment_gpios = devm_gpiod_get_array(dev, "segment", GPIOD_OUT_LOW);
@@ -76,15 +86,7 @@ static int seg_led_probe(struct platform_device *pdev)
 	if (priv->segment_gpios->ndescs < 7 || priv->segment_gpios->ndescs > 8)
 		return -EINVAL;
 
-	return linedisp_register(&priv->linedisp, dev, 1, &seg_led_linedisp_ops);
-}
-
-static void seg_led_remove(struct platform_device *pdev)
-{
-	struct seg_led_priv *priv = platform_get_drvdata(pdev);
-
-	cancel_delayed_work_sync(&priv->work);
-	linedisp_unregister(&priv->linedisp);
+	return devm_linedisp_register(dev, &priv->linedisp, 1, &seg_led_linedisp_ops);
 }
 
 static const struct of_device_id seg_led_of_match[] = {
@@ -95,7 +97,6 @@ MODULE_DEVICE_TABLE(of, seg_led_of_match);
 
 static struct platform_driver seg_led_driver = {
 	.probe = seg_led_probe,
-	.remove = seg_led_remove,
 	.driver = {
 		.name = "seg-led-gpio",
 		.of_match_table = seg_led_of_match,
-- 
2.55.0

[PATCH 0/3 3/3] auxdisplay: max6959: fix work initialization race and convert to devm_linedisp_register()

[kr494167]

From: Surendra Singh Chouhan <kr494167@gmail.com>

INIT_DELAYED_WORK(&priv->work, max6959_disp_update) was previously called
inside max6959_linedisp_get_map_type(), which is invoked during/after
linedisp_register(). Initializing a delayed_work structure inside a map
query callback can re-initialize an active work item or race with
max6959_linedisp_update().

In addition, max6959_i2c_remove() called cancel_delayed_work_sync() before
linedisp_unregister(&priv->linedisp), allowing sysfs updates to
reschedule work after cancel_delayed_work_sync() completed.

Fix these by moving INIT_DELAYED_WORK() to probe(), using
devm_add_action_or_reset() for work cancellation, and converting to
devm_linedisp_register(). Registering devm_linedisp_register() after
work cancellation action ensures proper LIFO teardown order, allowing
max6959_i2c_remove() to be removed entirely.

Fixes: a9bcd02fa422 ("auxdisplay: Add driver for MAX695x 7-segment LED controllers")
Signed-off-by: Surendra Singh Chouhan <kr494167@gmail.com>
---
 drivers/auxdisplay/max6959.c | 26 +++++++++++++-------------
 1 file changed, 13 insertions(+), 13 deletions(-)

diff --git a/drivers/auxdisplay/max6959.c b/drivers/auxdisplay/max6959.c
index 888788a1ff08..795bdac9af3f 100644
--- a/drivers/auxdisplay/max6959.c
+++ b/drivers/auxdisplay/max6959.c
@@ -63,11 +63,15 @@ static void max6959_disp_update(struct work_struct *work)
 	regmap_bulk_write(priv->regmap, REG_DIGIT(0), buf, ARRAY_SIZE(buf));
 }
 
-static int max6959_linedisp_get_map_type(struct linedisp *linedisp)
+static void max6959_cancel_work(void *data)
 {
-	struct max6959_priv *priv = container_of(linedisp, struct max6959_priv, linedisp);
+	struct delayed_work *work = data;
 
-	INIT_DELAYED_WORK(&priv->work, max6959_disp_update);
+	cancel_delayed_work_sync(work);
+}
+
+static int max6959_linedisp_get_map_type(struct linedisp *linedisp)
+{
 	return LINEDISP_MAP_SEG7;
 }
 
@@ -123,6 +127,11 @@ static int max6959_i2c_probe(struct i2c_client *client)
 	if (!priv)
 		return -ENOMEM;
 
+	INIT_DELAYED_WORK(&priv->work, max6959_disp_update);
+	ret = devm_add_action_or_reset(dev, max6959_cancel_work, &priv->work);
+	if (ret)
+		return ret;
+
 	priv->regmap = devm_regmap_init_i2c(client, &max6959_regmap_config);
 	if (IS_ERR(priv->regmap))
 		return PTR_ERR(priv->regmap);
@@ -131,7 +140,7 @@ static int max6959_i2c_probe(struct i2c_client *client)
 	if (ret)
 		return ret;
 
-	ret = linedisp_register(&priv->linedisp, dev, 4, &max6959_linedisp_ops);
+	ret = devm_linedisp_register(dev, &priv->linedisp, 4, &max6959_linedisp_ops);
 	if (ret)
 		return ret;
 
@@ -140,14 +149,6 @@ static int max6959_i2c_probe(struct i2c_client *client)
 	return 0;
 }
 
-static void max6959_i2c_remove(struct i2c_client *client)
-{
-	struct max6959_priv *priv = i2c_get_clientdata(client);
-
-	cancel_delayed_work_sync(&priv->work);
-	linedisp_unregister(&priv->linedisp);
-}
-
 static int max6959_suspend(struct device *dev)
 {
 	return max6959_enable(dev_get_drvdata(dev), false);
@@ -179,7 +180,6 @@ static struct i2c_driver max6959_i2c_driver = {
 		.of_match_table = max6959_of_table,
 	},
 	.probe = max6959_i2c_probe,
-	.remove = max6959_i2c_remove,
 	.id_table = max6959_i2c_id,
 };
 module_i2c_driver(max6959_i2c_driver);
-- 
2.55.0