Messages in this thread | | | Subject | Re: [PATCH] tty: use printk_safe context at tty_msg() | From | Jiri Slaby <> | Date | Tue, 6 Apr 2021 06:51:37 +0200 |
| |
On 03. 04. 21, 6:14, Tetsuo Handa wrote: > syzbot is reporting circular locking dependency due to calling printk() > with port lock held [1]. When this problem was reported, we worried > whether printk_safe context will remain available in future kernels [2], > and then this problem was forgotten. But in order to utilize syzbot's > resource for finding other bugs/reproducers by closing this one of top > crashers, let's apply a patch which counts on availability of printk_safe > context. > > syzbot is also reporting same dependency due to memory allocation fault > injection at tty_buffer_alloc(). Although __GFP_NOWARN cannot prevent > memory allocation fault injection from calling printk(), let's use > __GFP_NOWARN at tty_buffer_alloc() in addition to using printk_safe > context, for generating many lines of messages due to warn_alloc() is > annoying. If we want to report it, we can use pr_warn() instead. > > [1] https://syzkaller.appspot.com/bug?id=39ea6caa479af471183997376dc7e90bc7d64a6a > [2] https://lkml.kernel.org/r/20190218054649.GA26686@jagdpanzerIV > > Reported-by: syzbot <syzbot+43e93968b964e369db0b@syzkaller.appspotmail.com> > Reported-by: syzbot <syzbot+3ed715090790806d8b18@syzkaller.appspotmail.com> > Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> > Fixes: b6da31b2c07c46f2 ("tty: Fix data race in tty_insert_flip_string_fixed_flag") > Cc: <stable@vger.kernel.org> # 4.18+ > --- > drivers/tty/tty_buffer.c | 5 ++++- > include/linux/tty.h | 9 ++++++++- > 2 files changed, 12 insertions(+), 2 deletions(-) > > diff --git a/drivers/tty/tty_buffer.c b/drivers/tty/tty_buffer.c > index 6d4995a5f318..d59f7873bc49 100644 > --- a/drivers/tty/tty_buffer.c > +++ b/drivers/tty/tty_buffer.c > @@ -156,6 +156,7 @@ static struct tty_buffer *tty_buffer_alloc(struct tty_port *port, size_t size) > { > struct llist_node *free; > struct tty_buffer *p; > + unsigned long flags; > > /* Round the buffer size out */ > size = __ALIGN_MASK(size, TTYB_ALIGN_MASK); > @@ -172,7 +173,9 @@ static struct tty_buffer *tty_buffer_alloc(struct tty_port *port, size_t size) > have queued and recycle that ? */ > if (atomic_read(&port->buf.mem_used) > port->buf.mem_limit) > return NULL; > - p = kmalloc(sizeof(struct tty_buffer) + 2 * size, GFP_ATOMIC); > + printk_safe_enter_irqsave(flags); > + p = kmalloc(sizeof(struct tty_buffer) + 2 * size, GFP_ATOMIC | __GFP_NOWARN); > + printk_safe_exit_irqrestore(flags); > if (p == NULL) > return NULL; > > diff --git a/include/linux/tty.h b/include/linux/tty.h > index 95fc2f100f12..7ae8eb46fec3 100644 > --- a/include/linux/tty.h > +++ b/include/linux/tty.h > @@ -14,6 +14,7 @@ > #include <uapi/linux/tty.h> > #include <linux/rwsem.h> > #include <linux/llist.h> > +#include <../../kernel/printk/internal.h>
Including printk's internal header in linux/tty.h doesn't look correct to me.
> @@ -773,7 +774,13 @@ static inline void proc_tty_unregister_driver(struct tty_driver *d) {} > #endif > > #define tty_msg(fn, tty, f, ...) \ > - fn("%s %s: " f, tty_driver_name(tty), tty_name(tty), ##__VA_ARGS__) > + do { \ > + unsigned long flags; \ > + \ > + printk_safe_enter_irqsave(flags); \ > + fn("%s %s: " f, tty_driver_name(tty), tty_name(tty), ##__VA_ARGS__); \ > + printk_safe_exit_irqrestore(flags); \ > + } while (0) > > #define tty_debug(tty, f, ...) tty_msg(pr_debug, tty, f, ##__VA_ARGS__) > #define tty_info(tty, f, ...) tty_msg(pr_info, tty, f, ##__VA_ARGS__) >
-- js
| |