tty, serial: a couple of unrelated fixes
Subsystems (2)
Jiri Slaby (SUSE)
maintainercover tty, serial: a couple of unrelated fixes
Three unrelated fixes which accumulated in my tree:
* 8250_exar: an error pointer could be stored into priv->gpio and later dereferenced on unbind, reported by Sashiko,
* 8250_hp300: a pure reorg to get rid of forward declarations and shrink the number of ifdefs,
* tty_io: compat_ptr() was applied to the wrong ioctl() argument since 2013.
Jiri Slaby (SUSE) (3):
serial: 8250_exar: check priv->gpio properly
serial: 8250_hp300: reorder HPDCA code
tty: fix misapplied compat_ptr() in tty_compat_ioctl()
drivers/tty/serial/8250/8250_exar.c | 2 +- drivers/tty/serial/8250/8250_hp300.c | 62 ++++++++++++---------------- drivers/tty/tty_io.c | 3 +- 3 files changed, 28 insertions(+), 39 deletions(-)
Signature · 1 line
2.56.0
Jiri Slaby (SUSE)
maintainerPATCH 1/3 serial: 8250_exar: check priv->gpio properly
priv->gpio is set by platform_device_register_full(), which returns error-encoded pointers on failure, not NULL. Sashiko reported:
Quoted 7 lines
Does
__xr17v35x_register_gpio()return anERR_PTRifplatform_device_register_full()fails?If it does, we might be storing an error pointer directly into
priv->gpioinxr17v35x_register_gpio()without validating it withIS_ERR(). Because the error is ignored, the device probe successfully continues.
Before commit 1145d1a0703f ("serial: 8250_exar: use platform_device_register_full()"), __xr17v35x_register_gpio() returned NULL on any failure (platform_device_alloc(), device_add_software_node() or platform_device_add() failing were all turned into a NULL return). The plain truthy check in xr17v35x_unregister_gpio() matched that convention. That commit switched registration to platform_device_register_full(), which reports failure as an ERR_PTR() instead of NULL, but the unregister check was never updated to match. As a result, a failed registration left an error pointer in priv->gpio, and unbind would tear it down as if it were a real platform_device, dereferencing the encoded error value in device_remove_software_node() and platform_device_unregister().
Use IS_ERR_OR_NULL() instead, so both the unset (non-EXAR vendor, priv->gpio never assigned) and failed-registration (ERR_PTR) cases are skipped.
--- drivers/tty/serial/8250/8250_exar.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/tty/serial/8250/8250_exar.c +1 -1
@@ -1181,7 +1181,7 @@ static int xr17v35x_register_gpio(struct exar8250 *priv, struct pci_dev *pcidev,static void xr17v35x_unregister_gpio (struct exar8250 *priv) { -if (priv->gpio) { +if (!IS_ERR_OR_NULL (priv->gpio)) {__xr17v35x_unregister_gpio (priv->gpio); priv->gpio =NULL ; }
Signature · 1 line
2.56.0
Jiri Slaby (SUSE)
maintainerPATCH 2/3 serial: 8250_hp300: reorder HPDCA code
In reply to Jiri Slaby (SUSE)To have less ifdefs (code is now under one) and no forward declarations.
--- drivers/tty/serial/8250/8250_hp300.c | 62 ++++++++++++---------------- 1 file changed, 26 insertions(+), 36 deletions(-)
drivers/tty/serial/8250/8250_hp300.c +26 -36
@@ -34,29 +34,6 @@ struct hp300_port {static struct hp300_port *hp300_ports;#endif -#ifdef CONFIG_HPDCA - -static int hpdca_init_one (struct dio_dev *d, -const struct dio_device_id *ent); -static void hpdca_remove_one (struct dio_dev *d); - -static struct dio_device_id hpdca_dio_tbl[] = { - {DIO_ID_DCA0 }, - {DIO_ID_DCA0REM }, - {DIO_ID_DCA1 }, - {DIO_ID_DCA1REM }, - {0 } -}; - -static struct dio_driver hpdca_driver = { - .name ="hpdca" , - .id_table = hpdca_dio_tbl, - .probe = hpdca_init_one, - .remove = hpdca_remove_one, -}; - -#endif -static unsigned int num_ports;extern int hp300_uart_scode;
@@ -197,6 +174,32 @@ static int hpdca_init_one(struct dio_dev *d, return 0; } + +static void hpdca_remove_one (struct dio_dev *d) +{ +struct uart_8250_port *uport =dio_get_drvdata (d); + +if (d->resource.start) { +/* Disable board-interrupts */ +out_8 (d->resource.start +DIO_VIRADDRBASE +DCA_IC ,0 ); + } +serial8250_unregister_port (uport); +} + +static struct dio_device_id hpdca_dio_tbl[] = { + {DIO_ID_DCA0 }, + {DIO_ID_DCA0REM }, + {DIO_ID_DCA1 }, + {DIO_ID_DCA1REM }, + {0 } +}; + +static struct dio_driver hpdca_driver = { + .name ="hpdca" , + .id_table = hpdca_dio_tbl, + .probe = hpdca_init_one, + .remove = hpdca_remove_one, +};#endif static int __init hp300_8250_init (void )
@@ -280,19 +283,6 @@ static int __init hp300_8250_init(void)return 0 ; } -#ifdef CONFIG_HPDCA -static void hpdca_remove_one (struct dio_dev *d) -{ -struct uart_8250_port *uport =dio_get_drvdata (d); - -if (d->resource.start) { -/* Disable board-interrupts */ -out_8 (d->resource.start +DIO_VIRADDRBASE +DCA_IC ,0 ); - } -serial8250_unregister_port (uport); -} -#endif - static void __exit hp300_8250_exit(void) {#ifdef CONFIG_HPAPCI
Signature · 1 line
2.56.0
Jiri Slaby (SUSE)
maintainerPATCH 3/3 tty: fix misapplied compat_ptr() in tty_compat_ioctl()
In reply to Jiri Slaby (SUSE)Since commit f0193d3ea73b below, cmd was converted with compat_ptr(), while arg was passed to ld->ops->ioctl() unmodified. Fix it so cmd stays cmd and arg gets converted through compat_ptr() as intended.
Notes:
2) compat_ptr() did nothing to cmd, provided it is uint,
1) compat_ptr() is no-op everywhere (was different on s390 before 8e0b986c59c6 ("s390: Remove compat support")),
3) arg might not have been chopped correctly until 8e0b986c59c6. But I am not sure why noone noticed all those (7) years (from f0193d3ea73b to 8e0b986c59c6)?
--- drivers/tty/tty_io.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)
drivers/tty/tty_io.c +1 -2
@@ -2969,8 +2969,7 @@ static long tty_compat_ioctl(struct file *file, unsigned int cmd,if (ld->ops->compat_ioctl) retval = ld->ops->compat_ioctl (tty, cmd, arg);if (retval == -ENOIOCTLCMD && ld->ops->ioctl) - retval = ld->ops->ioctl (tty, (unsigned long )compat_ptr (cmd), - arg); + retval = ld->ops->ioctl (tty, cmd, (unsigned long )compat_ptr (arg));tty_ldisc_deref (ld);return retval;
Signature · 1 line
2.56.0
[email protected]
AI reviewRe: [PATCH 3/3] tty: fix misapplied compat_ptr() in tty_compat_ioctl()
Sashiko has reviewed this patch and found no issues.
Quoted 12 lines
Since commit
f0193d3ea73bbelow,cmdwas converted withcompat_ptr(), whileargwas passed told->ops->ioctl()unmodified. Fix it socmdstayscmdandarggets converted throughcompat_ptr()as intended.Notes:
2)compat_ptr()did nothing tocmd, provided it is uint,
1)compat_ptr()is no-op everywhere (was different on s390 before8e0b986c59c6("s390: Remove compat support")),
3)argmight not have been chopped correctly until8e0b986c59c6. But I am not sure why noone noticed all those (7) years (fromf0193d3ea73bto8e0b986c59c6)?
Quoted 2 lines
Fixes f0193d3ea73b ("change semantics of ldisc ->compat_ioctl()")Signed-off-by Jiri Slaby (SUSE) <[email protected]>
Sashiko has reviewed this patch and found no issues.
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=3
[email protected]
AI reviewRe: [PATCH 2/3] serial: 8250_hp300: reorder HPDCA code
In reply to Jiri Slaby (SUSE)Sashiko has reviewed this patch and found no issues.
Quoted 3 lines
To have less ifdefs (code is now under one) and no forward declarations.
Signed-off-by Jiri Slaby (SUSE) <[email protected]>
Sashiko has reviewed this patch and found no issues.
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=2
[email protected]
AI reviewRe: [PATCH 1/3] serial: 8250_exar: check priv->gpio properly
In reply to Jiri Slaby (SUSE)Sashiko has reviewed this patch and found no issues.
Quoted 19 lines
priv->gpiois set byplatform_device_register_full(), which returns error-encoded pointers on failure, not NULL. Sashiko reported:Quoted 7 lines
Does
__xr17v35x_register_gpio()return anERR_PTRifplatform_device_register_full()fails?If it does, we might be storing an error pointer directly into
priv->gpioinxr17v35x_register_gpio()without validating it withIS_ERR(). Because the error is ignored, the device probe successfully continues.Before commit
1145d1a0703f("serial: 8250_exar: useplatform_device_register_full()"),__xr17v35x_register_gpio()returned NULL on any failure (platform_device_alloc(),device_add_software_node()orplatform_device_add()failing were all [ ... ]priv->gpionever assigned) and failed-registration (ERR_PTR) cases are skipped.
Quoted 2 lines
Fixes 1145d1a0703f ("serial: 8250_exar: use platform_device_register_full()")Signed-off-by Jiri Slaby (SUSE) <[email protected]>
Sashiko has reviewed this patch and found no issues.
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1