All of lore.kernel.org
 help / color / mirror / Atom feed
From: Danny Kaehn <danny.kaehn@plexus.com>
To: <jikos@kernel.org>, <benjamin.tissoires@redhat.com>,
	<andriy.shevchenko@linux.intel.com>
Cc: <linux-input@vger.kernel.org>, <ethan.twardy@plexus.com>,
	Danny Kaehn <danny.kaehn@plexus.com>
Subject: [PATCH] hid: cp2112: Fix duplicate workqueue initialization
Date: Tue, 19 Sep 2023 16:22:45 -0500	[thread overview]
Message-ID: <20230919212245.483646-1-danny.kaehn@plexus.com> (raw)

Previously the cp2112 driver called INIT_DELAYED_WORK within
cp2112_gpio_irq_startup, resulting in duplicate initilizations of the
workqueue on subsequent IRQ startups following an initial request. This
resulted in a warning in set_work_data in workqueue.c, as well as a rare
NULL dereference within process_one_work in workqueue.c.

Initialize the workqueue within _probe instead.

Signed-off-by: Danny Kaehn <danny.kaehn@plexus.com>
---

Note -- the warning & NULL dereference that were caused by this were
completely decoupled from the driver code, making this a fairly tricky
bug to track down. I wonder if there would be a way to add a WARN into
__INIT_WORK if an already initialized workqueue is re-initialized
without a lot of overhead...

 drivers/hid/hid-cp2112.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/hid/hid-cp2112.c b/drivers/hid/hid-cp2112.c
index 54c33a24f844..36f76c6dfa20 100644
--- a/drivers/hid/hid-cp2112.c
+++ b/drivers/hid/hid-cp2112.c
@@ -1151,8 +1151,6 @@ static unsigned int cp2112_gpio_irq_startup(struct irq_data *d)
 	struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
 	struct cp2112_device *dev = gpiochip_get_data(gc);
 
-	INIT_DELAYED_WORK(&dev->gpio_poll_worker, cp2112_gpio_poll_callback);
-
 	if (!dev->gpio_poll) {
 		dev->gpio_poll = true;
 		schedule_delayed_work(&dev->gpio_poll_worker, 0);
@@ -1307,6 +1305,8 @@ static int cp2112_probe(struct hid_device *hdev, const struct hid_device_id *id)
 	girq->handler = handle_simple_irq;
 	girq->threaded = true;
 
+	INIT_DELAYED_WORK(&dev->gpio_poll_worker, cp2112_gpio_poll_callback);
+
 	ret = gpiochip_add_data(&dev->gc, dev);
 	if (ret < 0) {
 		hid_err(hdev, "error registering gpio chip\n");
-- 
2.25.1


             reply	other threads:[~2023-09-19 21:49 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-09-19 21:22 Danny Kaehn [this message]
2023-09-20 13:04 ` [PATCH] hid: cp2112: Fix duplicate workqueue initialization Andy Shevchenko
2023-09-20 19:10   ` Danny Kaehn
2023-09-21  9:42     ` andriy.shevchenko
2023-10-04 19:19       ` Jiri Kosina

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20230919212245.483646-1-danny.kaehn@plexus.com \
    --to=danny.kaehn@plexus.com \
    --cc=andriy.shevchenko@linux.intel.com \
    --cc=benjamin.tissoires@redhat.com \
    --cc=ethan.twardy@plexus.com \
    --cc=jikos@kernel.org \
    --cc=linux-input@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.