[PATCH] printk: Remove console options before decoding the name

[PATCH] printk: Remove console options before decoding the name

[David Engraf]

This fixes a regression when a console option includes ':'. Commit
7640f1a44eba ("printk: Add match_devname_and_update_preferred_console()")
introduced console=DEVNAME:0.0 hardware style addressing by looking for a
colon. If the colon is part of an option the name is handled as devname
instead of ttyname.

Fix by handling the options first which will add a NULL terminator to the
string.

Signed-off-by: David Engraf <david.engraf@sysgo.com>
---
 kernel/printk/printk.c | 12 +++++-------
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
index 6d3d18a50da74..c297a0ae04f7b 100644
--- a/kernel/printk/printk.c
+++ b/kernel/printk/printk.c
@@ -2646,24 +2646,22 @@ static int __init console_setup(char *str)
 	if (_braille_console_setup(&str, &brl_options))
 		return 1;
 
+	/* Decode str into name, index, options */
+	options = strchr(str, ',');
+	if (options)
+		*(options++) = 0;
+
 	/* For a DEVNAME:0.0 style console the character device is unknown early */
 	if (strchr(str, ':'))
 		devname = buf;
 	else
 		ttyname = buf;
 
-	/*
-	 * Decode str into name, index, options.
-	 */
 	if (ttyname && isdigit(str[0]))
 		scnprintf(buf, sizeof(buf), "ttyS%s", str);
 	else
 		strscpy(buf, str);
 
-	options = strchr(str, ',');
-	if (options)
-		*(options++) = 0;
-
 #ifdef __sparc__
 	if (!strcmp(str, "ttya"))
 		strscpy(buf, "ttyS0");
-- 
2.53.0

Re: [PATCH] printk: Remove console options before decoding the name

[Tony Lindgren]

On Wed, Sep 16, 2026 at 08:32:18AM +0300, David Engraf wrote:
> --- a/kernel/printk/printk.c
> +++ b/kernel/printk/printk.c
> @@ -2646,24 +2646,22 @@ static int __init console_setup(char *str)
>  	if (_braille_console_setup(&str, &brl_options))
>  		return 1;
>  
> +	/* Decode str into name, index, options */
> +	options = strchr(str, ',');
> +	if (options)
> +		*(options++) = 0;
> +

How about update the comment for why it needs to be first?

Maybe something like:

Decode str into options first. The options may contain a ':' used also
for DEVNAME.

Re: [PATCH] printk: Remove console options before decoding the name

[David Engraf]

On 16.09.26 08:48 wrote Tony Lindgren:
> On Wed, Sep 16, 2026 at 08:32:18AM +0300, David Engraf wrote:
>> --- a/kernel/printk/printk.c
>> +++ b/kernel/printk/printk.c
>> @@ -2646,24 +2646,22 @@ static int __init console_setup(char *str)
>>   	if (_braille_console_setup(&str, &brl_options))
>>   		return 1;
>>   
>> +	/* Decode str into name, index, options */
>> +	options = strchr(str, ',');
>> +	if (options)
>> +		*(options++) = 0;
>> +
> 
> How about update the comment for why it needs to be first?
> 
> Maybe something like:
> 
> Decode str into options first. The options may contain a ':' used also
> for DEVNAME.

Okay I can update the comment if there are no other objections.

Best regards
- David

[PATCH v2] printk: Remove console options before decoding the name

[David Engraf]

This fixes a regression when a console option includes ':'. Commit
7640f1a44eba ("printk: Add match_devname_and_update_preferred_console()")
introduced console=DEVNAME:0.0 hardware style addressing by looking for a
colon. If the colon is part of an option the name is handled as devname
instead of ttyname.

Fix by handling the options first which will add a NULL terminator to the
string.

Signed-off-by: David Engraf <david.engraf@sysgo.com>
---
 kernel/printk/printk.c | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
index 6d3d18a50da74..f4803fe05a0aa 100644
--- a/kernel/printk/printk.c
+++ b/kernel/printk/printk.c
@@ -2646,24 +2646,25 @@ static int __init console_setup(char *str)
 	if (_braille_console_setup(&str, &brl_options))
 		return 1;
 
+	/*
+	 * Decode str into name, index and options. Start with options, since
+	 * it might also contain a ':' used for DEVNAME.
+	 */
+	options = strchr(str, ',');
+	if (options)
+		*(options++) = 0;
+
 	/* For a DEVNAME:0.0 style console the character device is unknown early */
 	if (strchr(str, ':'))
 		devname = buf;
 	else
 		ttyname = buf;
 
