From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-10.3 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI,NICE_REPLY_A, SPF_HELO_NONE,SPF_PASS,USER_AGENT_SANE_1 autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id E5556C433B4 for ; Wed, 14 Apr 2021 11:11:56 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id BC06E610FC for ; Wed, 14 Apr 2021 11:11:56 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S239428AbhDNLMQ (ORCPT ); Wed, 14 Apr 2021 07:12:16 -0400 Received: from www262.sakura.ne.jp ([202.181.97.72]:51287 "EHLO www262.sakura.ne.jp" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230281AbhDNLMM (ORCPT ); Wed, 14 Apr 2021 07:12:12 -0400 Received: from fsav304.sakura.ne.jp (fsav304.sakura.ne.jp [153.120.85.135]) by www262.sakura.ne.jp (8.15.2/8.15.2) with ESMTP id 13EBBnhj073324; Wed, 14 Apr 2021 20:11:50 +0900 (JST) (envelope-from penguin-kernel@i-love.sakura.ne.jp) Received: from www262.sakura.ne.jp (202.181.97.72) by fsav304.sakura.ne.jp (F-Secure/fsigk_smtp/550/fsav304.sakura.ne.jp); Wed, 14 Apr 2021 20:11:49 +0900 (JST) X-Virus-Status: clean(F-Secure/fsigk_smtp/550/fsav304.sakura.ne.jp) Received: from [192.168.1.9] (M106072142033.v4.enabler.ne.jp [106.72.142.33]) (authenticated bits=0) by www262.sakura.ne.jp (8.15.2/8.15.2) with ESMTPSA id 13EBBhIP073266 (version=TLSv1.2 cipher=AES256-GCM-SHA384 bits=256 verify=NO); Wed, 14 Apr 2021 20:11:49 +0900 (JST) (envelope-from penguin-kernel@i-love.sakura.ne.jp) Subject: Re: How to handle concurrent access to /dev/ttyprintk ? From: Tetsuo Handa To: Greg Kroah-Hartman , Samo Pogacnik , Jiri Slaby Cc: Petr Mladek , Sergey Senozhatsky , Steven Rostedt , John Ogness , linux-kernel@vger.kernel.org, syzkaller-bugs References: <20210403041444.4081-1-penguin-kernel@I-love.SAKURA.ne.jp> <3c15d32f-c568-7f6f-fa7e-af4deb9b49f9@i-love.sakura.ne.jp> <051b550c-1cdd-6503-d2b7-0877bf0578fc@i-love.sakura.ne.jp> <32e75be6-6e9f-b33f-d585-13db220519da@i-love.sakura.ne.jp> Message-ID: <095d5393-b212-c4d8-5d6d-666bd505cc3d@i-love.sakura.ne.jp> Date: Wed, 14 Apr 2021 20:11:40 +0900 User-Agent: Mozilla/5.0 (Windows NT 6.3; Win64; x64; rv:78.0) Gecko/20100101 Thunderbird/78.9.1 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 2021/04/14 9:45, Tetsuo Handa wrote: > On 2021/04/12 21:04, Greg Kroah-Hartman wrote: >>> Since syzkaller is a fuzzer, syzkaller happily opens /dev/ttyprintk from >>> multiple threads. Should we update syzkaller to use CONFIG_TTY_PRINTK=n ? >> >> Why? Can you not hit the same tty code paths from any other tty driver >> being open multiple times? Why is ttyprintk somehow "special" here? > > I found a simplified reproducer. If we call ioctl(TIOCVHANGUP) on /dev/ttyprintk , > "struct ttyprintk_port tpk_port".port.count cannot be decremented by > tty_port_close() from tpk_close() due to tty_hung_up_p() == true when > close() is called. As a result, tty->count and port count gets out of sync. > > Then, when /dev/ttyprintk is opened again and then closed without calling > ioctl(TIOCVHANGUP), this message is printed due to tty_hung_up_p() == false. > > If I replace /dev/ttyprintk with /dev/ttyS0 in the reproducer shown above, > this message is not printed. > The difference between /dev/ttyS0 and /dev/ttyprintk is that the former provides uart_hangup() as "struct tty_operations"->hangup while the latter does not provide "struct tty_operations"->hangup . It seems to me that below patch avoids this message, but I'm not familiar with tty code. Is this fix correct? Don't we need to enforce all drivers to provide "struct tty_operations"->hangup in order to reset port count ? diff --git a/drivers/char/ttyprintk.c b/drivers/char/ttyprintk.c index 6a0059e508e3..ff3b9a41b91b 100644 --- a/drivers/char/ttyprintk.c +++ b/drivers/char/ttyprintk.c @@ -158,12 +158,26 @@ static int tpk_ioctl(struct tty_struct *tty, return 0; } +/* + * TTY operations hangup function. + */ +static void tpk_hangup(struct tty_struct *tty) +{ + struct ttyprintk_port *tpkp = tty->driver_data; + unsigned long flags; + + spin_lock_irqsave(&tpkp->port.lock, flags); + tpkp->port.count = 0; + spin_unlock_irqrestore(&tpkp->port.lock, flags); +} + static const struct tty_operations ttyprintk_ops = { .open = tpk_open, .close = tpk_close, .write = tpk_write, .write_room = tpk_write_room, .ioctl = tpk_ioctl, + .hangup = tpk_hangup, }; static const struct tty_port_operations null_ops = { };