Re: [PATCH 10/11] spi/s3c64xx: improve error handling
On Friday 10 August 2012, Kukjin Kim wrote: > BTW for same reason, probably, we need following fix? > > arch/arm/mach-tegra/tegra2_emc.c |4 ++-- > arch/c6x/kernel/setup.c |2 +- > arch/powerpc/kernel/ibmebus.c|2 +- > arch/powerpc/kernel/pci_of_scan.c|2 +- > arch/powerpc/kernel/prom.c |2 +- > arch/powerpc/kernel/rtas_pci.c |2 +- > arch/powerpc/kernel/vio.c|2 +- > arch/powerpc/platforms/44x/warp.c|2 +- > ... Actually not. The difference is that only s3c64xx_get_slave_ctrldata accesses the node pointer outside of the look (after break). This fails when there are no child nodes at all. In the other cases, the only use of the node pointer is inside the loop, where it is guaranteed to be valid. Arnd -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 10/11] spi/s3c64xx: improve error handling
On 8 August 2012 20:17, Arnd Bergmann wrote:
> When a device tree definition os an s3c64xx SPI master is missing
> a "controller-data" subnode, the newly added s3c64xx_get_slave_ctrldata
> function might use uninitialized memory in place of that node,
> which was correctly reported by gcc.
>
> Without this patch, building s3c6400_defconfig results in:
>
> drivers/spi/spi-s3c64xx.c: In function 's3c64xx_get_slave_ctrldata.isra.25':
> drivers/spi/spi-s3c64xx.c:841:5: warning: 'data_np' may be used uninitialized
> in this function [-Wuninitialized]
>
> Signed-off-by: Arnd Bergmann
> Cc: Thomas Abraham
> Cc: Jaswinder Singh
> Cc: Grant Likely
> Cc: Kukjin Kim
> ---
> drivers/spi/spi-s3c64xx.c |2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/spi/spi-s3c64xx.c b/drivers/spi/spi-s3c64xx.c
> index 646a765..cfa2c35 100644
> --- a/drivers/spi/spi-s3c64xx.c
> +++ b/drivers/spi/spi-s3c64xx.c
> @@ -826,7 +826,7 @@ static struct s3c64xx_spi_csinfo
> *s3c64xx_get_slave_ctrldata(
> struct spi_device *spi)
> {
> struct s3c64xx_spi_csinfo *cs;
> - struct device_node *slave_np, *data_np;
> + struct device_node *slave_np, *data_np = NULL;
> u32 fb_delay = 0;
>
> slave_np = spi->dev.of_node;
> --
> 1.7.10
>
Acked-by: Thomas Abraham
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH 10/11] spi/s3c64xx: improve error handling
Arnd Bergmann wrote:
>
> When a device tree definition os an s3c64xx SPI master is missing
> a "controller-data" subnode, the newly added s3c64xx_get_slave_ctrldata
> function might use uninitialized memory in place of that node,
> which was correctly reported by gcc.
>
> Without this patch, building s3c6400_defconfig results in:
>
> drivers/spi/spi-s3c64xx.c: In function
> 's3c64xx_get_slave_ctrldata.isra.25':
> drivers/spi/spi-s3c64xx.c:841:5: warning: 'data_np' may be used
> uninitialized in this function [-Wuninitialized]
>
> Signed-off-by: Arnd Bergmann
> Cc: Thomas Abraham
> Cc: Jaswinder Singh
> Cc: Grant Likely
> Cc: Kukjin Kim
> ---
> drivers/spi/spi-s3c64xx.c |2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/spi/spi-s3c64xx.c b/drivers/spi/spi-s3c64xx.c
> index 646a765..cfa2c35 100644
> --- a/drivers/spi/spi-s3c64xx.c
> +++ b/drivers/spi/spi-s3c64xx.c
> @@ -826,7 +826,7 @@ static struct s3c64xx_spi_csinfo
> *s3c64xx_get_slave_ctrldata(
> struct spi_device *spi)
> {
> struct s3c64xx_spi_csinfo *cs;
> - struct device_node *slave_np, *data_np;
> + struct device_node *slave_np, *data_np = NULL;
> u32 fb_delay = 0;
>
> slave_np = spi->dev.of_node;
> --
> 1.7.10
Looks ok to me,
Acked-by: Kukjin Kim
BTW for same reason, probably, we need following fix?
Thanks.
Best regards,
Kgene.
--
Kukjin Kim , Senior Engineer,
SW Solution Development Team, Samsung Electronics Co., Ltd.
From: Kukjin Kim
Signed-off-by: Kukjin Kim
---
arch/arm/mach-tegra/tegra2_emc.c |4 ++--
arch/c6x/kernel/setup.c |2 +-
arch/powerpc/kernel/ibmebus.c|2 +-
arch/powerpc/kernel/pci_of_scan.c|2 +-
arch/powerpc/kernel/prom.c |2 +-
arch/powerpc/kernel/rtas_pci.c |2 +-
arch/powerpc/kernel/vio.c|2 +-
arch/powerpc/platforms/44x/warp.c|2 +-
arch/powerpc/platforms/cell/setup.c |2 +-
arch/powerpc/platforms/powernv/opal.c|2 +-
arch/powerpc/platforms/powernv/pci-ioda.c|2 +-
arch/powerpc/platforms/powernv/pci-p5ioc2.c |2 +-
arch/powerpc/platforms/pseries/eeh.c | 12 ++--
arch/powerpc/sysdev/mv64x60_dev.c|2 +-
drivers/dma/fsldma.c |2 +-
drivers/gpu/drm/nouveau/nouveau_connector.c |2 +-
drivers/hwmon/ads1015.c |2 +-
drivers/i2c/busses/i2c-powermac.c|2 +-
drivers/i2c/busses/i2c-pxa-pci.c |2 +-
drivers/i2c/i2c-mux.c|2 +-
drivers/iio/adc/at91_adc.c |2 +-
drivers/input/keyboard/samsung-keypad.c |2 +-
drivers/leds/leds-gpio.c |2 +-
drivers/net/ethernet/freescale/fsl_pq_mdio.c |2 +-
drivers/net/phy/mdio-mux.c |2 +-
drivers/of/of_i2c.c |2 +-
drivers/of/of_mdio.c |2 +-
drivers/of/of_pci.c |2 +-
drivers/of/platform.c|6 +++---
drivers/pinctrl/pinctrl-imx.c|4 ++--
drivers/pinctrl/pinctrl-tegra.c |2 +-
drivers/pinctrl/spear/pinctrl-spear.c|2 +-
drivers/regulator/da9052-regulator.c |2 +-
drivers/regulator/mc13xxx-regulator-core.c |2 +-
drivers/regulator/of_regulator.c |2 +-
drivers/spi/spi.c|2 +-
drivers/tty/hvc/hvc_opal.c |2 +-
37 files changed, 46 insertions(+), 46 deletions(-)
diff --git a/arch/arm/mach-tegra/tegra2_emc.c
b/arch/arm/mach-tegra/tegra2_emc.c
index 5070d83..d045a9e 100644
--- a/arch/arm/mach-tegra/tegra2_emc.c
+++ b/arch/arm/mach-tegra/tegra2_emc.c
@@ -181,7 +181,7 @@ int tegra_emc_set_rate(unsigned long rate)
#ifdef CONFIG_OF
static struct device_node *tegra_emc_ramcode_devnode(struct device_node
*np)
{
- struct device_node *iter;
+ struct device_node *iter = NULL;
u32 reg;
for_each_child_of_node(np, iter) {
@@ -198,7 +198,7 @@ static struct tegra_emc_pdata *tegra_emc_dt_parse_pdata(
struct platform_device *pdev)
{
struct device_node *np = pdev->dev.of_node;
- struct device_node *tnp, *iter;
+ struct device_node *tnp, *iter = NULL;
struct tegra_emc_pdata *pdata;
int ret, i, num_tables;
diff --git a/arch/c6x/kernel/setup.c b/arch/c6x/kernel/setup.c
index f4e72bd..0f4cd29 100644
--- a/arch/c6x/kernel/setup.c
+++ b/arch/c6x/kernel/setup.c
@@ -99,7 +99,7 @@ static void __init get_cpuinfo(void)
unsigned long core_khz;
u64 tmp;
struct cpuinfo_c6x *p;
- struct device_node *node, *np;
+ struct device_node *node, *np = NULL;
p = _cpu(cpu_data, smp_processor_id());
diff --git
RE: [PATCH 10/11] spi/s3c64xx: improve error handling
Arnd Bergmann wrote:
When a device tree definition os an s3c64xx SPI master is missing
a controller-data subnode, the newly added s3c64xx_get_slave_ctrldata
function might use uninitialized memory in place of that node,
which was correctly reported by gcc.
Without this patch, building s3c6400_defconfig results in:
drivers/spi/spi-s3c64xx.c: In function
's3c64xx_get_slave_ctrldata.isra.25':
drivers/spi/spi-s3c64xx.c:841:5: warning: 'data_np' may be used
uninitialized in this function [-Wuninitialized]
Signed-off-by: Arnd Bergmann a...@arndb.de
Cc: Thomas Abraham thomas.abra...@linaro.org
Cc: Jaswinder Singh jaswinder.si...@linaro.org
Cc: Grant Likely grant.lik...@secretlab.ca
Cc: Kukjin Kim kgene@samsung.com
---
drivers/spi/spi-s3c64xx.c |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/spi/spi-s3c64xx.c b/drivers/spi/spi-s3c64xx.c
index 646a765..cfa2c35 100644
--- a/drivers/spi/spi-s3c64xx.c
+++ b/drivers/spi/spi-s3c64xx.c
@@ -826,7 +826,7 @@ static struct s3c64xx_spi_csinfo
*s3c64xx_get_slave_ctrldata(
struct spi_device *spi)
{
struct s3c64xx_spi_csinfo *cs;
- struct device_node *slave_np, *data_np;
+ struct device_node *slave_np, *data_np = NULL;
u32 fb_delay = 0;
slave_np = spi-dev.of_node;
--
1.7.10
Looks ok to me,
Acked-by: Kukjin Kim kgene@samsung.com
BTW for same reason, probably, we need following fix?
Thanks.
Best regards,
Kgene.
--
Kukjin Kim kgene@samsung.com, Senior Engineer,
SW Solution Development Team, Samsung Electronics Co., Ltd.
From: Kukjin Kim kgene@samsung.com
Signed-off-by: Kukjin Kim kgene@samsung.com
---
arch/arm/mach-tegra/tegra2_emc.c |4 ++--
arch/c6x/kernel/setup.c |2 +-
arch/powerpc/kernel/ibmebus.c|2 +-
arch/powerpc/kernel/pci_of_scan.c|2 +-
arch/powerpc/kernel/prom.c |2 +-
arch/powerpc/kernel/rtas_pci.c |2 +-
arch/powerpc/kernel/vio.c|2 +-
arch/powerpc/platforms/44x/warp.c|2 +-
arch/powerpc/platforms/cell/setup.c |2 +-
arch/powerpc/platforms/powernv/opal.c|2 +-
arch/powerpc/platforms/powernv/pci-ioda.c|2 +-
arch/powerpc/platforms/powernv/pci-p5ioc2.c |2 +-
arch/powerpc/platforms/pseries/eeh.c | 12 ++--
arch/powerpc/sysdev/mv64x60_dev.c|2 +-
drivers/dma/fsldma.c |2 +-
drivers/gpu/drm/nouveau/nouveau_connector.c |2 +-
drivers/hwmon/ads1015.c |2 +-
drivers/i2c/busses/i2c-powermac.c|2 +-
drivers/i2c/busses/i2c-pxa-pci.c |2 +-
drivers/i2c/i2c-mux.c|2 +-
drivers/iio/adc/at91_adc.c |2 +-
drivers/input/keyboard/samsung-keypad.c |2 +-
drivers/leds/leds-gpio.c |2 +-
drivers/net/ethernet/freescale/fsl_pq_mdio.c |2 +-
drivers/net/phy/mdio-mux.c |2 +-
drivers/of/of_i2c.c |2 +-
drivers/of/of_mdio.c |2 +-
drivers/of/of_pci.c |2 +-
drivers/of/platform.c|6 +++---
drivers/pinctrl/pinctrl-imx.c|4 ++--
drivers/pinctrl/pinctrl-tegra.c |2 +-
drivers/pinctrl/spear/pinctrl-spear.c|2 +-
drivers/regulator/da9052-regulator.c |2 +-
drivers/regulator/mc13xxx-regulator-core.c |2 +-
drivers/regulator/of_regulator.c |2 +-
drivers/spi/spi.c|2 +-
drivers/tty/hvc/hvc_opal.c |2 +-
37 files changed, 46 insertions(+), 46 deletions(-)
diff --git a/arch/arm/mach-tegra/tegra2_emc.c
b/arch/arm/mach-tegra/tegra2_emc.c
index 5070d83..d045a9e 100644
--- a/arch/arm/mach-tegra/tegra2_emc.c
+++ b/arch/arm/mach-tegra/tegra2_emc.c
@@ -181,7 +181,7 @@ int tegra_emc_set_rate(unsigned long rate)
#ifdef CONFIG_OF
static struct device_node *tegra_emc_ramcode_devnode(struct device_node
*np)
{
- struct device_node *iter;
+ struct device_node *iter = NULL;
u32 reg;
for_each_child_of_node(np, iter) {
@@ -198,7 +198,7 @@ static struct tegra_emc_pdata *tegra_emc_dt_parse_pdata(
struct platform_device *pdev)
{
struct device_node *np = pdev-dev.of_node;
- struct device_node *tnp, *iter;
+ struct device_node *tnp, *iter = NULL;
struct tegra_emc_pdata *pdata;
int ret, i, num_tables;
diff --git a/arch/c6x/kernel/setup.c b/arch/c6x/kernel/setup.c
index f4e72bd..0f4cd29 100644
--- a/arch/c6x/kernel/setup.c
+++ b/arch/c6x/kernel/setup.c
@@ -99,7 +99,7 @@ static void __init get_cpuinfo(void)
unsigned long core_khz;
u64 tmp;
struct cpuinfo_c6x *p;
- struct device_node
Re: [PATCH 10/11] spi/s3c64xx: improve error handling
On 8 August 2012 20:17, Arnd Bergmann a...@arndb.de wrote:
When a device tree definition os an s3c64xx SPI master is missing
a controller-data subnode, the newly added s3c64xx_get_slave_ctrldata
function might use uninitialized memory in place of that node,
which was correctly reported by gcc.
Without this patch, building s3c6400_defconfig results in:
drivers/spi/spi-s3c64xx.c: In function 's3c64xx_get_slave_ctrldata.isra.25':
drivers/spi/spi-s3c64xx.c:841:5: warning: 'data_np' may be used uninitialized
in this function [-Wuninitialized]
Signed-off-by: Arnd Bergmann a...@arndb.de
Cc: Thomas Abraham thomas.abra...@linaro.org
Cc: Jaswinder Singh jaswinder.si...@linaro.org
Cc: Grant Likely grant.lik...@secretlab.ca
Cc: Kukjin Kim kgene@samsung.com
---
drivers/spi/spi-s3c64xx.c |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/spi/spi-s3c64xx.c b/drivers/spi/spi-s3c64xx.c
index 646a765..cfa2c35 100644
--- a/drivers/spi/spi-s3c64xx.c
+++ b/drivers/spi/spi-s3c64xx.c
@@ -826,7 +826,7 @@ static struct s3c64xx_spi_csinfo
*s3c64xx_get_slave_ctrldata(
struct spi_device *spi)
{
struct s3c64xx_spi_csinfo *cs;
- struct device_node *slave_np, *data_np;
+ struct device_node *slave_np, *data_np = NULL;
u32 fb_delay = 0;
slave_np = spi-dev.of_node;
--
1.7.10
Acked-by: Thomas Abraham thomas.abra...@linaro.org
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 10/11] spi/s3c64xx: improve error handling
On Friday 10 August 2012, Kukjin Kim wrote: BTW for same reason, probably, we need following fix? arch/arm/mach-tegra/tegra2_emc.c |4 ++-- arch/c6x/kernel/setup.c |2 +- arch/powerpc/kernel/ibmebus.c|2 +- arch/powerpc/kernel/pci_of_scan.c|2 +- arch/powerpc/kernel/prom.c |2 +- arch/powerpc/kernel/rtas_pci.c |2 +- arch/powerpc/kernel/vio.c|2 +- arch/powerpc/platforms/44x/warp.c|2 +- ... Actually not. The difference is that only s3c64xx_get_slave_ctrldata accesses the node pointer outside of the look (after break). This fails when there are no child nodes at all. In the other cases, the only use of the node pointer is inside the loop, where it is guaranteed to be valid. Arnd -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