-	/*
-	 * Decode str into name, index, options.
-	 */
 	if (ttyname && isdigit(str[0]))
 		scnprintf(buf, sizeof(buf), "ttyS%s", str);
 	else
 		strscpy(buf, str);
 
-	options = strchr(str, ',');
-	if (options)
-		*(options++) = 0;
-
 #ifdef __sparc__
 	if (!strcmp(str, "ttya"))
 		strscpy(buf, "ttyS0");
-- 
2.53.0

Re: [PATCH v2] printk: Remove console options before decoding the name

[Tony Lindgren]

On Thu, Sep 17, 2026 at 09:05:51AM +0300, David Engraf wrote:
> This fixes a regression when a console option includes ':'. Commit
> 7640f1a44eba ("printk: Add match_devname_and_update_preferred_console()")
> introduced console=DEVNAME:0.0 hardware style addressing by looking for a
> colon. If the colon is part of an option the name is handled as devname
> instead of ttyname.

Just curious, which console did you hit this issue with?

In any case, thanks for updating the comments:

Reviewed-by: Tony Lindgren <tony.lindgren@linux.intel.com>

Re: [PATCH v2] printk: Remove console options before decoding the name

[David Engraf]

On 17.09.26 17:03 wrote Tony Lindgren:
> On Thu, Sep 17, 2026 at 09:05:51AM +0300, David Engraf wrote:
>> This fixes a regression when a console option includes ':'. Commit
>> 7640f1a44eba ("printk: Add match_devname_and_update_preferred_console()")
>> introduced console=DEVNAME:0.0 hardware style addressing by looking for a
>> colon. If the colon is part of an option the name is handled as devname
>> instead of ttyname.
> 
> Just curious, which console did you hit this issue with?

It's a self-developed console driver to access our hypervisor.

> In any case, thanks for updating the comments:
> 
> Reviewed-by: Tony Lindgren <tony.lindgren@linux.intel.com>

Thanks
- David

Re: [PATCH v2] printk: Remove console options before decoding the name

[Tony Lindgren]

On Fri, Sep 18, 2026 at 09:27:03AM +0300, David Engraf wrote:
> On 17.09.26 17:03 wrote Tony Lindgren:
> > On Thu, Sep 17, 2026 at 09:05:51AM +0300, David Engraf wrote:
> > > This fixes a regression when a console option includes ':'. Commit
> > > 7640f1a44eba ("printk: Add match_devname_and_update_preferred_console()")
> > > introduced console=DEVNAME:0.0 hardware style addressing by looking for a
> > > colon. If the colon is part of an option the name is handled as devname
> > > instead of ttyname.
> > 
> > Just curious, which console did you hit this issue with?
> 
> It's a self-developed console driver to access our hypervisor.

OK thanks. From a "fix or feature" point of view, I wonder if this issue
can happen with some of the current Linux console drivers too?

Re: [PATCH v2] printk: Remove console options before decoding the name

[David Engraf]

On 21.09.26 08:17 wrote Tony Lindgren:
> On Fri, Sep 18, 2026 at 09:27:03AM +0300, David Engraf wrote:
>> On 17.09.26 17:03 wrote Tony Lindgren:
>>> On Thu, Sep 17, 2026 at 09:05:51AM +0300, David Engraf wrote:
>>>> This fixes a regression when a console option includes ':'. Commit
>>>> 7640f1a44eba ("printk: Add match_devname_and_update_preferred_console()")
>>>> introduced console=DEVNAME:0.0 hardware style addressing by looking for a
>>>> colon. If the colon is part of an option the name is handled as devname
>>>> instead of ttyname.
>>>
>>> Just curious, which console did you hit this issue with?
>>
>> It's a self-developed console driver to access our hypervisor.
> 
> OK thanks. From a "fix or feature" point of view, I wonder if this issue
> can happen with some of the current Linux console drivers too?

AFAIK there is no restriction using ':' in the console options even if I 
don't know any in-kernel driver using it. That's why I would say it's a 
bug fix.

Best regards
- David

Re: [PATCH v2] printk: Remove console options before decoding the name

[Petr Mladek]

