2023-10-04 08:56:10

by Tony Lindgren

[permalink] [raw]
Subject: [PATCH] serial: 8250: Check for valid console index

Let's not allow negative numbers for console index.

Signed-off-by: Tony Lindgren <[email protected]>
---
drivers/tty/serial/8250/8250_core.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c
--- a/drivers/tty/serial/8250/8250_core.c
+++ b/drivers/tty/serial/8250/8250_core.c
@@ -611,7 +611,7 @@ static int univ8250_console_setup(struct console *co, char *options)
* if so, search for the first available port that does have
* console support.
*/
- if (co->index >= UART_NR)
+ if (co->index < 0 || co->index >= UART_NR)
co->index = 0;

/*
--
2.42.0


2023-10-04 09:06:40

by Ilpo Järvinen

[permalink] [raw]
Subject: Re: [PATCH] serial: 8250: Check for valid console index

On Wed, 4 Oct 2023, Tony Lindgren wrote:

> Let's not allow negative numbers for console index.
>
> Signed-off-by: Tony Lindgren <[email protected]>
> ---
> drivers/tty/serial/8250/8250_core.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c
> --- a/drivers/tty/serial/8250/8250_core.c
> +++ b/drivers/tty/serial/8250/8250_core.c
> @@ -611,7 +611,7 @@ static int univ8250_console_setup(struct console *co, char *options)
> * if so, search for the first available port that does have
> * console support.
> */
> - if (co->index >= UART_NR)
> + if (co->index < 0 || co->index >= UART_NR)
> co->index = 0;

The inconsistencies how different serial drivers handle this situation
seem staggering. Perhaps there should be some effort to make the behavior
uniform across them?

--
i.

2023-10-04 09:09:10

by Tony Lindgren

[permalink] [raw]
Subject: Re: [PATCH] serial: 8250: Check for valid console index

* Ilpo Järvinen <[email protected]> [231004 09:05]:
> On Wed, 4 Oct 2023, Tony Lindgren wrote:
>
> > Let's not allow negative numbers for console index.
> >
> > Signed-off-by: Tony Lindgren <[email protected]>
> > ---
> > drivers/tty/serial/8250/8250_core.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c
> > --- a/drivers/tty/serial/8250/8250_core.c
> > +++ b/drivers/tty/serial/8250/8250_core.c
> > @@ -611,7 +611,7 @@ static int univ8250_console_setup(struct console *co, char *options)
> > * if so, search for the first available port that does have
> > * console support.
> > */
> > - if (co->index >= UART_NR)
> > + if (co->index < 0 || co->index >= UART_NR)
> > co->index = 0;
>
> The inconsistencies how different serial drivers handle this situation
> seem staggering. Perhaps there should be some effort to make the behavior
> uniform across them?

Hmm yeah we should just have them all check for co->index < 0.

Regards,

Tony

2023-10-04 09:13:42

by Ilpo Järvinen

[permalink] [raw]
Subject: Re: [PATCH] serial: 8250: Check for valid console index

On Wed, 4 Oct 2023, Tony Lindgren wrote:

> * Ilpo Järvinen <[email protected]> [231004 09:05]:
> > On Wed, 4 Oct 2023, Tony Lindgren wrote:
> >
> > > Let's not allow negative numbers for console index.
> > >
> > > Signed-off-by: Tony Lindgren <[email protected]>
> > > ---
> > > drivers/tty/serial/8250/8250_core.c | 2 +-
> > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c
> > > --- a/drivers/tty/serial/8250/8250_core.c
> > > +++ b/drivers/tty/serial/8250/8250_core.c
> > > @@ -611,7 +611,7 @@ static int univ8250_console_setup(struct console *co, char *options)
> > > * if so, search for the first available port that does have
> > > * console support.
> > > */
> > > - if (co->index >= UART_NR)
> > > + if (co->index < 0 || co->index >= UART_NR)
> > > co->index = 0;
> >
> > The inconsistencies how different serial drivers handle this situation
> > seem staggering. Perhaps there should be some effort to make the behavior
> > uniform across them?
>
> Hmm yeah we should just have them all check for co->index < 0.

Right but it's only about that, some return -Exx codes (more than one -Exx
variant) and some do assign just 0 like here.

--
i.

2023-10-04 09:21:41

by Tony Lindgren

[permalink] [raw]
Subject: Re: [PATCH] serial: 8250: Check for valid console index

* Ilpo Järvinen <[email protected]> [231004 09:13]:
> On Wed, 4 Oct 2023, Tony Lindgren wrote:
>
> > * Ilpo Järvinen <[email protected]> [231004 09:05]:
> > > On Wed, 4 Oct 2023, Tony Lindgren wrote:
> > >
> > > > Let's not allow negative numbers for console index.
> > > >
> > > > Signed-off-by: Tony Lindgren <[email protected]>
> > > > ---
> > > > drivers/tty/serial/8250/8250_core.c | 2 +-
> > > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > > >
> > > > diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c
> > > > --- a/drivers/tty/serial/8250/8250_core.c
> > > > +++ b/drivers/tty/serial/8250/8250_core.c
> > > > @@ -611,7 +611,7 @@ static int univ8250_console_setup(struct console *co, char *options)
> > > > * if so, search for the first available port that does have
> > > > * console support.
> > > > */
> > > > - if (co->index >= UART_NR)
> > > > + if (co->index < 0 || co->index >= UART_NR)
> > > > co->index = 0;
> > >
> > > The inconsistencies how different serial drivers handle this situation
> > > seem staggering. Perhaps there should be some effort to make the behavior
> > > uniform across them?
> >
> > Hmm yeah we should just have them all check for co->index < 0.
>
> Right but it's only about that, some return -Exx codes (more than one -Exx
> variant) and some do assign just 0 like here.

What do you have in mind then? They should all assume co->index < 0 is an
invalid console index still, right :)

Regards,

Tony

2023-10-05 14:29:51

by Andy Shevchenko

[permalink] [raw]
Subject: Re: [PATCH] serial: 8250: Check for valid console index

On Wed, Oct 04, 2023 at 11:55:10AM +0300, Tony Lindgren wrote:
> Let's not allow negative numbers for console index.

...

> - if (co->index >= UART_NR)
> + if (co->index < 0 || co->index >= UART_NR)
> co->index = 0;

in_range() ?

--
With Best Regards,
Andy Shevchenko