racy calls to platform_device_add_data after platform_device_register_simple

Milton Miller miltonm at bga.com
Tue Jun 12 01:27:21 EST 2007


On Fri Jun 8 18:54:38 EST 2007, Mark Zhan wrote:
> @@ -775,38 +774,42 @@ static int __init cpm_uart_of_init(void)
> +               if (strstr(model, "SMC")) {
> +                       cpm_uart_dev = 
> platform_device_register_simple("fsl-cpm-smc:uart",
> +                                                       i, &r[0], 3);
> +               } else if (strstr(model, "SCC")) {
> +                       cpm_uart_dev = 
> platform_device_register_simple("fsl-cpm-scc:uart",
> +                                                       i, &r[0], 3);
> +               }
> +
> +               if (IS_ERR(cpm_uart_dev)) {
> +                       ret = PTR_ERR(cpm_uart_dev);
> +                       goto err;
> +               }
>
> +               cpm_uart_data.uart_clk = ppc_proc_freq;
>                 cpm_uart_data.tx_num_fifo = 4;
>                 cpm_uart_data.tx_buf_size = 32;
>                 cpm_uart_data.rx_num_fifo = 4;
> @@ -816,10 +819,8 @@ static int __init cpm_uart_of_init(void)
>                 cpm_uart_data.clk_tx = *((u32 *)of_get_property(np,
>                                                 "tx-clock", NULL));
>
> -               ret =
> -                   platform_device_add_data(cpm_uart_dev, 
> &cpm_uart_data,
> -                                            sizeof(struct
> -                                                   
> fs_uart_platform_info));
> +               ret = platform_device_add_data(cpm_uart_dev, 
> &cpm_uart_data,
> +                                       sizeof(struct 
> fs_uart_platform_info));

This code (and others like the ethernet code above it) is racy in that 
the device is visible to the device layer without all of the data 
needed by the driver.  Since its being run as an arch_initcall, the 
driver is not yet registered so its not quite a bug yet.

Since this seems to be appearing frequently as we add wrappers from the 
device tree to the subsystem layer drivers, we should create an 
extended function that takes a data pointer and size (NULL/zero would 
mean none), and define platform_device_register_simple in terms of it.

milton




More information about the Linuxppc-dev mailing list