On Thu 2026-09-17 09:05:51, David Engraf wrote:
> This fixes a regression when a console option includes ':'. Commit
> 7640f1a44eba ("printk: Add match_devname_and_update_preferred_console()")
> introduced console=DEVNAME:0.0 hardware style addressing by looking for a
> colon. If the colon is part of an option the name is handled as devname
> instead of ttyname.
> 
> Fix by handling the options first which will add a NULL terminator to the
> string.
> 
> Signed-off-by: David Engraf <david.engraf@sysgo.com>
> ---
>  kernel/printk/printk.c | 15 ++++++++-------
>  1 file changed, 8 insertions(+), 7 deletions(-)
> 
> diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
> index 6d3d18a50da74..f4803fe05a0aa 100644
> --- a/kernel/printk/printk.c
> +++ b/kernel/printk/printk.c
> @@ -2646,24 +2646,25 @@ static int __init console_setup(char *str)
>  	if (_braille_console_setup(&str, &brl_options))
>  		return 1;
>  
> +	/*
> +	 * Decode str into name, index and options. Start with options, since
> +	 * it might also contain a ':' used for DEVNAME.
> +	 */
> +	options = strchr(str, ',');
> +	if (options)
> +		*(options++) = 0;
> +
>  	/* For a DEVNAME:0.0 style console the character device is unknown early */
>  	if (strchr(str, ':'))
>  		devname = buf;
>  	else
>  		ttyname = buf;
>  
> -	/*
> -	 * Decode str into name, index, options.
> -	 */
>  	if (ttyname && isdigit(str[0]))
>  		scnprintf(buf, sizeof(buf), "ttyS%s", str);
>  	else
>  		strscpy(buf, str);

Sashiko AI has the following comment:

| Does moving the options parsing and null-termination earlier in this function
| leave the loop below with an unreachable condition?
| 
| Since str is now truncated at the first comma before being copied into buf,
| buf will never contain a comma. This means the comma check inside the loop
| over buf appears to be structurally impossible to satisfy:
| 
| 	for (s = buf; *s; s++)
| 		if ((ttyname && isdigit(*s)) || *s == ',')
| 			break;
| 
| Can the comma check be safely removed from the loop condition?

And it is right. The original code copied the original string into
"buf". The new does not copy the options any longer.

It would deserve some refactoring to make the code cleaner.
Something like:

diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
index f4803fe05a0a..966744fb4bcc 100644
--- a/kernel/printk/printk.c
+++ b/kernel/printk/printk.c
@@ -2672,17 +2672,18 @@ static int __init console_setup(char *str)
 		strscpy(buf, "ttyS1");
 #endif
 
-	for (s = buf; *s; s++)
-		if ((ttyname && isdigit(*s)) || *s == ',')
-			break;
-
-	/* @idx will get defined when devname matches. */
-	if (devname)
-		idx = -1;
-	else
+	if (ttyname) {
+		/* Detect @idx in ttyname and remove it. */
+		for (s = buf; *s; s++) {
+			if (isdigit(*s))
+				break;
+		}
 		idx = simple_strtoul(s, NULL, 10);
-
-	*s = 0;
+		*s = 0;
+	} else {
+		/* @idx will get defined when devname matches. */
+		idx = -1;
+	}
 
 	__add_preferred_console(ttyname, idx, devname, options, brl_options, true);
 	return 1;


I see two possibilities. We could either merge this cleanup into the
original patch and send v3. Or we could add it on top of the original
patch.

I would slightly prefer v3 and have both changes in a single patch.

Best Regards,
Petr

Re: [PATCH v2] printk: Remove console options before decoding the name

[Tony Lindgren]

