Dereferncing of_id pointer will result in exception in current
implementation since of_match_device() will assign it to NULL.
Adding NULL check for protection.
Signed-off-by: Ameer Hamza <[email protected]>
---
sound/soc/generic/test-component.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/sound/soc/generic/test-component.c b/sound/soc/generic/test-component.c
index 85385a771d80..8fc97d3ff011 100644
--- a/sound/soc/generic/test-component.c
+++ b/sound/soc/generic/test-component.c
@@ -532,13 +532,16 @@ static int test_driver_probe(struct platform_device *pdev)
struct device_node *node = dev->of_node;
struct device_node *ep;
const struct of_device_id *of_id = of_match_device(test_of_match, &pdev->dev);
- const struct test_adata *adata = of_id->data;
+ const struct test_adata *adata;
struct snd_soc_component_driver *cdriv;
struct snd_soc_dai_driver *ddriv;
struct test_dai_name *dname;
struct test_priv *priv;
int num, ret, i;
+ if (!of_id)
+ return -EINVAL;
+ adata = of_id->data;
num = of_graph_get_endpoint_count(node);
if (!num) {
dev_err(dev, "no port exits\n");
--
2.25.1
Hi Ameer
Thank you for your patch.
> Dereferncing of_id pointer will result in exception in current
> implementation since of_match_device() will assign it to NULL.
> Adding NULL check for protection.
(snip)
> @@ -532,13 +532,16 @@ static int test_driver_probe(struct platform_device *pdev)
> struct device_node *node = dev->of_node;
> struct device_node *ep;
> const struct of_device_id *of_id = of_match_device(test_of_match, &pdev->dev);
> - const struct test_adata *adata = of_id->data;
> + const struct test_adata *adata;
> struct snd_soc_component_driver *cdriv;
> struct snd_soc_dai_driver *ddriv;
> struct test_dai_name *dname;
> struct test_priv *priv;
> int num, ret, i;
>
> + if (!of_id)
> + return -EINVAL;
> + adata = of_id->data;
But hmm...
Probing this driver without adata is strange for me.
How did probe this driver ??
Best regards
---
Kuninori Morimoto
On Sun, Dec 05, 2021 at 10:40:27PM +0000, Kuninori Morimoto wrote:
>
> Hi Ameer
>
> Thank you for your patch.
>
> > Dereferncing of_id pointer will result in exception in current
> > implementation since of_match_device() will assign it to NULL.
> > Adding NULL check for protection.
> (snip)
> > @@ -532,13 +532,16 @@ static int test_driver_probe(struct platform_device *pdev)
> > struct device_node *node = dev->of_node;
> > struct device_node *ep;
> > const struct of_device_id *of_id = of_match_device(test_of_match, &pdev->dev);
> > - const struct test_adata *adata = of_id->data;
> > + const struct test_adata *adata;
> > struct snd_soc_component_driver *cdriv;
> > struct snd_soc_dai_driver *ddriv;
> > struct test_dai_name *dname;
> > struct test_priv *priv;
> > int num, ret, i;
> >
> > + if (!of_id)
> > + return -EINVAL;
> > + adata = of_id->data;
>
> But hmm...
> Probing this driver without adata is strange for me.
> How did probe this driver ??
Thank you for your response. Unfortunately, I am not aware of
implementation details of this component. Coverity suggested that there
can be a potential NULL pointer access which seems logical to me. Do you
agree with coverity here?
>
> Best regards
> ---
> Kuninori Morimoto
On Mon, 6 Dec 2021 01:42:00 +0500, Ameer Hamza wrote:
> Dereferncing of_id pointer will result in exception in current
> implementation since of_match_device() will assign it to NULL.
> Adding NULL check for protection.
>
>
Applied to
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next
Thanks!
[1/1] ASoC: test-component: fix null pointer dereference.
commit: c686316ec1210d43653c91e104c1e4cd0156dc89
All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying
to this mail.
Thanks,
Mark
Hi Ameer
>> Probing this driver without adata is strange for me.
>> How did probe this driver ??
>
> Thank you for your response. Unfortunately, I am not aware of
> implementation details of this component. Coverity suggested that there
> can be a potential NULL pointer access which seems logical to me. Do you
> agree with coverity here?
I think no potential NULL pointer access, because this driver can't
be called without of_id->data.
But, potential NULL pointer check itself is good idea.
It seems your patch was already accepted :)
I noticed that we can replace it to use of_device_get_match_data()
- const struct of_device_id *of_id = of_match_device(test_of_match, &pdev->dev);
- const struct test_adata *adata = of_id->data;
+ const struct test_adata *adata = of_device_get_match_data(&pdev->dev);
Best regards
---
Kuninori Morimoto
On Mon, Dec 06, 2021 at 11:19:36PM +0000, Kuninori Morimoto wrote:
>
> Hi Ameer
>
> >> Probing this driver without adata is strange for me.
> >> How did probe this driver ??
> >
> > Thank you for your response. Unfortunately, I am not aware of
> > implementation details of this component. Coverity suggested that there
> > can be a potential NULL pointer access which seems logical to me. Do you
> > agree with coverity here?
>
> I think no potential NULL pointer access, because this driver can't
> be called without of_id->data.
> But, potential NULL pointer check itself is good idea.
> It seems your patch was already accepted :)
Yes, indeed.
> I noticed that we can replace it to use of_device_get_match_data()
>
> - const struct of_device_id *of_id = of_match_device(test_of_match, &pdev->dev);
> - const struct test_adata *adata = of_id->data;
> + const struct test_adata *adata = of_device_get_match_data(&pdev->dev);
Thanks, that's a cleaner way. Can you advise how should proceed with
this since previous patch is already applied. Should I send a updated
version of patch, e.g., v2 or a new patch.
>
> Best regards
> ---
> Kuninori Morimoto
On Tue, Dec 07, 2021 at 01:15:22PM +0500, Ameer Hamza wrote:
> On Mon, Dec 06, 2021 at 11:19:36PM +0000, Kuninori Morimoto wrote:
> > - const struct test_adata *adata = of_id->data;
> > + const struct test_adata *adata = of_device_get_match_data(&pdev->dev);
> Thanks, that's a cleaner way. Can you advise how should proceed with
> this since previous patch is already applied. Should I send a updated
> version of patch, e.g., v2 or a new patch.
Please send an incremental patch on top of what is already applied as
covered in the message saying the patch was applied.
Dereferncing of_id pointer will result in exception in current
implementation since of_match_device() will assign it to NULL.
Adding NULL check for protection.
Signed-off-by: Ameer Hamza <[email protected]>
---
changes in v2:
Replace of_match_device() with of_device_get_match_data to simplify the
logic as suggested by Kuninori Morimoto
---
sound/soc/generic/test-component.c | 8 ++------
1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/sound/soc/generic/test-component.c b/sound/soc/generic/test-component.c
index 8fc97d3ff011..5da4725d9e16 100644
--- a/sound/soc/generic/test-component.c
+++ b/sound/soc/generic/test-component.c
@@ -531,17 +531,13 @@ static int test_driver_probe(struct platform_device *pdev)
struct device *dev = &pdev->dev;
struct device_node *node = dev->of_node;
struct device_node *ep;
- const struct of_device_id *of_id = of_match_device(test_of_match, &pdev->dev);
- const struct test_adata *adata;
+ const struct test_adata *adata = of_device_get_match_data(&pdev->dev);
struct snd_soc_component_driver *cdriv;
struct snd_soc_dai_driver *ddriv;
struct test_dai_name *dname;
struct test_priv *priv;
int num, ret, i;
- if (!of_id)
- return -EINVAL;
- adata = of_id->data;
num = of_graph_get_endpoint_count(node);
if (!num) {
dev_err(dev, "no port exits\n");
@@ -552,7 +548,7 @@ static int test_driver_probe(struct platform_device *pdev)
cdriv = devm_kzalloc(dev, sizeof(*cdriv), GFP_KERNEL);
ddriv = devm_kzalloc(dev, sizeof(*ddriv) * num, GFP_KERNEL);
dname = devm_kzalloc(dev, sizeof(*dname) * num, GFP_KERNEL);
- if (!priv || !cdriv || !ddriv || !dname)
+ if (!priv || !cdriv || !ddriv || !dname || !adata)
return -EINVAL;
priv->dev = dev;
--
2.25.1
Hi Ameer, Mark
>> > - const struct test_adata *adata = of_id->data;
>> > + const struct test_adata *adata = of_device_get_match_data(&pdev->dev);
>
>> Thanks, that's a cleaner way. Can you advise how should proceed with
>> this since previous patch is already applied. Should I send a updated
>> version of patch, e.g., v2 or a new patch.
>
> Please send an incremental patch on top of what is already applied as
> covered in the message saying the patch was applied.
No worry.
I will post that patch
Best regards
---
Kuninori Morimoto
Hi Ameer
Ah, you posted the patch :)
> Subject: [PATCH v2] ASoC: test-component: fix null pointer dereference.
>
> Dereferncing of_id pointer will result in exception in current
> implementation since of_match_device() will assign it to NULL.
> Adding NULL check for protection.
Previous your patch was already accepted,
thus this is not v2 patch.
git log should indicate is replace of_match_device() to of_device_get_match_data()
Best regards
---
Kuninori Morimoto
On Wed, Dec 08, 2021 at 12:05:46AM +0000, Kuninori Morimoto wrote:
>
> Hi Ameer
>
> Ah, you posted the patch :)
>
> > Subject: [PATCH v2] ASoC: test-component: fix null pointer dereference.
> >
> > Dereferncing of_id pointer will result in exception in current
> > implementation since of_match_device() will assign it to NULL.
> > Adding NULL check for protection.
>
> Previous your patch was already accepted,
> thus this is not v2 patch.
> git log should indicate is replace of_match_device() to of_device_get_match_data()
Thank you for your kind response and clarifying the process. Let me send
the updated patch. :)
Best Regards,
Ameer Hamza.
Getting device data from of_device_get_match_data() for a cleaner
implementation.
Signed-off-by: Ameer Hamza <[email protected]>
---
sound/soc/generic/test-component.c | 8 ++------
1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/sound/soc/generic/test-component.c b/sound/soc/generic/test-component.c
index 8fc97d3ff011..5da4725d9e16 100644
--- a/sound/soc/generic/test-component.c
+++ b/sound/soc/generic/test-component.c
@@ -531,17 +531,13 @@ static int test_driver_probe(struct platform_device *pdev)
struct device *dev = &pdev->dev;
struct device_node *node = dev->of_node;
struct device_node *ep;
- const struct of_device_id *of_id = of_match_device(test_of_match, &pdev->dev);
- const struct test_adata *adata;
+ const struct test_adata *adata = of_device_get_match_data(&pdev->dev);
struct snd_soc_component_driver *cdriv;
struct snd_soc_dai_driver *ddriv;
struct test_dai_name *dname;
struct test_priv *priv;
int num, ret, i;
- if (!of_id)
- return -EINVAL;
- adata = of_id->data;
num = of_graph_get_endpoint_count(node);
if (!num) {
dev_err(dev, "no port exits\n");
@@ -552,7 +548,7 @@ static int test_driver_probe(struct platform_device *pdev)
cdriv = devm_kzalloc(dev, sizeof(*cdriv), GFP_KERNEL);
ddriv = devm_kzalloc(dev, sizeof(*ddriv) * num, GFP_KERNEL);
dname = devm_kzalloc(dev, sizeof(*dname) * num, GFP_KERNEL);
- if (!priv || !cdriv || !ddriv || !dname)
+ if (!priv || !cdriv || !ddriv || !dname || !adata)
return -EINVAL;
priv->dev = dev;
--
2.25.1
On Wed, Dec 08, 2021 at 05:36:59PM +0500, Ameer Hamza wrote:
> Getting device data from of_device_get_match_data() for a cleaner
> implementation.
>
> Signed-off-by: Ameer Hamza <[email protected]>
> ---
> sound/soc/generic/test-component.c | 8 ++------
> 1 file changed, 2 insertions(+), 6 deletions(-)
>
> diff --git a/sound/soc/generic/test-component.c b/sound/soc/generic/test-component.c
> index 8fc97d3ff011..5da4725d9e16 100644
> --- a/sound/soc/generic/test-component.c
> +++ b/sound/soc/generic/test-component.c
> @@ -531,17 +531,13 @@ static int test_driver_probe(struct platform_device *pdev)
> struct device *dev = &pdev->dev;
> struct device_node *node = dev->of_node;
> struct device_node *ep;
> - const struct of_device_id *of_id = of_match_device(test_of_match, &pdev->dev);
> - const struct test_adata *adata;
> + const struct test_adata *adata = of_device_get_match_data(&pdev->dev);
> struct snd_soc_component_driver *cdriv;
> struct snd_soc_dai_driver *ddriv;
> struct test_dai_name *dname;
> struct test_priv *priv;
> int num, ret, i;
>
> - if (!of_id)
> - return -EINVAL;
> - adata = of_id->data;
> num = of_graph_get_endpoint_count(node);
> if (!num) {
> dev_err(dev, "no port exits\n");
> @@ -552,7 +548,7 @@ static int test_driver_probe(struct platform_device *pdev)
> cdriv = devm_kzalloc(dev, sizeof(*cdriv), GFP_KERNEL);
> ddriv = devm_kzalloc(dev, sizeof(*ddriv) * num, GFP_KERNEL);
> dname = devm_kzalloc(dev, sizeof(*dname) * num, GFP_KERNEL);
> - if (!priv || !cdriv || !ddriv || !dname)
> + if (!priv || !cdriv || !ddriv || !dname || !adata)
> return -EINVAL;
>
> priv->dev = dev;
> --
Hi Kuninori,
Would be really appreciated if you can review the patch please.
Best Regards,
Ameer Hamza.
Hi
> Getting device data from of_device_get_match_data() for a cleaner
> implementation.
>
> Signed-off-by: Ameer Hamza <[email protected]>
> ---
Reviewed-by: Kuninori Morimoto <[email protected]>
Best regards
---
Kuninori Morimoto
On Tue, 7 Dec 2021 19:23:09 +0500, Ameer Hamza wrote:
> Dereferncing of_id pointer will result in exception in current
> implementation since of_match_device() will assign it to NULL.
> Adding NULL check for protection.
>
>
Applied to
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next
Thanks!
[1/1] ASoC: test-component: fix null pointer dereference.
commit: c686316ec1210d43653c91e104c1e4cd0156dc89
All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying
to this mail.
Thanks,
Mark