What is the intended usage of /dev/ttyprintk ? It seems that drivers/char/ttyprintk.c was not designed to be opened by multiple processes. As a result, syzbot can trigger tty_warn() flooding enough to fire khungtaskd warning due to tty_port_close().
Do we need to allow concurrent access to /dev/ttyprintk ? If we can't change /dev/ttyprintk exclusively open()able by only one thread, how to handle concurrent access to /dev/ttyprintk ? On 2021/04/07 23:24, Tetsuo Handa wrote: > On 2021/04/07 22:48, Greg Kroah-Hartman wrote: >>> By the way, as soon as applying this patch, I guess that syzkaller starts >>> generating hung task reports because /dev/ttyprintk can trivially trigger >>> flood of >>> >>> tty_warn(tty, "%s: tty->count = 1 port count = %d\n", __func__, >>> port->count); >>> >>> message, and adding >>> >>> if (strcmp(tty_driver_name(tty), "ttyprintk")) >> >> Odd, how can ttyprintk() generate that mess? > > So far three tests and results: > > https://groups.google.com/g/syzkaller-bugs/c/yRLYijD2tbw/m/WifLgadvAAAJ > https://groups.google.com/g/syzkaller-bugs/c/yRLYijD2tbw/m/w2_MiMmAAAAJ > https://groups.google.com/g/syzkaller-bugs/c/yRLYijD2tbw/m/hfsQqSOPAAAJ > > Patch https://syzkaller.appspot.com/x/patch.diff?x=145e4c9ad00000 generated > console output https://syzkaller.appspot.com/x/log.txt?x=162f9fced00000 . > > Patch https://syzkaller.appspot.com/x/patch.diff?x=14839931d00000 did not > flood the console output enough to fire khungtaskd. > > Maybe it is because /dev/ttyprintk can be opened/closed by multiple processes > without serialization? > > Running > > for i in $(seq 1 100); do sleep 1 > /dev/ttyprintk & done > > results in > > tty_port_close_start: tty->count = 1 port count = 100 > > . If tty_port_open() from tpk_open() can do > > spin_lock_irq(&port->lock); > ++port->count; > spin_unlock_irq(&port->lock); > > when tty_port_close_start() from tty_port_close() from tpk_close() is doing > > spin_lock_irqsave(&port->lock, flags); > if (tty->count == 1 && port->count != 1) { > tty_warn(tty, "%s: tty->count = 1 port count = %d\n", __func__, > port->count); > port->count = 1; > } > if (--port->count < 0) { > tty_warn(tty, "%s: bad port count (%d)\n", __func__, > port->count); > port->count = 0; > } > > if (port->count) { > spin_unlock_irqrestore(&port->lock, flags); > return 0; > } > spin_unlock_irqrestore(&port->lock, flags); > > , what prevents port->count from getting larger than 1 ? >