On Wed, Sep 23, 2026 at 11:37:03AM +0200, Petr Mladek wrote:
> On Thu 2026-09-17 09:05:51, David Engraf wrote:
> > This fixes a regression when a console option includes ':'. Commit
> > 7640f1a44eba ("printk: Add match_devname_and_update_preferred_console()")
> > introduced console=DEVNAME:0.0 hardware style addressing by looking for a
> > colon. If the colon is part of an option the name is handled as devname
> > instead of ttyname.
> > 
> > Fix by handling the options first which will add a NULL terminator to the
> > string.
> > 
> > Signed-off-by: David Engraf <david.engraf@sysgo.com>
> > ---
> >  kernel/printk/printk.c | 15 ++++++++-------
> >  1 file changed, 8 insertions(+), 7 deletions(-)
> > 
> > diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
> > index 6d3d18a50da74..f4803fe05a0aa 100644
> > --- a/kernel/printk/printk.c
> > +++ b/kernel/printk/printk.c
> > @@ -2646,24 +2646,25 @@ static int __init console_setup(char *str)
> >  	if (_braille_console_setup(&str, &brl_options))
> >  		return 1;
> >  
> > +	/*
> > +	 * Decode str into name, index and options. Start with options, since
> > +	 * it might also contain a ':' used for DEVNAME.
> > +	 */
> > +	options = strchr(str, ',');
> > +	if (options)
> > +		*(options++) = 0;
> > +
> >  	/* For a DEVNAME:0.0 style console the character device is unknown early */
> >  	if (strchr(str, ':'))
> >  		devname = buf;
> >  	else
> >  		ttyname = buf;
> >  
> > -	/*
> > -	 * Decode str into name, index, options.
> > -	 */
> >  	if (ttyname && isdigit(str[0]))
> >  		scnprintf(buf, sizeof(buf), "ttyS%s", str);
> >  	else
> >  		strscpy(buf, str);
> 
> Sashiko AI has the following comment:
> 
> | Does moving the options parsing and null-termination earlier in this function
> | leave the loop below with an unreachable condition?
> | 
> | Since str is now truncated at the first comma before being copied into buf,
> | buf will never contain a comma. This means the comma check inside the loop
> | over buf appears to be structurally impossible to satisfy:
> | 
> | 	for (s = buf; *s; s++)
> | 		if ((ttyname && isdigit(*s)) || *s == ',')
> | 			break;
> | 
> | Can the comma check be safely removed from the loop condition?
> 
> And it is right. The original code copied the original string into
> "buf". The new does not copy the options any longer.

OK
 
> It would deserve some refactoring to make the code cleaner.
> Something like:
> 
> diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
> index f4803fe05a0a..966744fb4bcc 100644
> --- a/kernel/printk/printk.c
> +++ b/kernel/printk/printk.c
> @@ -2672,17 +2672,18 @@ static int __init console_setup(char *str)
>  		strscpy(buf, "ttyS1");
>  #endif
>  
> -	for (s = buf; *s; s++)
> -		if ((ttyname && isdigit(*s)) || *s == ',')
> -			break;
> -
> -	/* @idx will get defined when devname matches. */
> -	if (devname)
> -		idx = -1;
> -	else
> +	if (ttyname) {
> +		/* Detect @idx in ttyname and remove it. */
> +		for (s = buf; *s; s++) {
> +			if (isdigit(*s))
> +				break;
> +		}
>  		idx = simple_strtoul(s, NULL, 10);
> -
> -	*s = 0;
> +		*s = 0;
> +	} else {
> +		/* @idx will get defined when devname matches. */
> +		idx = -1;
> +	}
>  
>  	__add_preferred_console(ttyname, idx, devname, options, brl_options, true);
>  	return 1;

Nice, you could now initialize idx = -1 to start with to leave out the
else for setting devname idx?
 
> I see two possibilities. We could either merge this cleanup into the
> original patch and send v3. Or we could add it on top of the original
> patch.
> 
> I would slightly prefer v3 and have both changes in a single patch.

Having a v3 sounds good to me.

Re: [PATCH v2] printk: Remove console options before decoding the name

[Petr Mladek]

