[PATCH] crypto: inside-secure - Fix null ptr derefence on rmmod for macchiatobin

This small patch fixes a null pointer derefence panic that occurred when
unloading the driver (using rmmod) on macchiatobin due to not setting
the platform driver data properly in the probe routine.

Signed-off-by: Pascal van Leeuwen <redacted>
---
 drivers/crypto/inside-secure/safexcel.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/crypto/inside-secure/safexcel.c b/drivers/crypto/inside-secure/safexcel.c
index 45443bf..423ea2d 100644
--- a/drivers/crypto/inside-secure/safexcel.c
+++ b/drivers/crypto/inside-secure/safexcel.c

@@ -1274,6 +1274,8 @@ static int safexcel_probe(struct platform_device *pdev)
 	priv->dev = dev;
 	priv->version = (enum safexcel_eip_version)of_device_get_match_data(dev);
 
+	platform_set_drvdata(pdev, priv);
+
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	priv->base = devm_ioremap_resource(dev, res);
 	if (IS_ERR(priv->base)) {

-- 
1.8.3.1

Hi Pascal,

On Fri, Jul 26, 2019 at 05:31:09PM +0200, Pascal van Leeuwen wrote:

This small patch fixes a null pointer derefence panic that occurred when
unloading the driver (using rmmod) on macchiatobin due to not setting
the platform driver data properly in the probe routine.

Signed-off-by: Pascal van Leeuwen <redacted>

As this is a fix you should add a Fixes: tag so that the patch gets
applied to stable trees. You can have a look at what this tag looks like
at: https://www.kernel.org/doc/html/latest/process/submitting-patches.html

This is already done in safexcel_probe(), near the end of the function.
I think you should remove the second call, to avoid setting the platform
driver data twice.

Out of curiosity, why calling platform_set_drvdata() earlier in the
probe fixes unloading the driver with rmmod?

Thanks!
Antoine

-- 
Antoine Ténart, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

-----Original Message-----

From: Antoine Tenart <redacted>
Sent: Tuesday, July 30, 2019 10:12 AM
To: Pascal van Leeuwen <redacted>
Cc: linux-crypto@vger.kernel.org; antoine.tenart@bootlin.com; herbert@gondor.apana.org.au;
davem@davemloft.net; Pascal Van Leeuwen [off-list ref]
Subject: Re: [PATCH] crypto: inside-secure - Fix null ptr derefence on rmmod for
macchiatobin

Hi Pascal,

On Fri, Jul 26, 2019 at 05:31:09PM +0200, Pascal van Leeuwen wrote:

quoted

This small patch fixes a null pointer derefence panic that occurred when
unloading the driver (using rmmod) on macchiatobin due to not setting
the platform driver data properly in the probe routine.

Signed-off-by: Pascal van Leeuwen <redacted>

As this is a fix you should add a Fixes: tag so that the patch gets
applied to stable trees. You can have a look at what this tag looks like
at: https://www.kernel.org/doc/html/latest/process/submitting-patches.html

As you already figured out by now, this patch just fixes something
that was broken by one of my earlier patches (which has not been
applied just yet). So I don't think it applies to stable trees.

quoted

---
 drivers/crypto/inside-secure/safexcel.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/crypto/inside-secure/safexcel.c b/drivers/crypto/inside-

secure/safexcel.c

quoted

index 45443bf..423ea2d 100644

--- a/drivers/crypto/inside-secure/safexcel.c
+++ b/drivers/crypto/inside-secure/safexcel.c

@@ -1274,6 +1274,8 @@ static int safexcel_probe(struct platform_device *pdev)
 	priv->dev = dev;
 	priv->version = (enum safexcel_eip_version)of_device_get_match_data(dev);

+	platform_set_drvdata(pdev, priv);
+

This is already done in safexcel_probe(), near the end of the function.
I think you should remove the second call, to avoid setting the platform
driver data twice.

Well, actually, my first patch that you are reviewing right now
accidentally removed that other call to platform_set_drvdata ...

Out of curiosity, why calling platform_set_drvdata() earlier in the
probe fixes unloading the driver with rmmod?

Not really. As long as it does get set somewhere :-)

Thanks!
Antoine

--
Antoine Ténart, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Regards,
Pascal van Leeuwen
Silicon IP Architect, Multi-Protocol Engines @ Verimatrix
www.insidesecure.com

On Tue, Jul 30, 2019 at 10:27:26AM +0000, Pascal Van Leeuwen wrote:

As you already figured out by now, this patch just fixes something
that was broken by one of my earlier patches (which has not been
applied just yet). So I don't think it applies to stable trees.

If your earlier patch has been applied to cryptodev already then you
should use a Fixes header.  If not then you should repost that patch
with the fix folded into it.

Thanks,
-- 
Email: Herbert Xu [off-list ref]
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

-----Original Message-----
From: Herbert Xu <herbert@gondor.apana.org.au>
Sent: Tuesday, July 30, 2019 1:35 PM
To: Pascal Van Leeuwen <redacted>
Cc: Antoine Tenart <redacted>; Pascal van Leeuwen
[off-list ref]; linux-crypto@vger.kernel.org; davem@davemloft.net
Subject: Re: [PATCH] crypto: inside-secure - Fix null ptr derefence on rmmod for
macchiatobin

On Tue, Jul 30, 2019 at 10:27:26AM +0000, Pascal Van Leeuwen wrote:

quoted

As you already figured out by now, this patch just fixes something
that was broken by one of my earlier patches (which has not been
applied just yet). So I don't think it applies to stable trees.

If your earlier patch has been applied to cryptodev already then you
should use a Fixes header.  If not then you should repost that patch
with the fix folded into it.

Yes, I understood that much. But thanks for spelling it out ;-)

Thanks,
--
Email: Herbert Xu [off-list ref]
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Regards,
Pascal van Leeuwen
Silicon IP Architect, Multi-Protocol Engines @ Verimatrix
www.insidesecure.com