* [PATCH 2/3] video/ssd1307fb: drop unneeded regulator NULL checks
2020-03-02 11:49 [PATCH 1/3] video/ssd1307fb: fix NULL pointer dereference in probe Ahmad Fatoum
@ 2020-03-02 11:49 ` Ahmad Fatoum
2020-03-02 11:49 ` [PATCH 3/3] video/ssd1307fb: make reset GPIO optional Ahmad Fatoum
2020-03-09 7:31 ` [PATCH 1/3] video/ssd1307fb: fix NULL pointer dereference in probe Sascha Hauer
2 siblings, 0 replies; 4+ messages in thread
From: Ahmad Fatoum @ 2020-03-02 11:49 UTC (permalink / raw)
To: barebox; +Cc: Ahmad Fatoum
regulator_{enable,disable} are already no-ops when the parameter is
NULL. Drop the NULL checks thusly.
Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
drivers/video/ssd1307fb.c | 16 ++++++----------
1 file changed, 6 insertions(+), 10 deletions(-)
diff --git a/drivers/video/ssd1307fb.c b/drivers/video/ssd1307fb.c
index 83a561a3e1fc..cc50698670e0 100644
--- a/drivers/video/ssd1307fb.c
+++ b/drivers/video/ssd1307fb.c
@@ -513,11 +513,9 @@ static int ssd1307fb_probe(struct device_d *dev)
goto reset_oled_error;
}
- if (par->vbat) {
- ret = regulator_disable(par->vbat);
- if (ret < 0)
- goto reset_oled_error;
- }
+ ret = regulator_disable(par->vbat);
+ if (ret < 0)
+ goto reset_oled_error;
i2c_set_clientdata(client, info);
@@ -525,11 +523,9 @@ static int ssd1307fb_probe(struct device_d *dev)
gpio_set_value(par->reset, 0);
udelay(4);
- if (par->vbat) {
- ret = regulator_enable(par->vbat);
- if (ret < 0)
- goto reset_oled_error;
- }
+ ret = regulator_enable(par->vbat);
+ if (ret < 0)
+ goto reset_oled_error;
mdelay(100);
--
2.25.0
_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox
^ permalink raw reply [flat|nested] 4+ messages in thread* [PATCH 3/3] video/ssd1307fb: make reset GPIO optional
2020-03-02 11:49 [PATCH 1/3] video/ssd1307fb: fix NULL pointer dereference in probe Ahmad Fatoum
2020-03-02 11:49 ` [PATCH 2/3] video/ssd1307fb: drop unneeded regulator NULL checks Ahmad Fatoum
@ 2020-03-02 11:49 ` Ahmad Fatoum
2020-03-09 7:31 ` [PATCH 1/3] video/ssd1307fb: fix NULL pointer dereference in probe Sascha Hauer
2 siblings, 0 replies; 4+ messages in thread
From: Ahmad Fatoum @ 2020-03-02 11:49 UTC (permalink / raw)
To: barebox; +Cc: Ahmad Fatoum
Both reset GPIO and regulator are optional as per the binding and the
driver can work without if we ignore their absence. Do so.
Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
drivers/video/ssd1307fb.c | 43 +++++++++++++++++++++------------------
1 file changed, 23 insertions(+), 20 deletions(-)
diff --git a/drivers/video/ssd1307fb.c b/drivers/video/ssd1307fb.c
index cc50698670e0..835814bf5306 100644
--- a/drivers/video/ssd1307fb.c
+++ b/drivers/video/ssd1307fb.c
@@ -415,12 +415,8 @@ static int ssd1307fb_probe(struct device_d *dev)
par->reset = of_get_named_gpio(node,
"reset-gpios", 0);
- if (!gpio_is_valid(par->reset)) {
- ret = par->reset;
- if (ret != -EPROBE_DEFER)
- dev_err(&client->dev,
- "Couldn't get named gpio 'reset-gpios': %s.\n",
- strerror(-ret));
+ if (!gpio_is_valid(par->reset) && par->reset == -EPROBE_DEFER) {
+ ret = -EPROBE_DEFER;
goto fb_alloc_error;
}
@@ -503,14 +499,16 @@ static int ssd1307fb_probe(struct device_d *dev)
info->screen_base = (u8 __force __iomem *)vmem;
- ret = gpio_request_one(par->reset,
- GPIOF_OUT_INIT_HIGH,
- "oled-reset");
- if (ret) {
- dev_err(&client->dev,
- "failed to request gpio %d: %d\n",
- par->reset, ret);
- goto reset_oled_error;
+ if (par->reset >= 0) {
+ ret = gpio_request_one(par->reset,
+ GPIOF_OUT_INIT_HIGH,
+ "oled-reset");
+ if (ret) {
+ dev_err(&client->dev,
+ "failed to request gpio %d: %d\n",
+ par->reset, ret);
+ goto reset_oled_error;
+ }
}
ret = regulator_disable(par->vbat);
@@ -519,18 +517,23 @@ static int ssd1307fb_probe(struct device_d *dev)
i2c_set_clientdata(client, info);
- /* Reset the screen */
- gpio_set_value(par->reset, 0);
- udelay(4);
+ if (par->reset > 0) {
+ /* Reset the screen */
+ gpio_set_value(par->reset, 0);
+ udelay(4);
+ }
ret = regulator_enable(par->vbat);
if (ret < 0)
goto reset_oled_error;
- mdelay(100);
+ if (par->vbat)
+ mdelay(100);
- gpio_set_value(par->reset, 1);
- udelay(4);
+ if (par->reset > 0) {
+ gpio_set_value(par->reset, 1);
+ udelay(4);
+ }
ret = ssd1307fb_init(par);
if (ret)
--
2.25.0
_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH 1/3] video/ssd1307fb: fix NULL pointer dereference in probe
2020-03-02 11:49 [PATCH 1/3] video/ssd1307fb: fix NULL pointer dereference in probe Ahmad Fatoum
2020-03-02 11:49 ` [PATCH 2/3] video/ssd1307fb: drop unneeded regulator NULL checks Ahmad Fatoum
2020-03-02 11:49 ` [PATCH 3/3] video/ssd1307fb: make reset GPIO optional Ahmad Fatoum
@ 2020-03-09 7:31 ` Sascha Hauer
2 siblings, 0 replies; 4+ messages in thread
From: Sascha Hauer @ 2020-03-09 7:31 UTC (permalink / raw)
To: Ahmad Fatoum; +Cc: barebox, bst
On Mon, Mar 02, 2020 at 12:49:53PM +0100, Ahmad Fatoum wrote:
> info->priv is dereferenced before a valid value has been set leading to
> a NULL pointer dereference in the probe function. Fix this.
>
> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> ---
Applied, thanks
Sascha
> Cc: bst@pengutronix.de
> Cc: str@pengutronix.de
>
> Hi,
>
> how did this ever work? Does the platform that you used this on not trap
> on NULL-pointer dereference?
>
> Cheers,
> ---
> drivers/video/ssd1307fb.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/video/ssd1307fb.c b/drivers/video/ssd1307fb.c
> index 70077e43a879..83a561a3e1fc 100644
> --- a/drivers/video/ssd1307fb.c
> +++ b/drivers/video/ssd1307fb.c
> @@ -405,8 +405,9 @@ static int ssd1307fb_probe(struct device_d *dev)
> }
>
> info = xzalloc(sizeof(struct fb_info));
> + par = xzalloc(sizeof(*par));
>
> - par = info->priv;
> + info->priv = par;
> par->info = info;
> par->client = client;
>
> @@ -574,6 +575,7 @@ reset_oled_error:
> free(vmem);
> fb_alloc_error:
> regulator_disable(par->vbat);
> + free(par);
> free(info);
> return ret;
> }
> --
> 2.25.0
>
>
> _______________________________________________
> barebox mailing list
> barebox@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/barebox
>
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox
^ permalink raw reply [flat|nested] 4+ messages in thread