[PATCH 1/2] of: Fix crash if an earlycon driver is not found

[PATCH 1/2] of: Fix crash if an earlycon driver is not found

[Kevin Cernekee]

__earlycon_of_table_sentinel.compatible is a char[128], not a pointer, so
it will never be NULL.  Checking it against NULL causes the match loop to
run past the end of the array, and eventually match a bogus entry, under
the following conditions:

 - Kernel command line specifies "earlycon" with no parameters
 - DT has a stdout-path pointing to a UART node
 - The UART driver doesn't use OF_EARLYCON_DECLARE (or maybe the console
   driver is compiled out)

Fix this by checking to see if match->compatible is a non-empty string.

Signed-off-by: Kevin Cernekee <cernekee@gmail.com>
Cc: <stable@vger.kernel.org> # 3.16+
---
 drivers/of/fdt.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
index d1ffca8..30e97bc 100644
--- a/drivers/of/fdt.c
+++ b/drivers/of/fdt.c
@@ -773,7 +773,7 @@ int __init early_init_dt_scan_chosen_serial(void)
 	if (offset < 0)
 		return -ENODEV;
 
-	while (match->compatible) {
+	while (match->compatible[0]) {
 		unsigned long addr;
 		if (fdt_node_check_compatible(fdt, offset, match->compatible)) {
 			match++;
-- 
2.1.1

[PATCH 2/2] tty: serial: bcm63xx: Allow device nodes to be renamed to /dev/ttyBCM*

[Kevin Cernekee]

By default, bcm63xx_uart.c uses the standard 8250 device naming and
major/minor numbers.  There are at least two situations where this could
be a problem:

1) Multiplatform kernels that need to support some chips that have 8250
UARTs and other chips that have bcm63xx UARTs.

2) Some older chips like BCM7125 have a mix of both UART types.

Add a new Kconfig option to tell the driver whether to register itself
as ttyS or ttyBCM.  By default it will retain the existing "ttyS"
behavior to avoid surprises.

Signed-off-by: Kevin Cernekee <cernekee@gmail.com>
---
 drivers/tty/serial/Kconfig        | 11 +++++++++++
 drivers/tty/serial/bcm63xx_uart.c |  8 ++++++++
 2 files changed, 19 insertions(+)

diff --git a/drivers/tty/serial/Kconfig b/drivers/tty/serial/Kconfig
index fdd851e..c82cfd2 100644
--- a/drivers/tty/serial/Kconfig
+++ b/drivers/tty/serial/Kconfig
@@ -1302,6 +1302,17 @@ config SERIAL_BCM63XX_CONSOLE
 	  If you have enabled the serial port on the BCM63xx CPU
 	  you can make it the console by answering Y to this option.
 
+config SERIAL_BCM63XX_TTYS
+	bool "Use ttyS[01] device nodes for bcm63xx_uart"
+	depends on SERIAL_BCM63XX
+	default y
+	help
+	  Say Y to name the serial ports /dev/ttyS0, ttyS1, ...
+	  This conflicts with the 8250 driver but may avoid breaking
+	  compatibility with existing init scripts.
+
+	  Say N to name the serial ports /dev/ttyBCM0, ttyBCM1, ...
+
 config SERIAL_GRLIB_GAISLER_APBUART
 	tristate "GRLIB APBUART serial support"
 	depends on OF && SPARC
diff --git a/drivers/tty/serial/bcm63xx_uart.c b/drivers/tty/serial/bcm63xx_uart.c
index e04e580..9f3dcc8 100644
--- a/drivers/tty/serial/bcm63xx_uart.c
+++ b/drivers/tty/serial/bcm63xx_uart.c
@@ -751,7 +751,11 @@ static int bcm_console_setup(struct console *co, char *options)
 static struct uart_driver bcm_uart_driver;
 
 static struct console bcm63xx_console = {
+#ifdef CONFIG_SERIAL_BCM63XX_TTYS
 	.name		= "ttyS",
+#else
+	.name		= "ttyBCM",
+#endif
 	.write		= bcm_console_write,
 	.device		= uart_console_device,
 	.setup		= bcm_console_setup,
@@ -796,9 +800,13 @@ OF_EARLYCON_DECLARE(bcm63xx_uart, "brcm,bcm6345-uart", bcm_early_console_setup);
 static struct uart_driver bcm_uart_driver = {
 	.owner		= THIS_MODULE,
 	.driver_name	= "bcm63xx_uart",
+#ifdef CONFIG_SERIAL_BCM63XX_TTYS
 	.dev_name	= "ttyS",
 	.major		= TTY_MAJOR,
 	.minor		= 64,
+#else
+	.dev_name	= "ttyBCM",
+#endif
 	.nr		= BCM63XX_NR_UARTS,
 	.cons		= BCM63XX_CONSOLE,
 };
-- 
2.1.1

Re: [PATCH 2/2] tty: serial: bcm63xx: Allow device nodes to be renamed to /dev/ttyBCM*

[Rob Herring]

On Sun, Nov 9, 2014 at 2:55 AM, Kevin Cernekee <cernekee@gmail.com> wrote:
> By default, bcm63xx_uart.c uses the standard 8250 device naming and
> major/minor numbers.  There are at least two situations where this could
> be a problem:
>
> 1) Multiplatform kernels that need to support some chips that have 8250
> UARTs and other chips that have bcm63xx UARTs.
>
> 2) Some older chips like BCM7125 have a mix of both UART types.
>
> Add a new Kconfig option to tell the driver whether to register itself
> as ttyS or ttyBCM.  By default it will retain the existing "ttyS"
> behavior to avoid surprises.

