All of lore.kernel.org
 help / color / mirror / Atom feed
From: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>
To: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>,
	Matti Vaittinen <mazziesaccount@gmail.com>
Cc: Chanwoo Choi <cw00.choi@samsung.com>,
	Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>,
	Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>,
	MyungJoo Ham <myungjoo.ham@samsung.com>,
	Hans de Goede <hdegoede@redhat.com>,
	Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>,
	Marek Szyprowski <m.szyprowski@samsung.com>,
	linux-kernel@vger.kernel.org
Subject: [PATCH RESEND v2 3/5] extcon: extcon-max77693.c: Fix potential work-queue cancellation race
Date: Tue, 8 Jun 2021 13:10:11 +0300	[thread overview]
Message-ID: <cbe8205eed8276f6e6db5003cfe51b8b0d4ac966.1623146580.git.matti.vaittinen@fi.rohmeurope.com> (raw)
In-Reply-To: <cover.1623146580.git.matti.vaittinen@fi.rohmeurope.com>

[-- Attachment #1: Type: text/plain, Size: 2792 bytes --]

The extcon IRQ schedules a work item. IRQ is requested using devm while
WQ is cancelld at remove(). This mixing of devm and manual unwinding has
potential case where the WQ has been emptied (.remove() was ran) but
devm unwinding of IRQ was not yet done. It may be possible the IRQ is
triggered at this point scheduling new work item to the already flushed
queue.

According to the input documentation the input device allocated by
devm_input_allocate_device() does not need to be explicitly unregistered.
Use the new devm_work_autocancel() and remove the remove() to simplify the
code.

Signed-off-by: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>
Reviewed-by: Hans de Goede <hdegoede@redhat.com>
---

Please note that the change is compile-tested only. All proper testing is
highly appreciated.
---
 drivers/extcon/extcon-max77693.c | 17 +++++------------
 1 file changed, 5 insertions(+), 12 deletions(-)

diff --git a/drivers/extcon/extcon-max77693.c b/drivers/extcon/extcon-max77693.c
index 92af97e00828..1f1d9ab0c5c7 100644
--- a/drivers/extcon/extcon-max77693.c
+++ b/drivers/extcon/extcon-max77693.c
@@ -5,6 +5,7 @@
 // Copyright (C) 2012 Samsung Electrnoics
 // Chanwoo Choi <cw00.choi@samsung.com>
 
+#include <linux/devm-helpers.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/i2c.h>
@@ -1127,7 +1128,10 @@ static int max77693_muic_probe(struct platform_device *pdev)
 	platform_set_drvdata(pdev, info);
 	mutex_init(&info->mutex);
 
-	INIT_WORK(&info->irq_work, max77693_muic_irq_work);
+	ret = devm_work_autocancel(&pdev->dev, &info->irq_work,
+				   max77693_muic_irq_work);
+	if (ret)
+		return ret;
 
 	/* Support irq domain for MAX77693 MUIC device */
 	for (i = 0; i < ARRAY_SIZE(muic_irqs); i++) {
@@ -1254,22 +1258,11 @@ static int max77693_muic_probe(struct platform_device *pdev)
 	return ret;
 }
 
-static int max77693_muic_remove(struct platform_device *pdev)
-{
-	struct max77693_muic_info *info = platform_get_drvdata(pdev);
-
-	cancel_work_sync(&info->irq_work);
-	input_unregister_device(info->dock);
-
-	return 0;
-}
-
 static struct platform_driver max77693_muic_driver = {
 	.driver		= {
 		.name	= DEV_NAME,
 	},
 	.probe		= max77693_muic_probe,
-	.remove		= max77693_muic_remove,
 };
 
 module_platform_driver(max77693_muic_driver);
-- 
2.25.4


-- 
Matti Vaittinen, Linux device drivers
ROHM Semiconductors, Finland SWDC
Kiviharjunlenkki 1E
90220 OULU
FINLAND

~~~ "I don't think so," said Rene Descartes. Just then he vanished ~~~
Simon says - in Latin please.
~~~ "non cogito me" dixit Rene Descarte, deinde evanescavit ~~~
Thanks to Simon Glass for the translation =] 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

  parent reply	other threads:[~2021-06-08 10:10 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-06-08 10:09 [PATCH RESEND v2 0/5] Add devm helper for work-queue initialization Matti Vaittinen
2021-06-08 10:09 ` [PATCH RESEND v2 1/5] devm-helpers: Add resource managed version of work init Matti Vaittinen
2021-06-08 10:09 ` [PATCH RESEND v2 2/5] extcon: extcon-max14577: Fix potential work-queue cancellation race Matti Vaittinen
2021-06-10  9:41   ` Chanwoo Choi
2021-06-08 10:10 ` Matti Vaittinen [this message]
2021-06-10  9:43   ` [PATCH RESEND v2 3/5] extcon: extcon-max77693.c: " Chanwoo Choi
2021-06-10  9:49     ` Hans de Goede
2021-06-11  9:04       ` Chanwoo Choi
2021-06-10  9:57     ` Matti Vaittinen
2021-06-10 22:14       ` Dmitry Torokhov
2021-06-11  7:16       ` Chanwoo Choi
2021-06-08 10:10 ` [PATCH RESEND v2 4/5] extcon: extcon-max8997: Fix IRQ freeing at error path Matti Vaittinen
2021-06-10  9:53   ` Chanwoo Choi
2021-06-08 10:10 ` [PATCH RESEND v2 5/5] extcon: extcon-max8997: Simplify driver using devm Matti Vaittinen
2021-06-10  9:57   ` Chanwoo Choi
2021-06-09 15:23 ` [PATCH RESEND v2 0/5] Add devm helper for work-queue initialization Hans de Goede
2021-06-10  1:02   ` Chanwoo Choi
2021-06-10  8:22     ` Vaittinen, Matti

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=cbe8205eed8276f6e6db5003cfe51b8b0d4ac966.1623146580.git.matti.vaittinen@fi.rohmeurope.com \
    --to=matti.vaittinen@fi.rohmeurope.com \
    --cc=b.zolnierkie@samsung.com \
    --cc=cw00.choi@samsung.com \
    --cc=hdegoede@redhat.com \
    --cc=krzysztof.kozlowski@canonical.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=m.szyprowski@samsung.com \
    --cc=mazziesaccount@gmail.com \
    --cc=myungjoo.ham@samsung.com \
    /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.