From: Frieder Schrempf <[email protected]>
As it is allowed to use the mctrl_gpio_* functions before
initialization (as the 8250 driver does according to 434be0ae7aa7),
it seems appropriate to have a NULL check in all of the functions.
Otherwise the mctrl_gpio_to_gpiod() function is prone to be used
in a context that can lead to a NULL pointer dereference.
Signed-off-by: Frieder Schrempf <[email protected]>
---
Changes in v3
=============
* Move the changes in mctrl_gpio_to_gpiod() to a separate patch
---
drivers/tty/serial/serial_mctrl_gpio.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/tty/serial/serial_mctrl_gpio.c b/drivers/tty/serial/serial_mctrl_gpio.c
index 2b400189be91..54c43e02e375 100644
--- a/drivers/tty/serial/serial_mctrl_gpio.c
+++ b/drivers/tty/serial/serial_mctrl_gpio.c
@@ -61,6 +61,9 @@ EXPORT_SYMBOL_GPL(mctrl_gpio_set);
struct gpio_desc *mctrl_gpio_to_gpiod(struct mctrl_gpios *gpios,
enum mctrl_gpio_idx gidx)
{
+ if (gpios == NULL)
+ return NULL;
+
return gpios->gpio[gidx];
}
EXPORT_SYMBOL_GPL(mctrl_gpio_to_gpiod);
--
2.17.1
On Fri, Aug 02, 2019 at 10:04:10AM +0000, Schrempf Frieder wrote:
> From: Frieder Schrempf <[email protected]>
>
> As it is allowed to use the mctrl_gpio_* functions before
> initialization (as the 8250 driver does according to 434be0ae7aa7),
Actually I was surprised some time ago that 8250 used serial_mctrl
without first initializing it and expecting it to work. I didn't look in
detail, but I wouldn't go so far to call this "allowed". The commit
itself calls it "workaround" which seems a better match.
> it seems appropriate to have a NULL check in all of the functions.
> Otherwise the mctrl_gpio_to_gpiod() function is prone to be used
> in a context that can lead to a NULL pointer dereference.
>
> Signed-off-by: Frieder Schrempf <[email protected]>
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-K?nig |
Industrial Linux Solutions | http://www.pengutronix.de/ |
On 02.08.19 14:12, Uwe Kleine-König wrote:
> On Fri, Aug 02, 2019 at 10:04:10AM +0000, Schrempf Frieder wrote:
>> From: Frieder Schrempf <[email protected]>
>>
>> As it is allowed to use the mctrl_gpio_* functions before
>> initialization (as the 8250 driver does according to 434be0ae7aa7),
>
> Actually I was surprised some time ago that 8250 used serial_mctrl
> without first initializing it and expecting it to work. I didn't look in
> detail, but I wouldn't go so far to call this "allowed". The commit
> itself calls it "workaround" which seems a better match.
Ok, but if this is considered to be a workaround and as the 8250 driver
does not use mctrl_gpio_to_gpiod(), we should maybe just drop this patch
instead of encouraging others to use mctrl_gpio before initialization.
I'm really not sure what's best, so depending on what you will propose,
I'll send a new version of this patch with adjusted commit message or not.
By the way, Uwe and Fabio: Thanks for your reviews!
>
>> it seems appropriate to have a NULL check in all of the functions.
>> Otherwise the mctrl_gpio_to_gpiod() function is prone to be used
>> in a context that can lead to a NULL pointer dereference.
>>
>> Signed-off-by: Frieder Schrempf <[email protected]>
>
> Best regards
> Uwe
>
Hello Frieder,
On Mon, Aug 05, 2019 at 09:01:39AM +0000, Schrempf Frieder wrote:
> On 02.08.19 14:12, Uwe Kleine-K?nig wrote:
> > On Fri, Aug 02, 2019 at 10:04:10AM +0000, Schrempf Frieder wrote:
> >> From: Frieder Schrempf <[email protected]>
> >>
> >> As it is allowed to use the mctrl_gpio_* functions before
> >> initialization (as the 8250 driver does according to 434be0ae7aa7),
> >
> > Actually I was surprised some time ago that 8250 used serial_mctrl
> > without first initializing it and expecting it to work. I didn't look in
> > detail, but I wouldn't go so far to call this "allowed". The commit
> > itself calls it "workaround" which seems a better match.
>
> Ok, but if this is considered to be a workaround and as the 8250 driver
> does not use mctrl_gpio_to_gpiod(), we should maybe just drop this patch
> instead of encouraging others to use mctrl_gpio before initialization.
>
> I'm really not sure what's best, so depending on what you will propose,
> I'll send a new version of this patch with adjusted commit message or not.
I wouldn't encourage usage of mctrl-gpio before it's initialized. So I
suggest to drop this patch.
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-K?nig |
Industrial Linux Solutions | http://www.pengutronix.de/ |
On 06.08.19 09:45, Uwe Kleine-König wrote:
> Hello Frieder,
>
> On Mon, Aug 05, 2019 at 09:01:39AM +0000, Schrempf Frieder wrote:
>> On 02.08.19 14:12, Uwe Kleine-König wrote:
>>> On Fri, Aug 02, 2019 at 10:04:10AM +0000, Schrempf Frieder wrote:
>>>> From: Frieder Schrempf <[email protected]>
>>>>
>>>> As it is allowed to use the mctrl_gpio_* functions before
>>>> initialization (as the 8250 driver does according to 434be0ae7aa7),
>>>
>>> Actually I was surprised some time ago that 8250 used serial_mctrl
>>> without first initializing it and expecting it to work. I didn't look in
>>> detail, but I wouldn't go so far to call this "allowed". The commit
>>> itself calls it "workaround" which seems a better match.
>>
>> Ok, but if this is considered to be a workaround and as the 8250 driver
>> does not use mctrl_gpio_to_gpiod(), we should maybe just drop this patch
>> instead of encouraging others to use mctrl_gpio before initialization.
>>
>> I'm really not sure what's best, so depending on what you will propose,
>> I'll send a new version of this patch with adjusted commit message or not.
>
> I wouldn't encourage usage of mctrl-gpio before it's initialized. So I
> suggest to drop this patch.
Ok, thanks.