| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <046968f7-3245-1df5-6e11-6f29292bbbc5@I-love.SAKURA.ne.jp>
Date: Sun, 28 Aug 2022 15:00:02 +0900
From: Tetsuo Handa <penguin-kernel@...ove.SAKURA.ne.jp>
To: Greg Tulli <greg.iforce@...il.com>,
Randy Dunlap <rdunlap@...radead.org>
Cc: linux-kernel@...r.kernel.org, linux-input@...r.kernel.org,
Dmitry Torokhov <dmitry.torokhov@...il.com>
Subject: Re: [Question] iforce_serio lockup
On 2022/07/30 4:24, Greg Tulli wrote:
> I was advised to use a vanilla kernel with CONFIG_PROVE_LOCKING and
> decode the symbols in the report. So I followed
> https://kernelnewbies.org/OutreachyfirstpatchSetup and here's the
> result. Thanks, Tetsuo Handa.
>
> WARNING: possible circular locking dependency detected
> 5.19.0-rc6iforce-00194-gf3a76018dd55 #2 Not tainted
OK. You confirmed that this is a real deadlock.
> 2022-07-11 11:32 GMT+02:00, Greg T <greg.iforce@...il.com>:
>
>> That problem vanishes if we don't call iforce_process_packet directly
>> from iforce_serio_irq, but from a tasklet. Is that a right approach?
Does untested diff shown below solve the problem?
I don't know about flush ordering in iforce_serio_disconnect().
drivers/input/joystick/iforce/iforce-serio.c | 51 +++++++++++++++++++-
1 file changed, 49 insertions(+), 2 deletions(-)
diff --git a/drivers/input/joystick/iforce/iforce-serio.c b/drivers/input/joystick/iforce/iforce-serio.c
index f95a81b9fac7..5ff650ce05ab 100644
--- a/drivers/input/joystick/iforce/iforce-serio.c
+++ b/drivers/input/joystick/iforce/iforce-serio.c
@@ -124,8 +124,7 @@ static void iforce_serio_write_wakeup(struct serio *serio)
iforce_serio_xmit(iforce);
}
-static irqreturn_t iforce_serio_irq(struct serio *serio,
- unsigned char data, unsigned int flags)
+static void do_iforce_serio(struct serio *serio, unsigned char data)
{
struct iforce_serio *iforce_serio = serio_get_drvdata(serio);
struct iforce *iforce = &iforce_serio->iforce;
@@ -183,6 +182,53 @@ static irqreturn_t iforce_serio_irq(struct serio *serio,
iforce_serio->csum = 0;
}
out:
+ return;
+}
+
+struct iforce_serio_irq {
+ struct list_head list;
+ struct serio *serio;
+ unsigned char data;
+};
+static LIST_HEAD(iforce_serio_irq_list);
+static DEFINE_SPINLOCK(iforce_serio_irq_list_lock);
+
+static void iforce_serio_workfn(struct work_struct *work)
+{
+ static DEFINE_MUTEX(lock);
+ struct iforce_serio_irq *ev;
+
+ mutex_lock(&lock);
+ spin_lock_irq(&iforce_serio_irq_list_lock);
+ while (!list_empty(&iforce_serio_irq_list)) {
+ ev = list_first_entry(&iforce_serio_irq_list, struct iforce_serio_irq, list);
+ list_del(&ev->list);
+ spin_unlock_irq(&iforce_serio_irq_list_lock);
+ do_iforce_serio(ev->serio, ev->data);
+ kfree(ev);
+ spin_lock_irq(&iforce_serio_irq_list_lock);
+ }
+ spin_unlock_irq(&iforce_serio_irq_list_lock);
+ mutex_unlock(&lock);
+}
+static DECLARE_WORK(iforce_serio_work, iforce_serio_workfn);
+
+static irqreturn_t iforce_serio_irq(struct serio *serio,
+ unsigned char data, unsigned int flags)
+{
+ bool was_empty;
+ struct iforce_serio_irq *ev = kmalloc(sizeof(*ev), GFP_ATOMIC | __GFP_NOWARN);
+
+ if (ev) {
+ ev->serio = serio;
+ ev->data = data;
+ spin_lock(&iforce_serio_irq_list_lock);
+ was_empty = list_empty(&iforce_serio_irq_list);
+ list_add_tail(&ev->list, &iforce_serio_irq_list);
+ spin_unlock(&iforce_serio_irq_list_lock);
+ if (was_empty)
+ queue_work(system_highpri_wq, &iforce_serio_work);
+ }
return IRQ_HANDLED;
}
@@ -220,6 +266,7 @@ static void iforce_serio_disconnect(struct serio *serio)
{
struct iforce_serio *iforce_serio = serio_get_drvdata(serio);
+ flush_work(&iforce_serio_work);
input_unregister_device(iforce_serio->iforce.dev);
serio_close(serio);
serio_set_drvdata(serio, NULL);
--
2.18.4
Powered by blists - more mailing lists