While I understand the desire to have stable names, this is the
opposite direction we want to go. Per platform tty names complicates
having a generic userspace. It is not so bad since most ARM platforms
use ttyS or ttyAMA, but just think what the kernel and userspace side
would look like if every single platform did this. We can't change
everything to ttyS because the other names are already an ABI.

This can be solved with a udev rule to create sym links. Or if you
just need to know which dev node is h/w uart X, you can get that from
sysfs.

Rob

> Signed-off-by: Kevin Cernekee <cernekee@gmail.com>
> ---
>  drivers/tty/serial/Kconfig        | 11 +++++++++++
>  drivers/tty/serial/bcm63xx_uart.c |  8 ++++++++
>  2 files changed, 19 insertions(+)
>
> diff --git a/drivers/tty/serial/Kconfig b/drivers/tty/serial/Kconfig
> index fdd851e..c82cfd2 100644
> --- a/drivers/tty/serial/Kconfig
> +++ b/drivers/tty/serial/Kconfig
> @@ -1302,6 +1302,17 @@ config SERIAL_BCM63XX_CONSOLE
>           If you have enabled the serial port on the BCM63xx CPU
>           you can make it the console by answering Y to this option.
>
> +config SERIAL_BCM63XX_TTYS
> +       bool "Use ttyS[01] device nodes for bcm63xx_uart"
> +       depends on SERIAL_BCM63XX
> +       default y
> +       help
> +         Say Y to name the serial ports /dev/ttyS0, ttyS1, ...
> +         This conflicts with the 8250 driver but may avoid breaking
> +         compatibility with existing init scripts.
> +
> +         Say N to name the serial ports /dev/ttyBCM0, ttyBCM1, ...
> +
>  config SERIAL_GRLIB_GAISLER_APBUART
>         tristate "GRLIB APBUART serial support"
>         depends on OF && SPARC
> diff --git a/drivers/tty/serial/bcm63xx_uart.c b/drivers/tty/serial/bcm63xx_uart.c
> index e04e580..9f3dcc8 100644
> --- a/drivers/tty/serial/bcm63xx_uart.c
> +++ b/drivers/tty/serial/bcm63xx_uart.c
> @@ -751,7 +751,11 @@ static int bcm_console_setup(struct console *co, char *options)
>  static struct uart_driver bcm_uart_driver;
>
>  static struct console bcm63xx_console = {
> +#ifdef CONFIG_SERIAL_BCM63XX_TTYS
>         .name           = "ttyS",
> +#else
> +       .name           = "ttyBCM",
> +#endif
>         .write          = bcm_console_write,
>         .device         = uart_console_device,
>         .setup          = bcm_console_setup,
> @@ -796,9 +800,13 @@ OF_EARLYCON_DECLARE(bcm63xx_uart, "brcm,bcm6345-uart", bcm_early_console_setup);
>  static struct uart_driver bcm_uart_driver = {
>         .owner          = THIS_MODULE,
>         .driver_name    = "bcm63xx_uart",
> +#ifdef CONFIG_SERIAL_BCM63XX_TTYS
>         .dev_name       = "ttyS",
>         .major          = TTY_MAJOR,
>         .minor          = 64,
> +#else
> +       .dev_name       = "ttyBCM",
> +#endif
>         .nr             = BCM63XX_NR_UARTS,
>         .cons           = BCM63XX_CONSOLE,
>  };
> --
> 2.1.1
>

Re: [PATCH 2/2] tty: serial: bcm63xx: Allow device nodes to be renamed to /dev/ttyBCM*

[Kevin Cernekee]

On Mon, Nov 10, 2014 at 6:25 AM, Rob Herring <robh@kernel.org> wrote:
> On Sun, Nov 9, 2014 at 2:55 AM, Kevin Cernekee <cernekee@gmail.com> wrote:
>> By default, bcm63xx_uart.c uses the standard 8250 device naming and
>> major/minor numbers.  There are at least two situations where this could
>> be a problem:
>>
>> 1) Multiplatform kernels that need to support some chips that have 8250
>> UARTs and other chips that have bcm63xx UARTs.
>>
>> 2) Some older chips like BCM7125 have a mix of both UART types.
>>
>> Add a new Kconfig option to tell the driver whether to register itself
>> as ttyS or ttyBCM.  By default it will retain the existing "ttyS"
>> behavior to avoid surprises.
>
> While I understand the desire to have stable names, this is the
> opposite direction we want to go. Per platform tty names complicates
> having a generic userspace. It is not so bad since most ARM platforms
> use ttyS or ttyAMA, but just think what the kernel and userspace side
> would look like if every single platform did this. We can't change
> everything to ttyS because the other names are already an ABI.
>
> This can be solved with a udev rule to create sym links.

