[PATCH v2 3/5] extcon: Use unique number for the extcon device ID

From: Andy Shevchenko
Date: Wed Apr 05 2023 - 11:28:41 EST


The use of atomic variable is still racy when we do not control which
device has been unregistered and there is a (theoretical) possibility
of the overflow that may cause a duplicate extcon device ID number
to be allocated next time a device is registered.

Replace above mentioned approach by using IDA framework.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>
Acked-by: Bumwoo Lee <bw365.lee@xxxxxxxxxxx>
---
drivers/extcon/extcon.c | 15 ++++++++++++---
drivers/extcon/extcon.h | 2 ++
2 files changed, 14 insertions(+), 3 deletions(-)

diff --git a/drivers/extcon/extcon.c b/drivers/extcon/extcon.c
index 75a0147703c0..daaded92cf80 100644
--- a/drivers/extcon/extcon.c
+++ b/drivers/extcon/extcon.c
@@ -16,6 +16,7 @@

#include <linux/module.h>
#include <linux/types.h>
+#include <linux/idr.h>
#include <linux/init.h>
#include <linux/device.h>
#include <linux/fs.h>
@@ -238,6 +239,7 @@ struct extcon_cable {

static struct class *extcon_class;

+static DEFINE_IDA(extcon_dev_ids);
static LIST_HEAD(extcon_dev_list);
static DEFINE_MUTEX(extcon_dev_list_lock);

@@ -1248,7 +1250,6 @@ static int extcon_alloc_groups(struct extcon_dev *edev)
int extcon_dev_register(struct extcon_dev *edev)
{
int ret, index = 0;
- static atomic_t edev_no = ATOMIC_INIT(-1);

ret = create_extcon_class();
if (ret < 0)
@@ -1269,8 +1270,13 @@ int extcon_dev_register(struct extcon_dev *edev)
edev->dev.class = extcon_class;
edev->dev.release = extcon_dev_release;

- dev_set_name(&edev->dev, "extcon%lu",
- (unsigned long)atomic_inc_return(&edev_no));
+ ret = ida_alloc(&extcon_dev_ids, GFP_KERNEL);
+ if (ret < 0)
+ return ret;
+
+ edev->id = ret;
+
+ dev_set_name(&edev->dev, "extcon%d", edev->id);

ret = extcon_alloc_cables(edev);
if (ret < 0)
@@ -1333,6 +1339,7 @@ int extcon_dev_register(struct extcon_dev *edev)
if (edev->max_supported)
kfree(edev->cables);
err_alloc_cables:
+ ida_free(&extcon_dev_ids, edev->id);

return ret;
}
@@ -1361,6 +1368,8 @@ void extcon_dev_unregister(struct extcon_dev *edev)
return;
}

+ ida_free(&extcon_dev_ids, edev->id);
+
device_unregister(&edev->dev);

if (edev->mutually_exclusive && edev->max_supported) {
diff --git a/drivers/extcon/extcon.h b/drivers/extcon/extcon.h
index 9ce7042606d7..5744c325e226 100644
--- a/drivers/extcon/extcon.h
+++ b/drivers/extcon/extcon.h
@@ -18,6 +18,7 @@
* {0x3, 0x6, 0x5, 0}. If it is {0xFFFFFFFF, 0}, there
* can be no simultaneous connections.
* @dev: Device of this extcon.
+ * @id: Unique device ID of this extcon.
* @state: Attach/detach state of this extcon. Do not provide at
* register-time.
* @nh_all: Notifier for the state change events for all supported
@@ -43,6 +44,7 @@ struct extcon_dev {

/* Internal data. Please do not set. */
struct device dev;
+ unsigned int id;
struct raw_notifier_head nh_all;
struct raw_notifier_head *nh;
struct list_head entry;
--
2.40.0.1.gaa8946217a0b