[PATCH] soc: sunxi: sram: order the publication of base against sram_dev

[PATCH] soc: sunxi: sram: order the publication of base against sram_dev

[Jaidev Shastri via B4 Relay]

From: Jaidev Shastri <jaidevshastri@vt.edu>

sunxi_sram_claim() is called from the probe functions of the EMAC, the
video engine and other consumers. It gates on the file-scope base
pointer:

	if (!base)
		return -EPROBE_DEFER;

and then walks sram_dev->of_node in sunxi_sram_of_parse().
sunxi_sram_probe() writes sram_dev and then base, both with plain
stores, and the claimer reads both with plain loads. Neither the two
stores nor the two loads are ordered, so a claimer on another CPU can
pass the gate and dereference sram_dev == NULL.

Keep the mapped registers in a local and publish base with
smp_store_release() once sram_dev is set; read it with
smp_load_acquire() in sunxi_sram_claim(). The error value is still
stored to base so that a failed probe keeps reporting it to claimers.

Found with MBCheck, a static herd7-based memory consistency checker.

Signed-off-by: Jaidev Shastri <jaidevshastri@vt.edu>
---
 drivers/soc/sunxi/sunxi_sram.c | 28 +++++++++++++++++++++-------
 1 file changed, 21 insertions(+), 7 deletions(-)

diff --git a/drivers/soc/sunxi/sunxi_sram.c b/drivers/soc/sunxi/sunxi_sram.c
index 2ccaeacf8..0e0b86ad3 100644
--- a/drivers/soc/sunxi/sunxi_sram.c
+++ b/drivers/soc/sunxi/sunxi_sram.c
@@ -266,10 +266,14 @@ int sunxi_sram_claim(struct device *dev)
 	int err;
 	int count = 0;
 
-	if (IS_ERR(base))
-		return PTR_ERR(base);
+	void __iomem *regs;
 
-	if (!base)
+	/* Pairs with smp_store_release() in sunxi_sram_probe(). */
+	regs = smp_load_acquire(&base);
+	if (IS_ERR(regs))
+		return PTR_ERR(regs);
+
+	if (!regs)
 		return -EPROBE_DEFER;
 
 	if (!dev || !dev->of_node)
@@ -410,6 +414,7 @@ static int __init sunxi_sram_probe(struct platform_device *pdev)
 	const struct sunxi_sramc_variant *variant;
 	struct device *dev = &pdev->dev;
 	struct regmap *regmap;
+	void __iomem *regs;
 	int ret;
 
 	sram_dev = &pdev->dev;
@@ -420,12 +425,21 @@ static int __init sunxi_sram_probe(struct platform_device *pdev)
 
 	dev_set_drvdata(dev, (struct sunxi_sramc_variant *)variant);
 
-	base = devm_platform_ioremap_resource(pdev, 0);
-	if (IS_ERR(base))
-		return PTR_ERR(base);
+	regs = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(regs)) {
+		base = regs;
+		return PTR_ERR(regs);
+	}
+
+	/*
+	 * base is the gate that sunxi_sram_claim() tests before it walks
+	 * sram_dev->of_node. Publish it after sram_dev so that a claimer on
+	 * another CPU that passes the gate also sees sram_dev.
+	 */
+	smp_store_release(&base, regs);
 
 	if (variant->num_emac_clocks || variant->has_ldo_ctrl) {
-		regmap = devm_regmap_init_mmio(dev, base, &sunxi_sram_regmap_config);
+		regmap = devm_regmap_init_mmio(dev, regs, &sunxi_sram_regmap_config);
 		if (IS_ERR(regmap))
 			return PTR_ERR(regmap);
 

---
base-commit: 93f51579e7df248780214094418f205253383cc5
change-id: 20260921-mb-sunxi-sram-41c3288a4660

Best regards,
--  
Jaidev Shastri <jaidevshastri@vt.edu>

Re: [PATCH] soc: sunxi: sram: order the publication of base against sram_dev

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] soc: sunxi: code and commit message mismatch regarding sram_dev dependency
--