Is it safe to register two console drivers named "ttyS" with the same
major/minor numbers?  Maybe there is a trick to making them coexist?

What I found is that everything worked fine on a system with
bcm63xx_uart hardware until I enabled the 8250 driver in .config.  At
that point the drivers clashed and I had no serial output
post-bootconsole.  It didn't even get to userland before it failed.
There are no DT entries mentioning 8250; the mere presence of the 8250
driver killed my console.

If this behavior is unexpected I can keep digging to find out what went wrong.

> Or if you
> just need to know which dev node is h/w uart X, you can get that from
> sysfs.

Right - I use busybox cttyhack in the init scripts to figure out the
console name, so the same rootfs image can be used for both ttyS0 and
ttyBCM0:

# Put a shell on the serial port
::respawn:/bin/cttyhack /bin/sh -l

Re: [PATCH 2/2] tty: serial: bcm63xx: Allow device nodes to be renamed to /dev/ttyBCM*

[Greg Kroah-Hartman]

On Mon, Nov 10, 2014 at 07:05:14AM -0800, Kevin Cernekee wrote:
> On Mon, Nov 10, 2014 at 6:25 AM, Rob Herring <robh@kernel.org> wrote:
> > On Sun, Nov 9, 2014 at 2:55 AM, Kevin Cernekee <cernekee@gmail.com> wrote:
> >> By default, bcm63xx_uart.c uses the standard 8250 device naming and
> >> major/minor numbers.  There are at least two situations where this could
> >> be a problem:
> >>
> >> 1) Multiplatform kernels that need to support some chips that have 8250
> >> UARTs and other chips that have bcm63xx UARTs.
> >>
> >> 2) Some older chips like BCM7125 have a mix of both UART types.
> >>
> >> Add a new Kconfig option to tell the driver whether to register itself
> >> as ttyS or ttyBCM.  By default it will retain the existing "ttyS"
> >> behavior to avoid surprises.
> >
> > While I understand the desire to have stable names, this is the
> > opposite direction we want to go. Per platform tty names complicates
> > having a generic userspace. It is not so bad since most ARM platforms
> > use ttyS or ttyAMA, but just think what the kernel and userspace side
> > would look like if every single platform did this. We can't change
> > everything to ttyS because the other names are already an ABI.
> >
> > This can be solved with a udev rule to create sym links.
> 
> Is it safe to register two console drivers named "ttyS" with the same
> major/minor numbers?