On Wed 2026-09-23 13:19:15, Tony Lindgren wrote:
> On Wed, Sep 23, 2026 at 11:37:03AM +0200, Petr Mladek wrote:
> > On Thu 2026-09-17 09:05:51, David Engraf wrote:
> > > This fixes a regression when a console option includes ':'. Commit
> > > 7640f1a44eba ("printk: Add match_devname_and_update_preferred_console()")
> > > introduced console=DEVNAME:0.0 hardware style addressing by looking for a
> > > colon. If the colon is part of an option the name is handled as devname
> > > instead of ttyname.
> > > 
> > > Fix by handling the options first which will add a NULL terminator to the
> > > string.
> > > 
> > > --- a/kernel/printk/printk.c
> > > +++ b/kernel/printk/printk.c
> > > @@ -2646,24 +2646,25 @@ static int __init console_setup(char *str)
> > >  	if (_braille_console_setup(&str, &brl_options))
> > >  		return 1;
> > >  
> > > +	/*
> > > +	 * Decode str into name, index and options. Start with options, since
> > > +	 * it might also contain a ':' used for DEVNAME.
> > > +	 */
> > > +	options = strchr(str, ',');
> > > +	if (options)
> > > +		*(options++) = 0;
> > > +
> > >  	/* For a DEVNAME:0.0 style console the character device is unknown early */
> > >  	if (strchr(str, ':'))
> > >  		devname = buf;
> > >  	else
> > >  		ttyname = buf;
> > >  
> > > -	/*
> > > -	 * Decode str into name, index, options.
> > > -	 */
> > >  	if (ttyname && isdigit(str[0]))
> > >  		scnprintf(buf, sizeof(buf), "ttyS%s", str);
> > >  	else
> > >  		strscpy(buf, str);
> > 
> > Sashiko AI has the following comment:
> > 
> > | Does moving the options parsing and null-termination earlier in this function
> > | leave the loop below with an unreachable condition?
> > | 
> > | Since str is now truncated at the first comma before being copied into buf,
> > | buf will never contain a comma. This means the comma check inside the loop
> > | over buf appears to be structurally impossible to satisfy:
> > | 
> > | 	for (s = buf; *s; s++)
> > | 		if ((ttyname && isdigit(*s)) || *s == ',')
> > | 			break;
> > | 
> > | Can the comma check be safely removed from the loop condition?
> > 
> > And it is right. The original code copied the original string into
> > "buf". The new does not copy the options any longer.
> 
> OK
>  
> > It would deserve some refactoring to make the code cleaner.
> > Something like:
> > 
> > diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
> > index f4803fe05a0a..966744fb4bcc 100644
> > --- a/kernel/printk/printk.c
> > +++ b/kernel/printk/printk.c
> > @@ -2672,17 +2672,18 @@ static int __init console_setup(char *str)
> >  		strscpy(buf, "ttyS1");
> >  #endif
> >  
> > -	for (s = buf; *s; s++)
> > -		if ((ttyname && isdigit(*s)) || *s == ',')
> > -			break;
> > -
> > -	/* @idx will get defined when devname matches. */
> > -	if (devname)
> > -		idx = -1;
> > -	else
> > +	if (ttyname) {
> > +		/* Detect @idx in ttyname and remove it. */
> > +		for (s = buf; *s; s++) {
> > +			if (isdigit(*s))
> > +				break;
> > +		}
> >  		idx = simple_strtoul(s, NULL, 10);
> > -
> > -	*s = 0;
> > +		*s = 0;
> > +	} else {
> > +		/* @idx will get defined when devname matches. */
> > +		idx = -1;
> > +	}
> >  
> >  	__add_preferred_console(ttyname, idx, devname, options, brl_options, true);
> >  	return 1;
> 
> Nice, you could now initialize idx = -1 to start with to leave out the
> else for setting devname idx?

I would personally prefer to keep the else part because it makes it
clear how the "devname" variant is handled. But I could live without
it as well.

> > I see two possibilities. We could either merge this cleanup into the
> > original patch and send v3. Or we could add it on top of the original
> > patch.
> > 
> > I would slightly prefer v3 and have both changes in a single patch.
> 
> Having a v3 sounds good to me.

Great.

Best Regards,
Petr

Re: [PATCH v2] printk: Remove console options before decoding the name

[Tony Lindgren]

On Wed, Sep 23, 2026 at 02:06:26PM +0200, Petr Mladek wrote:
> On Wed 2026-09-23 13:19:15, Tony Lindgren wrote:
> > Nice, you could now initialize idx = -1 to start with to leave out the
> > else for setting devname idx?
> 
> I would personally prefer to keep the else part because it makes it
> clear how the "devname" variant is handled. But I could live without
> it as well.

OK thanks, that works just fine for me.

Re: [PATCH v2] printk: Remove console options before decoding the name

[David Engraf]

On 23.09.26 15:14 wrote Tony Lindgren:
> On Wed, Sep 23, 2026 at 02:06:26PM +0200, Petr Mladek wrote:
>> On Wed 2026-09-23 13:19:15, Tony Lindgren wrote:
>>> Nice, you could now initialize idx = -1 to start with to leave out the
>>> else for setting devname idx?
>>
>> I would personally prefer to keep the else part because it makes it
>> clear how the "devname" variant is handled. But I could live without
>> it as well.
> 
> OK thanks, that works just fine for me.

Thanks for the review. I'm going to prepare v3 including Petr's changes.

Best regards
- David