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;
From:   Sven Schnelle <svens@linux.ibm.com>
To:     Jiri Slaby <jirislaby@kernel.org>
Cc:     Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        Christian Borntraeger <borntraeger@de.ibm.com>,
        linux-kernel@vger.kernel.org, linux-s390@vger.kernel.org
Subject: Re: [PATCH 1/1] tty: fix out-of-bounds access in
 tty_driver_lookup_tty()
References: <20221207075236.23171-1-svens@linux.ibm.com>
        <20221207075236.23171-2-svens@linux.ibm.com>
        <6614f5d2-d99b-ea8a-f660-7a3433499a9f@kernel.org>
Date:   Fri, 09 Dec 2022 09:10:30 +0100
In-Reply-To: <6614f5d2-d99b-ea8a-f660-7a3433499a9f@kernel.org> (Jiri Slaby's
        message of "Fri, 9 Dec 2022 08:17:47 +0100")
Message-ID: <yt9dtu25vvyh.fsf@linux.ibm.com>
User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/28.0.50 (gnu/linux)
MIME-Version: 1.0
Content-Type: text/plain
Precedence: bulk

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?

Thanks
Sven