Not at all, think about what you are asking for here.

Is the kernel allowed to register two block devices with the same
major/minor numbers?

greg k-h

Re: [PATCH 2/2] tty: serial: bcm63xx: Allow device nodes to be renamed to /dev/ttyBCM*

[Kevin Cernekee]

On Mon, Nov 10, 2014 at 10:30 AM, Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
> On Mon, Nov 10, 2014 at 07:05:14AM -0800, Kevin Cernekee wrote:
>> On Mon, Nov 10, 2014 at 6:25 AM, Rob Herring <robh@kernel.org> wrote:
>> > On Sun, Nov 9, 2014 at 2:55 AM, Kevin Cernekee <cernekee@gmail.com> wrote:
>> >> By default, bcm63xx_uart.c uses the standard 8250 device naming and
>> >> major/minor numbers.  There are at least two situations where this could
>> >> be a problem:
>> >>
>> >> 1) Multiplatform kernels that need to support some chips that have 8250
>> >> UARTs and other chips that have bcm63xx UARTs.
>> >>
>> >> 2) Some older chips like BCM7125 have a mix of both UART types.
>> >>
>> >> Add a new Kconfig option to tell the driver whether to register itself
>> >> as ttyS or ttyBCM.  By default it will retain the existing "ttyS"
>> >> behavior to avoid surprises.
>> >
>> > While I understand the desire to have stable names, this is the
>> > opposite direction we want to go. Per platform tty names complicates
>> > having a generic userspace. It is not so bad since most ARM platforms
>> > use ttyS or ttyAMA, but just think what the kernel and userspace side
>> > would look like if every single platform did this. We can't change
>> > everything to ttyS because the other names are already an ABI.
>> >
>> > This can be solved with a udev rule to create sym links.
>>
>> Is it safe to register two console drivers named "ttyS" with the same
>> major/minor numbers?
>
> Not at all, think about what you are asking for here.
>
> Is the kernel allowed to register two block devices with the same
> major/minor numbers?

So, is there a recommended solution for building a multiplatform
kernel that includes more than one serial driver that wants to claim
major 4, minor 64?  Let's start with the easy case (#1 from above):
only one of the drivers actually has a DT entry and gets probed.

Browsing around drivers/tty/serial I identified several other drivers
that use 4/64: apbuart, dz, ip22zilog, m32r_sio, mcf, mrst_max3110,
pxa, sunhv, zs.  Unfortunately none of these seem to be built as part
of ARM's MULTI_V7 kernel.

My other idea was to keep using "ttyS" for bcm63xx_uart, but remove
the major/minor numbers from the driver code so the kernel
auto-assigns them.  In this case I also need to set
CONFIG_SERIAL_8250_RUNTIME_UARTS=0 to prevent the 8250 driver from
creating ttyS[0-3] on its own, and then I'll need to retest the 8250
based platform to make sure it didn't break.  Does that sound
plausible?

Re: [PATCH 2/2] tty: serial: bcm63xx: Allow device nodes to be renamed to /dev/ttyBCM*

[Rob Herring]

On Mon, Nov 10, 2014 at 9:05 AM, Kevin Cernekee <cernekee@gmail.com> wrote:
> On Mon, Nov 10, 2014 at 6:25 AM, Rob Herring <robh@kernel.org> wrote:
>> On Sun, Nov 9, 2014 at 2:55 AM, Kevin Cernekee <cernekee@gmail.com> wrote:
>>> By default, bcm63xx_uart.c uses the standard 8250 device naming and
>>> major/minor numbers.  There are at least two situations where this could
>>> be a problem:
>>>
>>> 1) Multiplatform kernels that need to support some chips that have 8250
>>> UARTs and other chips that have bcm63xx UARTs.
>>>
>>> 2) Some older chips like BCM7125 have a mix of both UART types.
>>>
>>> Add a new Kconfig option to tell the driver whether to register itself
>>> as ttyS or ttyBCM.  By default it will retain the existing "ttyS"
>>> behavior to avoid surprises.
>>
>> While I understand the desire to have stable names, this is the
>> opposite direction we want to go. Per platform tty names complicates
>> having a generic userspace. It is not so bad since most ARM platforms
>> use ttyS or ttyAMA, but just think what the kernel and userspace side
>> would look like if every single platform did this. We can't change
>> everything to ttyS because the other names are already an ABI.
>>
>> This can be solved with a udev rule to create sym links.
>
> Is it safe to register two console drivers named "ttyS" with the same
> major/minor numbers?  Maybe there is a trick to making them coexist?

