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(-)

2.56.0

priv->gpio is set by platform_device_register_full(), which returns error-encoded pointers on failure, not NULL. Sashiko reported:

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(-)

index 57847c5bd2fc..b1e20fd4cfd2 100644
@@ -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;
 	}

2.56.0

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(-)

index e9599c5c2fea..68ce5972c296 100644
@@ -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

2.56.0

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(-)

index 1a6c8a1bc764..150052f6d9db 100644
@@ -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;

2.56.0