[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20200103084339.GA855576@kroah.com>
Date: Fri, 3 Jan 2020 09:43:39 +0100
From: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
To: Zhenzhong Duan <zhenzhong.duan@...il.com>
Cc: linux-kernel@...r.kernel.org, x86@...nel.org,
Arnd Bergmann <arnd@...db.de>
Subject: Re: [PATCH RESEND] ttyprintk: fix a potential sleeping in interrupt
context issue
On Fri, Jan 03, 2020 at 11:45:41AM +0800, Zhenzhong Duan wrote:
> Google syzbot reports:
> BUG: sleeping function called from invalid context at
> kernel/locking/mutex.c:938
> in_atomic(): 1, irqs_disabled(): 0, non_block: 0, pid: 0, name: swapper/1
> 1 lock held by swapper/1/0:
> ...
> Call Trace:
> <IRQ>
> dump_stack+0x197/0x210
> ___might_sleep.cold+0x1fb/0x23e
> __might_sleep+0x95/0x190
> __mutex_lock+0xc5/0x13c0
> mutex_lock_nested+0x16/0x20
> tpk_write+0x5d/0x340
> resync_tnc+0x1b6/0x320
> call_timer_fn+0x1ac/0x780
> run_timer_softirq+0x6c3/0x1790
> __do_softirq+0x262/0x98c
> irq_exit+0x19b/0x1e0
> smp_apic_timer_interrupt+0x1a3/0x610
> apic_timer_interrupt+0xf/0x20
> </IRQ>
>
> Fix it by using spinlock in process context instead of mutex and having
> interrupt disabled in critical section.
>
> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@...il.com>
> Cc: Arnd Bergmann <arnd@...db.de>
> Cc: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
> ---
> drivers/char/ttyprintk.c | 15 +++++++++------
> 1 file changed, 9 insertions(+), 6 deletions(-)
Why was this resent? What differs from the first version that required
it to be resent?
Always give us a clue here please :)
>
> diff --git a/drivers/char/ttyprintk.c b/drivers/char/ttyprintk.c
> index 4f24e46ebe7c..56db949a7b70 100644
> --- a/drivers/char/ttyprintk.c
> +++ b/drivers/char/ttyprintk.c
> @@ -15,10 +15,11 @@
> #include <linux/serial.h>
> #include <linux/tty.h>
> #include <linux/module.h>
> +#include <linux/spinlock.h>
>
> struct ttyprintk_port {
> struct tty_port port;
> - struct mutex port_write_mutex;
> + spinlock_t spinlock;
> };
>
> static struct ttyprintk_port tpk_port;
> @@ -99,11 +100,12 @@ static int tpk_open(struct tty_struct *tty, struct file *filp)
> static void tpk_close(struct tty_struct *tty, struct file *filp)
> {
> struct ttyprintk_port *tpkp = tty->driver_data;
> + unsigned long flags;
>
> - mutex_lock(&tpkp->port_write_mutex);
> + spin_lock_irqsave(&tpkp->spinlock, flags);
> /* flush tpk_printk buffer */
> tpk_printk(NULL, 0);
Are you sure you can call this with a spinlock held?
Doesn't your trace above show the opposite?
What is wrong with sleeping during the mutex you currently have? How is
syzbot reporting this error, is there a reproducer somewhere?
thanks,
greg k-h
Powered by blists - more mailing lists