No, but I think you can do dynamic minor numbers. I seem to recall
this coming up with the Samsung UARTs a while back.

Rob

> What I found is that everything worked fine on a system with
> bcm63xx_uart hardware until I enabled the 8250 driver in .config.  At
> that point the drivers clashed and I had no serial output
> post-bootconsole.  It didn't even get to userland before it failed.
> There are no DT entries mentioning 8250; the mere presence of the 8250
> driver killed my console.
>
> If this behavior is unexpected I can keep digging to find out what went wrong.
>
>> Or if you
>> just need to know which dev node is h/w uart X, you can get that from
>> sysfs.
>
> Right - I use busybox cttyhack in the init scripts to figure out the
> console name, so the same rootfs image can be used for both ttyS0 and
> ttyBCM0:
>
> # Put a shell on the serial port
> ::respawn:/bin/cttyhack /bin/sh -l

Re: [PATCH 2/2] tty: serial: bcm63xx: Allow device nodes to be renamed to /dev/ttyBCM*

[Kevin Cernekee]

On Mon, Nov 10, 2014 at 11:22 AM, Rob Herring <robh@kernel.org> wrote:
>>> This can be solved with a udev rule to create sym links.
>>
>> Is it safe to register two console drivers named "ttyS" with the same
>> major/minor numbers?  Maybe there is a trick to making them coexist?
>
> No, but I think you can do dynamic minor numbers. I seem to recall
> this coming up with the Samsung UARTs a while back.

The other variations I've seen in the tree are:

nwpserial: ttySQ, major 4 minor 68 (not 64)

sunhv, sunsab, sunsu, sunzilog: set uart_driver->major to 4 but let
uart_driver->minor default to 0

SERIAL_ATMEL_TTYAT: compile-time selectable between ttySn (4/64) and
ttyATn (204/154).  txx9 does something similar using
SERIAL_TXX9_STDSERIAL.

A whole bunch of other SoC serial drivers use major 204 and a custom
name like "ttyAL".  Some of these show up in
Documentation/devices.txt; others don't.  ~3 drivers use 204/64 from
the middle of the Altix assigned range.

What is the current best practice for new drivers?

Re: [PATCH 2/2] tty: serial: bcm63xx: Allow device nodes to be renamed to /dev/ttyBCM*

[Rob Herring]

On Mon, Nov 10, 2014 at 1:50 PM, Kevin Cernekee <cernekee@gmail.com> wrote:
> On Mon, Nov 10, 2014 at 11:22 AM, Rob Herring <robh@kernel.org> wrote:
>>>> This can be solved with a udev rule to create sym links.
>>>
>>> Is it safe to register two console drivers named "ttyS" with the same
>>> major/minor numbers?  Maybe there is a trick to making them coexist?
>>
>> No, but I think you can do dynamic minor numbers. I seem to recall
>> this coming up with the Samsung UARTs a while back.
>
> The other variations I've seen in the tree are:
>
> nwpserial: ttySQ, major 4 minor 68 (not 64)
>
> sunhv, sunsab, sunsu, sunzilog: set uart_driver->major to 4 but let
> uart_driver->minor default to 0
>
> SERIAL_ATMEL_TTYAT: compile-time selectable between ttySn (4/64) and
> ttyATn (204/154).  txx9 does something similar using
> SERIAL_TXX9_STDSERIAL.
>
> A whole bunch of other SoC serial drivers use major 204 and a custom
> name like "ttyAL".  Some of these show up in
> Documentation/devices.txt; others don't.  ~3 drivers use 204/64 from
> the middle of the Altix assigned range.
>
> What is the current best practice for new drivers?

