[PATCH] spi: xilinx_spi: Fix potential null pointer access

It was incorrectly using an old priv->regs pointer, and may lead to null pointer access.
Signed-off-by: Jiajie Chen c@jia.je ---
drivers/spi/xilinx_spi.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/spi/xilinx_spi.c b/drivers/spi/xilinx_spi.c index 4e9115dafe..e759b66000 100644 --- a/drivers/spi/xilinx_spi.c +++ b/drivers/spi/xilinx_spi.c @@ -112,9 +112,7 @@ struct xilinx_spi_priv { static int xilinx_spi_probe(struct udevice *bus) { struct xilinx_spi_priv *priv = dev_get_priv(bus); - struct xilinx_spi_regs *regs = priv->regs; - - priv->regs = (struct xilinx_spi_regs *)dev_read_addr(bus); + struct xilinx_spi_regs *regs = priv->regs = (struct xilinx_spi_regs *)dev_read_addr(bus);
priv->fifo_depth = dev_read_u32_default(bus, "fifo-size", 0);

On 2/21/23 06:22, Jiajie Chen wrote:
It was incorrectly using an old priv->regs pointer, and may lead to null pointer access.
I would describe it a little bit differently to describe what it happening.
priv structure is initiated by DM core to zeros that's why regs property is pointing to 0 address + spi offsets. That's why likely spi resets never happened.
Signed-off-by: Jiajie Chen c@jia.je
drivers/spi/xilinx_spi.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/spi/xilinx_spi.c b/drivers/spi/xilinx_spi.c index 4e9115dafe..e759b66000 100644 --- a/drivers/spi/xilinx_spi.c +++ b/drivers/spi/xilinx_spi.c @@ -112,9 +112,7 @@ struct xilinx_spi_priv { static int xilinx_spi_probe(struct udevice *bus) { struct xilinx_spi_priv *priv = dev_get_priv(bus);
- struct xilinx_spi_regs *regs = priv->regs;
- priv->regs = (struct xilinx_spi_regs *)dev_read_addr(bus);
- struct xilinx_spi_regs *regs = priv->regs = (struct xilinx_spi_regs *)dev_read_addr(bus);
The patch is good but I pretty much don't like this long line. Can you please do it on 2 lines instead?
It would be easier to read.
Thanks, Michal

Comments below.
On 2023/2/27 22:51, Michal Simek wrote:
On 2/21/23 06:22, Jiajie Chen wrote:
It was incorrectly using an old priv->regs pointer, and may lead to null pointer access.
I would describe it a little bit differently to describe what it happening.
priv structure is initiated by DM core to zeros that's why regs property is pointing to 0 address + spi offsets. That's why likely spi resets never happened.
Thanks, I will post a v2 patch.
Signed-off-by: Jiajie Chen c@jia.je
drivers/spi/xilinx_spi.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/spi/xilinx_spi.c b/drivers/spi/xilinx_spi.c index 4e9115dafe..e759b66000 100644 --- a/drivers/spi/xilinx_spi.c +++ b/drivers/spi/xilinx_spi.c @@ -112,9 +112,7 @@ struct xilinx_spi_priv { static int xilinx_spi_probe(struct udevice *bus) { struct xilinx_spi_priv *priv = dev_get_priv(bus); - struct xilinx_spi_regs *regs = priv->regs;
- priv->regs = (struct xilinx_spi_regs *)dev_read_addr(bus); + struct xilinx_spi_regs *regs = priv->regs = (struct xilinx_spi_regs *)dev_read_addr(bus);
The patch is good but I pretty much don't like this long line. Can you please do it on 2 lines instead?
Sure.
It would be easier to read.
Thanks, Michal
participants (2)
-
Jiajie Chen
-
Michal Simek