* [PATCH v1 1/3] serial: 8250_exar: Avoid NULL pointer dereference at ->exit() @ 2021-06-08 14:42 Andy Shevchenko 2021-06-08 14:42 ` [PATCH v1 2/3] serial: 8250_exar: Extract exar_get_platform() helper Andy Shevchenko ` (2 more replies) 0 siblings, 3 replies; 7+ messages in thread From: Andy Shevchenko @ 2021-06-08 14:42 UTC (permalink / raw) To: Andy Shevchenko, linux-serial, linux-kernel Cc: Greg Kroah-Hartman, Jiri Slaby, Heikki Krogerus, Maxim Levitsky It's possible that during ->exit() the private_data is NULL, for instance when there was no GPIO device instantiated. Due to this we may not dereference it. Add a respective check. Note, for now ->exit() only makes sense when GPIO device was instantiated, that's why we may use the check for entire function. Fixes: 81171e7d31a6 ("serial: 8250_exar: Constify the software nodes") Reported-by: Maxim Levitsky <mlevitsk@redhat.com> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> --- drivers/tty/serial/8250/8250_exar.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/drivers/tty/serial/8250/8250_exar.c b/drivers/tty/serial/8250/8250_exar.c index 2f49c580139b..bd4e9f6ac29c 100644 --- a/drivers/tty/serial/8250/8250_exar.c +++ b/drivers/tty/serial/8250/8250_exar.c @@ -553,7 +553,11 @@ static void pci_xr17v35x_exit(struct pci_dev *pcidev) { struct exar8250 *priv = pci_get_drvdata(pcidev); struct uart_8250_port *port = serial8250_get_port(priv->line[0]); - struct platform_device *pdev = port->port.private_data; + struct platform_device *pdev; + + pdev = port->port.private_data; + if (!pdev) + return; device_remove_software_node(&pdev->dev); platform_device_unregister(pdev); -- 2.30.2 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH v1 2/3] serial: 8250_exar: Extract exar_get_platform() helper 2021-06-08 14:42 [PATCH v1 1/3] serial: 8250_exar: Avoid NULL pointer dereference at ->exit() Andy Shevchenko @ 2021-06-08 14:42 ` Andy Shevchenko 2021-06-09 12:41 ` Greg Kroah-Hartman 2021-06-08 14:42 ` [PATCH v1 3/3] serial: 8250_exar: Add ->unregister_gpio() callback Andy Shevchenko 2021-06-08 15:09 ` [PATCH v1 1/3] serial: 8250_exar: Avoid NULL pointer dereference at ->exit() Maxim Levitsky 2 siblings, 1 reply; 7+ messages in thread From: Andy Shevchenko @ 2021-06-08 14:42 UTC (permalink / raw) To: Andy Shevchenko, linux-serial, linux-kernel Cc: Greg Kroah-Hartman, Jiri Slaby, Heikki Krogerus We would like to use DMI matching in other functions as well. Hence, extract it as exar_get_platform() helper function. Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> --- drivers/tty/serial/8250/8250_exar.c | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/drivers/tty/serial/8250/8250_exar.c b/drivers/tty/serial/8250/8250_exar.c index bd4e9f6ac29c..3ffeedc29c83 100644 --- a/drivers/tty/serial/8250/8250_exar.c +++ b/drivers/tty/serial/8250/8250_exar.c @@ -501,23 +501,27 @@ static const struct dmi_system_id exar_platforms[] = { {} }; +static const struct exar8250_platform *exar_get_platform(void) +{ + const struct dmi_system_id *dmi_match; + + dmi_match = dmi_first_match(exar_platforms); + if (dmi_match) + return dmi_match->driver_data; + + return &exar8250_default_platform; +} + static int pci_xr17v35x_setup(struct exar8250 *priv, struct pci_dev *pcidev, struct uart_8250_port *port, int idx) { - const struct exar8250_platform *platform; - const struct dmi_system_id *dmi_match; + const struct exar8250_platform *platform = exar_get_platform(); unsigned int offset = idx * 0x400; unsigned int baud = 7812500; u8 __iomem *p; int ret; - dmi_match = dmi_first_match(exar_platforms); - if (dmi_match) - platform = dmi_match->driver_data; - else - platform = &exar8250_default_platform; - port->port.uartclk = baud * 16; port->port.rs485_config = platform->rs485_config; -- 2.30.2 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v1 2/3] serial: 8250_exar: Extract exar_get_platform() helper 2021-06-08 14:42 ` [PATCH v1 2/3] serial: 8250_exar: Extract exar_get_platform() helper Andy Shevchenko @ 2021-06-09 12:41 ` Greg Kroah-Hartman 2021-06-09 17:02 ` Andy Shevchenko 0 siblings, 1 reply; 7+ messages in thread From: Greg Kroah-Hartman @ 2021-06-09 12:41 UTC (permalink / raw) To: Andy Shevchenko; +Cc: linux-serial, linux-kernel, Jiri Slaby, Heikki Krogerus On Tue, Jun 08, 2021 at 05:42:38PM +0300, Andy Shevchenko wrote: > We would like to use DMI matching in other functions as well. > Hence, extract it as exar_get_platform() helper function. > > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > --- > drivers/tty/serial/8250/8250_exar.c | 20 ++++++++++++-------- > 1 file changed, 12 insertions(+), 8 deletions(-) > > diff --git a/drivers/tty/serial/8250/8250_exar.c b/drivers/tty/serial/8250/8250_exar.c > index bd4e9f6ac29c..3ffeedc29c83 100644 > --- a/drivers/tty/serial/8250/8250_exar.c > +++ b/drivers/tty/serial/8250/8250_exar.c > @@ -501,23 +501,27 @@ static const struct dmi_system_id exar_platforms[] = { > {} > }; > > +static const struct exar8250_platform *exar_get_platform(void) > +{ > + const struct dmi_system_id *dmi_match; > + > + dmi_match = dmi_first_match(exar_platforms); > + if (dmi_match) > + return dmi_match->driver_data; > + > + return &exar8250_default_platform; > +} > + > static int > pci_xr17v35x_setup(struct exar8250 *priv, struct pci_dev *pcidev, > struct uart_8250_port *port, int idx) > { > - const struct exar8250_platform *platform; > - const struct dmi_system_id *dmi_match; > + const struct exar8250_platform *platform = exar_get_platform(); > unsigned int offset = idx * 0x400; > unsigned int baud = 7812500; > u8 __iomem *p; > int ret; > > - dmi_match = dmi_first_match(exar_platforms); > - if (dmi_match) > - platform = dmi_match->driver_data; > - else > - platform = &exar8250_default_platform; > - > port->port.uartclk = baud * 16; > port->port.rs485_config = platform->rs485_config; > > -- > 2.30.2 > Do not mix "fixes with features" in a single series, as I now have to pick it apart and apply it to different branches by hand :( Please do different series for the two different things if at all possible. thanks, greg k-h ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v1 2/3] serial: 8250_exar: Extract exar_get_platform() helper 2021-06-09 12:41 ` Greg Kroah-Hartman @ 2021-06-09 17:02 ` Andy Shevchenko 0 siblings, 0 replies; 7+ messages in thread From: Andy Shevchenko @ 2021-06-09 17:02 UTC (permalink / raw) To: Greg Kroah-Hartman Cc: linux-serial, linux-kernel, Jiri Slaby, Heikki Krogerus On Wed, Jun 09, 2021 at 02:41:38PM +0200, Greg Kroah-Hartman wrote: > On Tue, Jun 08, 2021 at 05:42:38PM +0300, Andy Shevchenko wrote: > > We would like to use DMI matching in other functions as well. > > Hence, extract it as exar_get_platform() helper function. > Do not mix "fixes with features" in a single series, as I now have to > pick it apart and apply it to different branches by hand :( > > Please do different series for the two different things if at all > possible. I see. Thank you for pointing out, I will keep in mind for the future submissions. -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v1 3/3] serial: 8250_exar: Add ->unregister_gpio() callback 2021-06-08 14:42 [PATCH v1 1/3] serial: 8250_exar: Avoid NULL pointer dereference at ->exit() Andy Shevchenko 2021-06-08 14:42 ` [PATCH v1 2/3] serial: 8250_exar: Extract exar_get_platform() helper Andy Shevchenko @ 2021-06-08 14:42 ` Andy Shevchenko 2021-06-09 12:42 ` Greg Kroah-Hartman 2021-06-08 15:09 ` [PATCH v1 1/3] serial: 8250_exar: Avoid NULL pointer dereference at ->exit() Maxim Levitsky 2 siblings, 1 reply; 7+ messages in thread From: Andy Shevchenko @ 2021-06-08 14:42 UTC (permalink / raw) To: Andy Shevchenko, linux-serial, linux-kernel Cc: Greg Kroah-Hartman, Jiri Slaby, Heikki Krogerus For the sake of reducing layering violation add ->unregister_gpio() callback and use it in the ->exit() one. Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> --- drivers/tty/serial/8250/8250_exar.c | 36 ++++++++++++++++++----------- 1 file changed, 23 insertions(+), 13 deletions(-) diff --git a/drivers/tty/serial/8250/8250_exar.c b/drivers/tty/serial/8250/8250_exar.c index 3ffeedc29c83..d502240bbcf2 100644 --- a/drivers/tty/serial/8250/8250_exar.c +++ b/drivers/tty/serial/8250/8250_exar.c @@ -114,6 +114,7 @@ struct exar8250; struct exar8250_platform { int (*rs485_config)(struct uart_port *, struct serial_rs485 *); int (*register_gpio)(struct pci_dev *, struct uart_8250_port *); + void (*unregister_gpio)(struct uart_8250_port *); }; /** @@ -352,9 +353,8 @@ static void setup_gpio(struct pci_dev *pcidev, u8 __iomem *p) writeb(0x00, p + UART_EXAR_MPIOOD_15_8); } -static void * -__xr17v35x_register_gpio(struct pci_dev *pcidev, - const struct software_node *node) +static struct platform_device *__xr17v35x_register_gpio(struct pci_dev *pcidev, + const struct software_node *node) { struct platform_device *pdev; @@ -374,6 +374,12 @@ __xr17v35x_register_gpio(struct pci_dev *pcidev, return pdev; } +static void __xr17v35x_unregister_gpio(struct platform_device *pdev) +{ + device_remove_software_node(&pdev->dev); + platform_device_unregister(pdev); +} + static const struct property_entry exar_gpio_properties[] = { PROPERTY_ENTRY_U32("exar,first-pin", 0), PROPERTY_ENTRY_U32("ngpios", 16), @@ -384,8 +390,7 @@ static const struct software_node exar_gpio_node = { .properties = exar_gpio_properties, }; -static int xr17v35x_register_gpio(struct pci_dev *pcidev, - struct uart_8250_port *port) +static int xr17v35x_register_gpio(struct pci_dev *pcidev, struct uart_8250_port *port) { if (pcidev->vendor == PCI_VENDOR_ID_EXAR) port->port.private_data = @@ -394,6 +399,15 @@ static int xr17v35x_register_gpio(struct pci_dev *pcidev, return 0; } +static void xr17v35x_unregister_gpio(struct uart_8250_port *port) +{ + if (!port->port.private_data) + return; + + __xr17v35x_unregister_gpio(port->port.private_data); + port->port.private_data = NULL; +} + static int generic_rs485_config(struct uart_port *port, struct serial_rs485 *rs485) { @@ -419,6 +433,7 @@ static int generic_rs485_config(struct uart_port *port, static const struct exar8250_platform exar8250_default_platform = { .register_gpio = xr17v35x_register_gpio, + .unregister_gpio = xr17v35x_unregister_gpio, .rs485_config = generic_rs485_config, }; @@ -484,6 +499,7 @@ static int iot2040_register_gpio(struct pci_dev *pcidev, static const struct exar8250_platform iot2040_platform = { .rs485_config = iot2040_rs485_config, .register_gpio = iot2040_register_gpio, + .unregister_gpio = xr17v35x_unregister_gpio, }; /* @@ -555,17 +571,11 @@ pci_xr17v35x_setup(struct exar8250 *priv, struct pci_dev *pcidev, static void pci_xr17v35x_exit(struct pci_dev *pcidev) { + const struct exar8250_platform *platform = exar_get_platform(); struct exar8250 *priv = pci_get_drvdata(pcidev); struct uart_8250_port *port = serial8250_get_port(priv->line[0]); - struct platform_device *pdev; - pdev = port->port.private_data; - if (!pdev) - return; - - device_remove_software_node(&pdev->dev); - platform_device_unregister(pdev); - port->port.private_data = NULL; + platform->unregister_gpio(port); } static inline void exar_misc_clear(struct exar8250 *priv) -- 2.30.2 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v1 3/3] serial: 8250_exar: Add ->unregister_gpio() callback 2021-06-08 14:42 ` [PATCH v1 3/3] serial: 8250_exar: Add ->unregister_gpio() callback Andy Shevchenko @ 2021-06-09 12:42 ` Greg Kroah-Hartman 0 siblings, 0 replies; 7+ messages in thread From: Greg Kroah-Hartman @ 2021-06-09 12:42 UTC (permalink / raw) To: Andy Shevchenko; +Cc: linux-serial, linux-kernel, Jiri Slaby, Heikki Krogerus On Tue, Jun 08, 2021 at 05:42:39PM +0300, Andy Shevchenko wrote: > For the sake of reducing layering violation add ->unregister_gpio() > callback and use it in the ->exit() one. > > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > --- > drivers/tty/serial/8250/8250_exar.c | 36 ++++++++++++++++++----------- > 1 file changed, 23 insertions(+), 13 deletions(-) > > diff --git a/drivers/tty/serial/8250/8250_exar.c b/drivers/tty/serial/8250/8250_exar.c > index 3ffeedc29c83..d502240bbcf2 100644 > --- a/drivers/tty/serial/8250/8250_exar.c > +++ b/drivers/tty/serial/8250/8250_exar.c > @@ -114,6 +114,7 @@ struct exar8250; > struct exar8250_platform { > int (*rs485_config)(struct uart_port *, struct serial_rs485 *); > int (*register_gpio)(struct pci_dev *, struct uart_8250_port *); > + void (*unregister_gpio)(struct uart_8250_port *); > }; > > /** > @@ -352,9 +353,8 @@ static void setup_gpio(struct pci_dev *pcidev, u8 __iomem *p) > writeb(0x00, p + UART_EXAR_MPIOOD_15_8); > } > > -static void * > -__xr17v35x_register_gpio(struct pci_dev *pcidev, > - const struct software_node *node) > +static struct platform_device *__xr17v35x_register_gpio(struct pci_dev *pcidev, > + const struct software_node *node) > { > struct platform_device *pdev; > > @@ -374,6 +374,12 @@ __xr17v35x_register_gpio(struct pci_dev *pcidev, > return pdev; > } > > +static void __xr17v35x_unregister_gpio(struct platform_device *pdev) > +{ > + device_remove_software_node(&pdev->dev); > + platform_device_unregister(pdev); > +} > + > static const struct property_entry exar_gpio_properties[] = { > PROPERTY_ENTRY_U32("exar,first-pin", 0), > PROPERTY_ENTRY_U32("ngpios", 16), > @@ -384,8 +390,7 @@ static const struct software_node exar_gpio_node = { > .properties = exar_gpio_properties, > }; > > -static int xr17v35x_register_gpio(struct pci_dev *pcidev, > - struct uart_8250_port *port) > +static int xr17v35x_register_gpio(struct pci_dev *pcidev, struct uart_8250_port *port) > { > if (pcidev->vendor == PCI_VENDOR_ID_EXAR) > port->port.private_data = > @@ -394,6 +399,15 @@ static int xr17v35x_register_gpio(struct pci_dev *pcidev, > return 0; > } > > +static void xr17v35x_unregister_gpio(struct uart_8250_port *port) > +{ > + if (!port->port.private_data) > + return; > + > + __xr17v35x_unregister_gpio(port->port.private_data); > + port->port.private_data = NULL; > +} > + > static int generic_rs485_config(struct uart_port *port, > struct serial_rs485 *rs485) > { > @@ -419,6 +433,7 @@ static int generic_rs485_config(struct uart_port *port, > > static const struct exar8250_platform exar8250_default_platform = { > .register_gpio = xr17v35x_register_gpio, > + .unregister_gpio = xr17v35x_unregister_gpio, > .rs485_config = generic_rs485_config, > }; > > @@ -484,6 +499,7 @@ static int iot2040_register_gpio(struct pci_dev *pcidev, > static const struct exar8250_platform iot2040_platform = { > .rs485_config = iot2040_rs485_config, > .register_gpio = iot2040_register_gpio, > + .unregister_gpio = xr17v35x_unregister_gpio, > }; > > /* > @@ -555,17 +571,11 @@ pci_xr17v35x_setup(struct exar8250 *priv, struct pci_dev *pcidev, > > static void pci_xr17v35x_exit(struct pci_dev *pcidev) > { > + const struct exar8250_platform *platform = exar_get_platform(); > struct exar8250 *priv = pci_get_drvdata(pcidev); > struct uart_8250_port *port = serial8250_get_port(priv->line[0]); > - struct platform_device *pdev; > > - pdev = port->port.private_data; > - if (!pdev) > - return; > - > - device_remove_software_node(&pdev->dev); > - platform_device_unregister(pdev); > - port->port.private_data = NULL; > + platform->unregister_gpio(port); > } > > static inline void exar_misc_clear(struct exar8250 *priv) > -- > 2.30.2 > This patch doesn't apply to my tty-next branch, so I've dropped it :( greg k-h ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v1 1/3] serial: 8250_exar: Avoid NULL pointer dereference at ->exit() 2021-06-08 14:42 [PATCH v1 1/3] serial: 8250_exar: Avoid NULL pointer dereference at ->exit() Andy Shevchenko 2021-06-08 14:42 ` [PATCH v1 2/3] serial: 8250_exar: Extract exar_get_platform() helper Andy Shevchenko 2021-06-08 14:42 ` [PATCH v1 3/3] serial: 8250_exar: Add ->unregister_gpio() callback Andy Shevchenko @ 2021-06-08 15:09 ` Maxim Levitsky 2 siblings, 0 replies; 7+ messages in thread From: Maxim Levitsky @ 2021-06-08 15:09 UTC (permalink / raw) To: Andy Shevchenko, linux-serial, linux-kernel Cc: Greg Kroah-Hartman, Jiri Slaby, Heikki Krogerus On Tue, 2021-06-08 at 17:42 +0300, Andy Shevchenko wrote: > It's possible that during ->exit() the private_data is NULL, > for instance when there was no GPIO device instantiated. > Due to this we may not dereference it. Add a respective check. > > Note, for now ->exit() only makes sense when GPIO device > was instantiated, that's why we may use the check for entire > function. > > Fixes: 81171e7d31a6 ("serial: 8250_exar: Constify the software nodes") > Reported-by: Maxim Levitsky <mlevitsk@redhat.com> > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > --- > drivers/tty/serial/8250/8250_exar.c | 6 +++++- > 1 file changed, 5 insertions(+), 1 deletion(-) > > diff --git a/drivers/tty/serial/8250/8250_exar.c b/drivers/tty/serial/8250/8250_exar.c > index 2f49c580139b..bd4e9f6ac29c 100644 > --- a/drivers/tty/serial/8250/8250_exar.c > +++ b/drivers/tty/serial/8250/8250_exar.c > @@ -553,7 +553,11 @@ static void pci_xr17v35x_exit(struct pci_dev *pcidev) > { > struct exar8250 *priv = pci_get_drvdata(pcidev); > struct uart_8250_port *port = serial8250_get_port(priv->line[0]); > - struct platform_device *pdev = port->port.private_data; > + struct platform_device *pdev; > + > + pdev = port->port.private_data; > + if (!pdev) > + return; > > device_remove_software_node(&pdev->dev); > platform_device_unregister(pdev); Hi! I just tested this patch and it does work! Thanks a lot for fixing this that fast! Tested-by: Maxim Levitsky <mlevitsk@redhat.com> Best regards, Maxim Levitsky ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2021-06-09 17:02 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-06-08 14:42 [PATCH v1 1/3] serial: 8250_exar: Avoid NULL pointer dereference at ->exit() Andy Shevchenko 2021-06-08 14:42 ` [PATCH v1 2/3] serial: 8250_exar: Extract exar_get_platform() helper Andy Shevchenko 2021-06-09 12:41 ` Greg Kroah-Hartman 2021-06-09 17:02 ` Andy Shevchenko 2021-06-08 14:42 ` [PATCH v1 3/3] serial: 8250_exar: Add ->unregister_gpio() callback Andy Shevchenko 2021-06-09 12:42 ` Greg Kroah-Hartman 2021-06-08 15:09 ` [PATCH v1 1/3] serial: 8250_exar: Avoid NULL pointer dereference at ->exit() Maxim Levitsky
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.