I think it would be using dynamic numbering, but would be good to have
others weigh in here. It looks like a dynamic major would solve your
problem. See tty_register_driver. Also, there was a patch to make this
the fallback behavior instead of an error[1], but it was never merged
(and it's not clear why). This was the Samsung related change I was
remembering.

Rob

[1] http://lists.linaro.org/pipermail/linaro-kernel/2014-January/010383.html

Re: [PATCH 2/2] tty: serial: bcm63xx: Allow device nodes to be renamed to /dev/ttyBCM*

[Kevin Cernekee]

On Tue, Nov 11, 2014 at 9:35 AM, Rob Herring <robh@kernel.org> wrote:
>> What is the current best practice for new drivers?
>
> I think it would be using dynamic numbering, but would be good to have
> others weigh in here. It looks like a dynamic major would solve your
> problem. See tty_register_driver. Also, there was a patch to make this
> the fallback behavior instead of an error[1], but it was never merged
> (and it's not clear why). This was the Samsung related change I was
> remembering.
>
> Rob
>
> [1] http://lists.linaro.org/pipermail/linaro-kernel/2014-January/010383.html

This doesn't seem to fix the coexistence problems with the 8250
driver.  Perhaps it is time to write a simpler version of the 8250
driver geared toward embedded applications?  Some items on my wishlist
include:

 - OF_EARLYCON support
 - Native-endian and big-endian register support
 - Move to a pure runtime-enumerated model and drop the ISA code
 - Drop support for obsolete chips (16450, 8250) and quirks for obsolete PCs

It looks like sunsu, serial-tegra, omap-serial, and possibly others
have already started down this path.  Or I could just write a
dedicated uart_bcm7xxx driver if that's easier...

Here are my results after grabbing V2 of Tushar's patch from the list
archives and reverting the bcm63xx_uart driver to claim "ttyS" with
major 4, minor 64:

 - Building with CONFIG_SERIAL_8250_NR_UARTS=0 rendered the 8250-based
system unbootable (it dies with no output right after the bootconsole
is disabled)

 - Building with CONFIG_SERIAL_8250_NR_UARTS=1 rendered the
bcm63xx_uart-based system unbootable, with the following output:

Serial: 8250/16550 driver, 1 ports, IRQ sharing disabled
Default device node (4:64) for ttyS is busy, using dynamic major number
bcm63xx_uart 10000100.serial: ttyS0 at MMIO 0x10000100 (irq = 28,
base_baud = 1562500) is a bcm63xx_uart
console [ttyS0] enabled
console [ttyS0] enabled
bootconsole [uart0] disabled
bootconsole [uart0] disabled
------------[ cut here ]------------
WARNING: CPU: 0 PID: 1 at fs/sysfs/dir.c:31 sysfs_warn_dup+0x64/0xbc()
sysfs: cannot create duplicate filename '/class/tty/ttyS0'
CPU: 0 PID: 1 Comm: swapper/0 Not tainted 3.18.0-rc4+ #384
Stack : 00000000 00000004 00000000 80075444 00000000 00000000 00000000 00000000
      809e5012 0000003a 00000001 00000000 00010000 87c282b8 8056dac4 805b9ce7
      00000001 00000000 809e39d8 87c282b8 87c6ae10 00000001 87c6ae10 804edeb0
      805c0000 80032984 00000000 00000000 80571d28 87c2da84 87c2da84 8056dac4
      00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
      ...
Call Trace:
[<8001a00c>] show_stack+0x64/0x7c
[<804eff94>] dump_stack+0xc8/0xfc
[<80032bd8>] warn_slowpath_common+0x7c/0xac
[<80032c68>] warn_slowpath_fmt+0x2c/0x38
[<8015c784>] sysfs_warn_dup+0x64/0xbc
[<8015cabc>] sysfs_do_create_link_sd.isra.2+0xec/0xf4
[<802c193c>] device_add+0x4b0/0x59c
[<802a452c>] tty_register_device_attr+0xe4/0x278
[<802b625c>] uart_add_one_port+0x344/0x474
[<802bbffc>] bcm_uart_probe+0x14c/0x240
[<802c6c20>] platform_drv_probe+0x54/0xc4
[<802c51bc>] really_probe+0xa0/0x2cc
[<802c5544>] __driver_attach+0xd0/0xd8
[<802c3660>] bus_for_each_dev+0x68/0xb0
[<802c4160>] bus_add_driver+0x168/0x230
[<802c5f6c>] driver_register+0x84/0x138
[<805e86ec>] bcm_uart_init+0x30/0x58
[<805d4ce0>] do_one_initcall+0x148/0x208
[<805d4f0c>] kernel_init_freeable+0x16c/0x228
[<804eb6dc>] kernel_init+0x10/0x100
[<80013ce8>] ret_from_kernel_thread+0x14/0x1c

---[ end trace dc92a8c272541296 ]---
bcm63xx_uart 10000100.serial: Cannot register tty device on line 0
libphy: Fixed MDIO Bus: probed
usbcore: registered new interface driver asix
usbcore: registered new interface driver ax88179_178a
usbcore: registered new interface driver cdc_ether
usbcore: registered new interface driver net1080
usbcore: registered new interface driver cdc_subset
usbcore: registered new interface driver zaurus
usbcore: registered new interface driver cdc_ncm
ehci_hcd: USB 2.0 'Enhanced' Host Controller (EHCI) Driver
ehci-platform: EHCI generic platform driver
ohci_hcd: USB 1.1 'Open' Host Controller (OHCI) Driver
ohci-platform: OHCI generic platform driver
usbcore: registered new interface driver usb-storage
TCP: cubic registered
NET: Registered protocol family 10
sit: IPv6 over IPv4 tunneling driver
NET: Registered protocol family 17
Freeing unused kernel memory: 4144K (805d4000 - 809e0000)
starting pid 797, tty '': '/etc/init.d/rcS'
Mounting virtual filesystems
Starting mdev
Configuring lo interface
Configuring sit0 interface
Starting network services
starting pid 826, tty '': '/bin/cttyhack /bin/sh -l'
process '/bin/cttyhack /bin/sh -l' (pid 826) exited. Scheduling for restart.
starting pid 828, tty '': '/bin/cttyhack /bin/sh -l'
process '/bin/cttyhack /bin/sh -l' (pid 828) exited. Scheduling for restart.
starting pid 830, tty '': '/bin/cttyhack /bin/sh -l'

Re: [PATCH 1/2] of: Fix crash if an earlycon driver is not found

[Rob Herring]

On Sun, Nov 9, 2014 at 2:55 AM, Kevin Cernekee <cernekee@gmail.com> wrote:
> __earlycon_of_table_sentinel.compatible is a char[128], not a pointer, so
> it will never be NULL.  Checking it against NULL causes the match loop to
> run past the end of the array, and eventually match a bogus entry, under
> the following conditions:
>
>  - Kernel command line specifies "earlycon" with no parameters
>  - DT has a stdout-path pointing to a UART node
>  - The UART driver doesn't use OF_EARLYCON_DECLARE (or maybe the console
>    driver is compiled out)
>
> Fix this by checking to see if match->compatible is a non-empty string.
>
> Signed-off-by: Kevin Cernekee <cernekee@gmail.com>
> Cc: <stable@vger.kernel.org> # 3.16+

Thanks. I'll queue this up.

BTW, you should not add stable CC when submitting for review, but
rather add a note for the maintainer to apply to stable. Only if a
commit is in mainline already and not flagged for stable, then you
send the patch with the stable tag to get the commit added to stable.
It's a bit confusing...

Rob

> ---
>  drivers/of/fdt.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
> index d1ffca8..30e97bc 100644
> --- a/drivers/of/fdt.c
> +++ b/drivers/of/fdt.c
> @@ -773,7 +773,7 @@ int __init early_init_dt_scan_chosen_serial(void)
>         if (offset < 0)
>                 return -ENODEV;
>
> -       while (match->compatible) {
> +       while (match->compatible[0]) {
>                 unsigned long addr;
>                 if (fdt_node_check_compatible(fdt, offset, match->compatible)) {
>                         match++;
> --
> 2.1.1
>

Re: [PATCH 1/2] of: Fix crash if an earlycon driver is not found

[Sergei Shtylyov]

On 11/10/2014 05:14 PM, Rob Herring wrote:

>> __earlycon_of_table_sentinel.compatible is a char[128], not a pointer, so
>> it will never be NULL.  Checking it against NULL causes the match loop to
>> run past the end of the array, and eventually match a bogus entry, under
>> the following conditions:

>>   - Kernel command line specifies "earlycon" with no parameters
>>   - DT has a stdout-path pointing to a UART node
>>   - The UART driver doesn't use OF_EARLYCON_DECLARE (or maybe the console
>>     driver is compiled out)

>> Fix this by checking to see if match->compatible is a non-empty string.

>> Signed-off-by: Kevin Cernekee <cernekee@gmail.com>
>> Cc: <stable@vger.kernel.org> # 3.16+

> Thanks. I'll queue this up.

> BTW, you should not add stable CC when submitting for review, but
> rather add a note for the maintainer to apply to stable. Only if a
> commit is in mainline already and not flagged for stable, then you
> send the patch with the stable tag to get the commit added to stable.
> It's a bit confusing...

    It's actually OK to tag the patch for stable (not really send to stable), 
so that that list gets automatically notified when the comment lands in the 
mainline. Unless the maintainer doesn't have his own rules about stable 
patches (like e.g. DaveM). Do you have alike rules?

> Rob

WBR, Sergei

Re: [PATCH 1/2] of: Fix crash if an earlycon driver is not found

[Grant Likely]

On Mon, 10 Nov 2014 08:14:01 -0600
, Rob Herring <robh@kernel.org>
 wrote:
> On Sun, Nov 9, 2014 at 2:55 AM, Kevin Cernekee <cernekee@gmail.com> wrote:
> > __earlycon_of_table_sentinel.compatible is a char[128], not a pointer, so
> > it will never be NULL.  Checking it against NULL causes the match loop to
> > run past the end of the array, and eventually match a bogus entry, under
> > the following conditions:
> >
> >  - Kernel command line specifies "earlycon" with no parameters
> >  - DT has a stdout-path pointing to a UART node
> >  - The UART driver doesn't use OF_EARLYCON_DECLARE (or maybe the console
> >    driver is compiled out)
> >
> > Fix this by checking to see if match->compatible is a non-empty string.
> >
> > Signed-off-by: Kevin Cernekee <cernekee@gmail.com>
> > Cc: <stable@vger.kernel.org> # 3.16+
> 
> Thanks. I'll queue this up.
> 
> BTW, you should not add stable CC when submitting for review, but
> rather add a note for the maintainer to apply to stable. Only if a
> commit is in mainline already and not flagged for stable, then you
> send the patch with the stable tag to get the commit added to stable.
> It's a bit confusing...


Oops, since you've picked it up I'll drop it from my tree. I'll let you
send the pull req to Linus.

g.

Re: [PATCH 1/2] of: Fix crash if an earlycon driver is not found

[Grant Likely]

On Sun,  9 Nov 2014 00:55:47 -0800
, Kevin Cernekee <cernekee@gmail.com>
 wrote:
> __earlycon_of_table_sentinel.compatible is a char[128], not a pointer, so
> it will never be NULL.  Checking it against NULL causes the match loop to
> run past the end of the array, and eventually match a bogus entry, under
> the following conditions:
> 
>  - Kernel command line specifies "earlycon" with no parameters
>  - DT has a stdout-path pointing to a UART node
>  - The UART driver doesn't use OF_EARLYCON_DECLARE (or maybe the console
>    driver is compiled out)
> 
> Fix this by checking to see if match->compatible is a non-empty string.
> 
> Signed-off-by: Kevin Cernekee <cernekee@gmail.com>
> Cc: <stable@vger.kernel.org> # 3.16+

Applied, thanks. Although a more robust fix would probably to be
checking for the end address of the section. This is a safe fix though,
so I'm picking it up.

g.

> ---
>  drivers/of/fdt.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
> index d1ffca8..30e97bc 100644
> --- a/drivers/of/fdt.c
> +++ b/drivers/of/fdt.c
> @@ -773,7 +773,7 @@ int __init early_init_dt_scan_chosen_serial(void)
>  	if (offset < 0)
>  		return -ENODEV;
>  
> -	while (match->compatible) {
> +	while (match->compatible[0]) {
>  		unsigned long addr;
>  		if (fdt_node_check_compatible(fdt, offset, match->compatible)) {
>  			match++;
> -- 
> 2.1.1
>