Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <ab7d0789-7e97-5ceb-a5de-110c62f08780@kernel.org>
Date:   Fri, 9 Dec 2022 09:43:32 +0100
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.5.0
Subject: Re: [PATCH 1/1] tty: fix out-of-bounds access in
 tty_driver_lookup_tty()
Content-Language: en-US
To:     Sven Schnelle <svens@linux.ibm.com>
Cc:     Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        Christian Borntraeger <borntraeger@de.ibm.com>,
        linux-kernel@vger.kernel.org, linux-s390@vger.kernel.org
References: <20221207075236.23171-1-svens@linux.ibm.com>
 <20221207075236.23171-2-svens@linux.ibm.com>
 <6614f5d2-d99b-ea8a-f660-7a3433499a9f@kernel.org>
 <yt9dtu25vvyh.fsf@linux.ibm.com>
From:   Jiri Slaby <jirislaby@kernel.org>
In-Reply-To: <yt9dtu25vvyh.fsf@linux.ibm.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 09. 12. 22, 9:10, Sven Schnelle wrote:
> Jiri Slaby <jirislaby@kernel.org> writes:
> 
>> On 07. 12. 22, 8:52, Sven Schnelle wrote:
>>> When specifying an invalid console= device like console=tty3270,
>>> tty_driver_lookup_tty() returns the tty struct without checking
>>> whether index is a valid number.
>>> [..]
>>
>> Reviewed-by: Jiri Slaby <jirislaby@kernel.org>
>>
>> Yes, this makes sense as a sanity check for all drivers. But I would
>> _also_ disallow registering such a console in vt:
>> --- a/drivers/tty/vt/vt.c
>> +++ b/drivers/tty/vt/vt.c
>> @@ -3075,6 +3075,11 @@ int vt_kmsg_redirect(int new)
>>    * The console must be locked when we get here.
>>    */
>>
>> +static int vt_console_setup(struct console *co, char *options)
>> +{
>> +       return co->index >= MAX_NR_CONSOLES ? -EINVAL : 0;
>> +}
>> +
>>   static void vt_console_print(struct console *co, const char *b,
>>   unsigned count)
>>   {
>>          struct vc_data *vc = vc_cons[fg_console].d;
>> @@ -3158,6 +3163,7 @@ static struct tty_driver
>> *vt_console_device(struct console *c, int *index)
>>
>>   static struct console vt_console_driver = {
>>          .name           = "tty",
>> +       .setup          = vt_console_setup,
>>          .write          = vt_console_print,
>>          .device         = vt_console_device,
>>          .unblank        = unblank_screen,
>>
>> That means dmesg would say:
>>    Console: colour dummy device 80x25
>>    printk: console [ttyS0] enabled
>>
>> And not:
>>    Console: colour dummy device 80x25
>>    printk: console [tty3270] enabled
>>    printk: console [ttyS0] enabled
> 
> Makes sense. Should i add that to my patch, add a second patch, or
> will you submit that?

If you can create a second patch, it would be great. And if you redo 
this one, please trim the stack traces (you can likely even drop the 
first one).

thanks,
-- 
js
suse labs