| Message ID | 20190309071424.3600-1-kjlu@umn.edu (mailing list archive) |
|---|---|
| State | New |
| Delegated to: | Kieran Bingham |
| Headers | show |
| Series | media: renesas-ceu: fix a potential NULL pointer dereference | expand |
Hi Kangjie, thanks for the patch. On Sat, Mar 09, 2019 at 01:14:24AM -0600, Kangjie Lu wrote: > In case of_match_device cannot find a match, the check returns > -EINVAL to avoid a potential NULL pointer dereference > > Signed-off-by: Kangjie Lu <kjlu@umn.edu> > --- > drivers/media/platform/renesas-ceu.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/drivers/media/platform/renesas-ceu.c b/drivers/media/platform/renesas-ceu.c > index 150196f7cf96..4aa807c0b6c7 100644 > --- a/drivers/media/platform/renesas-ceu.c > +++ b/drivers/media/platform/renesas-ceu.c > @@ -1682,7 +1682,10 @@ static int ceu_probe(struct platform_device *pdev) > > if (IS_ENABLED(CONFIG_OF) && dev->of_node) { > ceu_data = of_match_device(ceu_of_match, dev)->data; > - num_subdevs = ceu_parse_dt(ceudev); > + if (unlikely(!ceu_data)) > + num_subdevs = -EINVAL; > + else > + num_subdevs = ceu_parse_dt(ceudev); I don't think this fix is required to be honest. If we call of_match_device() here we're sure CONFIG_OF is enabled, and if the driver probed, so a matching compatible string has proved to exist. Furthermore, if you want to protect against of_match_device() returning a NULL pointer, you should change this line first, as it would dereference an invalid pointer: ceu_data = of_match_device(ceu_of_match, dev)->data; but again, I don't think this might happen. Thanks j > } else if (dev->platform_data) { > /* Assume SH4 if booting with platform data. */ > ceu_data = &ceu_data_sh4; > -- > 2.17.1 >
On 3/9/19 8:14 AM, Kangjie Lu wrote: > In case of_match_device cannot find a match, the check returns > -EINVAL to avoid a potential NULL pointer dereference > > Signed-off-by: Kangjie Lu <kjlu@umn.edu> > --- > drivers/media/platform/renesas-ceu.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/drivers/media/platform/renesas-ceu.c b/drivers/media/platform/renesas-ceu.c > index 150196f7cf96..4aa807c0b6c7 100644 > --- a/drivers/media/platform/renesas-ceu.c > +++ b/drivers/media/platform/renesas-ceu.c > @@ -1682,7 +1682,10 @@ static int ceu_probe(struct platform_device *pdev) > > if (IS_ENABLED(CONFIG_OF) && dev->of_node) { > ceu_data = of_match_device(ceu_of_match, dev)->data; > - num_subdevs = ceu_parse_dt(ceudev); A far as I can tell ceu_parse_dt will never return 0. I'm dropping this patch. Regards, Hans > + if (unlikely(!ceu_data)) > + num_subdevs = -EINVAL; > + else > + num_subdevs = ceu_parse_dt(ceudev); > } else if (dev->platform_data) { > /* Assume SH4 if booting with platform data. */ > ceu_data = &ceu_data_sh4; >
diff --git a/drivers/media/platform/renesas-ceu.c b/drivers/media/platform/renesas-ceu.c index 150196f7cf96..4aa807c0b6c7 100644 --- a/drivers/media/platform/renesas-ceu.c +++ b/drivers/media/platform/renesas-ceu.c @@ -1682,7 +1682,10 @@ static int ceu_probe(struct platform_device *pdev) if (IS_ENABLED(CONFIG_OF) && dev->of_node) { ceu_data = of_match_device(ceu_of_match, dev)->data; - num_subdevs = ceu_parse_dt(ceudev); + if (unlikely(!ceu_data)) + num_subdevs = -EINVAL; + else + num_subdevs = ceu_parse_dt(ceudev); } else if (dev->platform_data) { /* Assume SH4 if booting with platform data. */ ceu_data = &ceu_data_sh4;
In case of_match_device cannot find a match, the check returns -EINVAL to avoid a potential NULL pointer dereference Signed-off-by: Kangjie Lu <kjlu@umn.edu> --- drivers/media/platform/renesas-ceu.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)