commit 02f3c870b0e732bf484750fa2453c39fde232ed6
Author: Jaidev Shastri <jaidevshastri@vt.edu>

soc: sunxi: sram: order the publication of base against sram_dev

This commit modifies sunxi_sram_probe() to use smp_store_release() when
publishing the base pointer, and sunxi_sram_claim() to use smp_load_acquire().
The stated goal is to prevent sunxi_sram_claim() from dereferencing a null
sram_dev pointer.

>     sunxi_sram_claim() is called from the probe functions of the EMAC, the
>     video engine and other consumers. It gates on the file-scope base
>     pointer:
>     
>             if (!base)
>                     return -EPROBE_DEFER;
>     
>     and then walks sram_dev->of_node in sunxi_sram_of_parse().
>     sunxi_sram_probe() writes sram_dev and then base, both with plain
>     stores, and the claimer reads both with plain loads. Neither the two
>     stores nor the two loads are ordered, so a claimer on another CPU can
>     pass the gate and dereference sram_dev == NULL.

[Severity: Medium]
Is this description accurate?

Looking at sunxi_sram_claim() in drivers/soc/sunxi/sunxi_sram.c, it checks
the base pointer but then walks the consumer's dev->of_node rather than
sram_dev->of_node:

int sunxi_sram_claim(struct device *dev)
{
        ...
        if (!dev || !dev->of_node)
                return -EINVAL;

        of_for_each_phandle(&it, err, dev->of_node, SUNXI_SRAM_PROP,
                            NULL, SUNXI_SRAM_CELLS) {
        ...
}

It doesn't appear that sunxi_sram_claim() accesses the sram_dev pointer at all,
so the data dependency described in the commit message doesn't seem to exist.

> diff --git a/drivers/soc/sunxi/sunxi_sram.c b/drivers/soc/sunxi/sunxi_sram.c
> index 2ccaeacf8c706..0e0b86ad3fd37 100644
> --- a/drivers/soc/sunxi/sunxi_sram.c
> +++ b/drivers/soc/sunxi/sunxi_sram.c
[ ... ]
> @@ -410,6 +414,7 @@ static int __init sunxi_sram_probe(struct platform_device *pdev)
>  	const struct sunxi_sramc_variant *variant;
>  	struct device *dev = &pdev->dev;
>  	struct regmap *regmap;
> +	void __iomem *regs;
>  	int ret;
>  
>  	sram_dev = &pdev->dev;
[ ... ]
> +	/*
> +	 * base is the gate that sunxi_sram_claim() tests before it walks
> +	 * sram_dev->of_node. Publish it after sram_dev so that a claimer on
> +	 * another CPU that passes the gate also sees sram_dev.
> +	 */

[Severity: Medium]
Does this comment need to be updated?

Since sunxi_sram_claim() iterates over dev->of_node rather than
sram_dev->of_node, the rationale for this memory barrier seems incorrect.

> +	smp_store_release(&base, regs);
>  
>  	if (variant->num_emac_clocks || variant->has_ldo_ctrl) {
> -		regmap = devm_regmap_init_mmio(dev, base, &sunxi_sram_regmap_config);
> +		regmap = devm_regmap_init_mmio(dev, regs, &sunxi_sram_regmap_config);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260921-mb-sunxi-sram-v1-1-ad7817f0ee7d@vt.edu?part=1

Re: [PATCH] soc: sunxi: sram: order the publication of base against sram_dev

[Jernej Škrabec]

Dne torek, 22. september 2026 ob 02:48:49 Srednjeevropski poletni čas je Jaidev Shastri via B4 Relay napisal(a):
> From: Jaidev Shastri <jaidevshastri@vt.edu>
> 
> sunxi_sram_claim() is called from the probe functions of the EMAC, the
> video engine and other consumers. It gates on the file-scope base
> pointer:
> 
> 	if (!base)
> 		return -EPROBE_DEFER;
> 
> and then walks sram_dev->of_node in sunxi_sram_of_parse().

Where do you see sunxi_sram_of_parse()? This function doesn't exist
in current kernel.

> sunxi_sram_probe() writes sram_dev and then base, both with plain
> stores, and the claimer reads both with plain loads. Neither the two
> stores nor the two loads are ordered, so a claimer on another CPU can
> pass the gate and dereference sram_dev == NULL.
> 
> Keep the mapped registers in a local and publish base with
> smp_store_release() once sram_dev is set; read it with
> smp_load_acquire() in sunxi_sram_claim(). The error value is still

Where does sunxi_sram_claim() use sram_dev? It's used only in debugfs
show method.

I don't think this solves real issue, so NAK.

Best regards,
Jernej

> stored to base so that a failed probe keeps reporting it to claimers.
> 
> Found with MBCheck, a static herd7-based memory consistency checker.
> 
> Signed-off-by: Jaidev Shastri <jaidevshastri@vt.edu>
> ---
>  drivers/soc/sunxi/sunxi_sram.c | 28 +++++++++++++++++++++-------
>  1 file changed, 21 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/soc/sunxi/sunxi_sram.c b/drivers/soc/sunxi/sunxi_sram.c
> index 2ccaeacf8..0e0b86ad3 100644
> --- a/drivers/soc/sunxi/sunxi_sram.c
> +++ b/drivers/soc/sunxi/sunxi_sram.c
> @@ -266,10 +266,14 @@ int sunxi_sram_claim(struct device *dev)
>  	int err;
>  	int count = 0;
>  
> -	if (IS_ERR(base))
> -		return PTR_ERR(base);
> +	void __iomem *regs;
>  
> -	if (!base)
> +	/* Pairs with smp_store_release() in sunxi_sram_probe(). */
> +	regs = smp_load_acquire(&base);
> +	if (IS_ERR(regs))
> +		return PTR_ERR(regs);
> +
> +	if (!regs)
>  		return -EPROBE_DEFER;
>  
>  	if (!dev || !dev->of_node)
> @@ -410,6 +414,7 @@ static int __init sunxi_sram_probe(struct platform_device *pdev)
>  	const struct sunxi_sramc_variant *variant;
>  	struct device *dev = &pdev->dev;
>  	struct regmap *regmap;
> +	void __iomem *regs;
>  	int ret;
>  
>  	sram_dev = &pdev->dev;
> @@ -420,12 +425,21 @@ static int __init sunxi_sram_probe(struct platform_device *pdev)
>  
>  	dev_set_drvdata(dev, (struct sunxi_sramc_variant *)variant);
>  
> -	base = devm_platform_ioremap_resource(pdev, 0);
> -	if (IS_ERR(base))
> -		return PTR_ERR(base);
> +	regs = devm_platform_ioremap_resource(pdev, 0);
> +	if (IS_ERR(regs)) {
> +		base = regs;
> +		return PTR_ERR(regs);
> +	}
> +
> +	/*
> +	 * base is the gate that sunxi_sram_claim() tests before it walks
> +	 * sram_dev->of_node. Publish it after sram_dev so that a claimer on
> +	 * another CPU that passes the gate also sees sram_dev.
> +	 */
> +	smp_store_release(&base, regs);
>  
>  	if (variant->num_emac_clocks || variant->has_ldo_ctrl) {
> -		regmap = devm_regmap_init_mmio(dev, base, &sunxi_sram_regmap_config);
> +		regmap = devm_regmap_init_mmio(dev, regs, &sunxi_sram_regmap_config);
>  		if (IS_ERR(regmap))
>  			return PTR_ERR(regmap);
>  
> 
> ---
> base-commit: 93f51579e7df248780214094418f205253383cc5
> change-id: 20260921-mb-sunxi-sram-41c3288a4660
> 
> Best regards,
> --  
> Jaidev Shastri <jaidevshastri@vt.edu>
> 
> 
>