linux-kernel - How to handle concurrent access to /dev/ttyprintk ?

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <cd213843-45fe-2eac-4943-0906ab8d272b@i-love.sakura.ne.jp>
Date:   Mon, 12 Apr 2021 19:39:04 +0900
From:   Tetsuo Handa <penguin-kernel@...ove.sakura.ne.jp>
To:     Samo Pogacnik <samo_pogacnik@....net>
Cc:     Petr Mladek <pmladek@...e.com>, Jiri Slaby <jirislaby@...nel.org>,
        Sergey Senozhatsky <sergey.senozhatsky@...il.com>,
        Steven Rostedt <rostedt@...dmis.org>,
        John Ogness <john.ogness@...utronix.de>,
        linux-kernel@...r.kernel.org,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>
Subject: How to handle concurrent access to /dev/ttyprintk ?

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 ?
>