2021-08-17 16:22:19

by Len Baker

[permalink] [raw]
Subject: [PATCH] input/serio: Prefer strscpy over strlcpy

strlcpy() reads the entire source buffer first. This read may exceed the
destination size limit. This is both inefficient and can lead to linear
read overflows if a source string is not NUL-terminated. The safe
replacement is strscpy().

This is a previous step in the path to remove the strlcpy() function
entirely from the kernel [1].

In the case where a constant string is copied, the strscpy() is not
very useful (we know that a read overflow can not happen, and if the
dst is big enough that the string will not be truncated). However,
the change is made to avoid the use of strlcpy().

[1] https://github.com/KSPP/linux/issues/89

Signed-off-by: Len Baker <[email protected]>
---
Hi Dmitry,

If you disagree with the constant string copy changes I can use
strcpy or memcpy instead. In this discussion [2] the patch was
rejected since it not fixed any real issue. However, if we want
to cleanup the proliferation of str*cpy() functions in the kernel
this can be a good start point.

[2] https://lore.kernel.org/linux-hardening/[email protected]/

Regards,
Len

drivers/input/serio/altera_ps2.c | 4 ++--
drivers/input/serio/ams_delta_serio.c | 4 ++--
drivers/input/serio/apbps2.c | 2 +-
drivers/input/serio/ct82c710.c | 2 +-
drivers/input/serio/i8042.c | 14 +++++++-------
drivers/input/serio/olpc_apsp.c | 8 ++++----
drivers/input/serio/parkbd.c | 2 +-
drivers/input/serio/pcips2.c | 4 ++--
drivers/input/serio/ps2-gpio.c | 4 ++--
drivers/input/serio/ps2mult.c | 2 +-
drivers/input/serio/q40kbd.c | 4 ++--
drivers/input/serio/rpckbd.c | 4 ++--
drivers/input/serio/sa1111ps2.c | 4 ++--
drivers/input/serio/serport.c | 2 +-
drivers/input/serio/sun4i-ps2.c | 4 ++--
15 files changed, 32 insertions(+), 32 deletions(-)

diff --git a/drivers/input/serio/altera_ps2.c b/drivers/input/serio/altera_ps2.c
index 379e9240c2b3..3a92304f64fb 100644
--- a/drivers/input/serio/altera_ps2.c
+++ b/drivers/input/serio/altera_ps2.c
@@ -110,8 +110,8 @@ static int altera_ps2_probe(struct platform_device *pdev)
serio->write = altera_ps2_write;
serio->open = altera_ps2_open;
serio->close = altera_ps2_close;
- strlcpy(serio->name, dev_name(&pdev->dev), sizeof(serio->name));
- strlcpy(serio->phys, dev_name(&pdev->dev), sizeof(serio->phys));
+ strscpy(serio->name, dev_name(&pdev->dev), sizeof(serio->name));
+ strscpy(serio->phys, dev_name(&pdev->dev), sizeof(serio->phys));
serio->port_data = ps2if;
serio->dev.parent = &pdev->dev;
ps2if->io = serio;
diff --git a/drivers/input/serio/ams_delta_serio.c b/drivers/input/serio/ams_delta_serio.c
index 1c0be299f179..ec93cb4573c3 100644
--- a/drivers/input/serio/ams_delta_serio.c
+++ b/drivers/input/serio/ams_delta_serio.c
@@ -159,8 +159,8 @@ static int ams_delta_serio_init(struct platform_device *pdev)
serio->id.type = SERIO_8042;
serio->open = ams_delta_serio_open;
serio->close = ams_delta_serio_close;
- strlcpy(serio->name, "AMS DELTA keyboard adapter", sizeof(serio->name));
- strlcpy(serio->phys, dev_name(&pdev->dev), sizeof(serio->phys));
+ strscpy(serio->name, "AMS DELTA keyboard adapter", sizeof(serio->name));
+ strscpy(serio->phys, dev_name(&pdev->dev), sizeof(serio->phys));
serio->dev.parent = &pdev->dev;
serio->port_data = priv;

diff --git a/drivers/input/serio/apbps2.c b/drivers/input/serio/apbps2.c
index 974d7bfae0a0..9c9ce097f8bf 100644
--- a/drivers/input/serio/apbps2.c
+++ b/drivers/input/serio/apbps2.c
@@ -176,7 +176,7 @@ static int apbps2_of_probe(struct platform_device *ofdev)
priv->io->close = apbps2_close;
priv->io->write = apbps2_write;
priv->io->port_data = priv;
- strlcpy(priv->io->name, "APBPS2 PS/2", sizeof(priv->io->name));
+ strscpy(priv->io->name, "APBPS2 PS/2", sizeof(priv->io->name));
snprintf(priv->io->phys, sizeof(priv->io->phys),
"apbps2_%d", apbps2_idx++);

diff --git a/drivers/input/serio/ct82c710.c b/drivers/input/serio/ct82c710.c
index d45009d654bf..752ce60e2211 100644
--- a/drivers/input/serio/ct82c710.c
+++ b/drivers/input/serio/ct82c710.c
@@ -170,7 +170,7 @@ static int ct82c710_probe(struct platform_device *dev)
ct82c710_port->open = ct82c710_open;
ct82c710_port->close = ct82c710_close;
ct82c710_port->write = ct82c710_write;
- strlcpy(ct82c710_port->name, "C&T 82c710 mouse port",
+ strscpy(ct82c710_port->name, "C&T 82c710 mouse port",
sizeof(ct82c710_port->name));
snprintf(ct82c710_port->phys, sizeof(ct82c710_port->phys),
"isa%16llx/serio0", (unsigned long long)CT82C710_DATA);
diff --git a/drivers/input/serio/i8042.c b/drivers/input/serio/i8042.c
index 0b9f1d0a8f8b..681b1f9678a7 100644
--- a/drivers/input/serio/i8042.c
+++ b/drivers/input/serio/i8042.c
@@ -1337,9 +1337,9 @@ static int __init i8042_create_kbd_port(void)
serio->ps2_cmd_mutex = &i8042_mutex;
serio->port_data = port;
serio->dev.parent = &i8042_platform_device->dev;
- strlcpy(serio->name, "i8042 KBD port", sizeof(serio->name));
- strlcpy(serio->phys, I8042_KBD_PHYS_DESC, sizeof(serio->phys));
- strlcpy(serio->firmware_id, i8042_kbd_firmware_id,
+ strscpy(serio->name, "i8042 KBD port", sizeof(serio->name));
+ strscpy(serio->phys, I8042_KBD_PHYS_DESC, sizeof(serio->phys));
+ strscpy(serio->firmware_id, i8042_kbd_firmware_id,
sizeof(serio->firmware_id));
set_primary_fwnode(&serio->dev, i8042_kbd_fwnode);

@@ -1367,15 +1367,15 @@ static int __init i8042_create_aux_port(int idx)
serio->port_data = port;
serio->dev.parent = &i8042_platform_device->dev;
if (idx < 0) {
- strlcpy(serio->name, "i8042 AUX port", sizeof(serio->name));
- strlcpy(serio->phys, I8042_AUX_PHYS_DESC, sizeof(serio->phys));
- strlcpy(serio->firmware_id, i8042_aux_firmware_id,
+ strscpy(serio->name, "i8042 AUX port", sizeof(serio->name));
+ strscpy(serio->phys, I8042_AUX_PHYS_DESC, sizeof(serio->phys));
+ strscpy(serio->firmware_id, i8042_aux_firmware_id,
sizeof(serio->firmware_id));
serio->close = i8042_port_close;
} else {
snprintf(serio->name, sizeof(serio->name), "i8042 AUX%d port", idx);
snprintf(serio->phys, sizeof(serio->phys), I8042_MUX_PHYS_DESC, idx + 1);
- strlcpy(serio->firmware_id, i8042_aux_firmware_id,
+ strscpy(serio->firmware_id, i8042_aux_firmware_id,
sizeof(serio->firmware_id));
}

diff --git a/drivers/input/serio/olpc_apsp.c b/drivers/input/serio/olpc_apsp.c
index 59de8d9b6710..04d2db982fb8 100644
--- a/drivers/input/serio/olpc_apsp.c
+++ b/drivers/input/serio/olpc_apsp.c
@@ -199,8 +199,8 @@ static int olpc_apsp_probe(struct platform_device *pdev)
kb_serio->close = olpc_apsp_close;
kb_serio->port_data = priv;
kb_serio->dev.parent = &pdev->dev;
- strlcpy(kb_serio->name, "sp keyboard", sizeof(kb_serio->name));
- strlcpy(kb_serio->phys, "sp/serio0", sizeof(kb_serio->phys));
+ strscpy(kb_serio->name, "sp keyboard", sizeof(kb_serio->name));
+ strscpy(kb_serio->phys, "sp/serio0", sizeof(kb_serio->phys));
priv->kbio = kb_serio;
serio_register_port(kb_serio);

@@ -216,8 +216,8 @@ static int olpc_apsp_probe(struct platform_device *pdev)
pad_serio->close = olpc_apsp_close;
pad_serio->port_data = priv;
pad_serio->dev.parent = &pdev->dev;
- strlcpy(pad_serio->name, "sp touchpad", sizeof(pad_serio->name));
- strlcpy(pad_serio->phys, "sp/serio1", sizeof(pad_serio->phys));
+ strscpy(pad_serio->name, "sp touchpad", sizeof(pad_serio->name));
+ strscpy(pad_serio->phys, "sp/serio1", sizeof(pad_serio->phys));
priv->padio = pad_serio;
serio_register_port(pad_serio);

diff --git a/drivers/input/serio/parkbd.c b/drivers/input/serio/parkbd.c
index 3ac57a91ede4..6bee653d1965 100644
--- a/drivers/input/serio/parkbd.c
+++ b/drivers/input/serio/parkbd.c
@@ -169,7 +169,7 @@ static struct serio *parkbd_allocate_serio(void)
if (serio) {
serio->id.type = parkbd_mode;
serio->write = parkbd_write;
- strlcpy(serio->name, "PARKBD AT/XT keyboard adapter", sizeof(serio->name));
+ strscpy(serio->name, "PARKBD AT/XT keyboard adapter", sizeof(serio->name));
snprintf(serio->phys, sizeof(serio->phys), "%s/serio0", parkbd_dev->port->name);
}

diff --git a/drivers/input/serio/pcips2.c b/drivers/input/serio/pcips2.c
index bedf75de0a2c..05878750f2c2 100644
--- a/drivers/input/serio/pcips2.c
+++ b/drivers/input/serio/pcips2.c
@@ -149,8 +149,8 @@ static int pcips2_probe(struct pci_dev *dev, const struct pci_device_id *id)
serio->write = pcips2_write;
serio->open = pcips2_open;
serio->close = pcips2_close;
- strlcpy(serio->name, pci_name(dev), sizeof(serio->name));
- strlcpy(serio->phys, dev_name(&dev->dev), sizeof(serio->phys));
+ strscpy(serio->name, pci_name(dev), sizeof(serio->name));
+ strscpy(serio->phys, dev_name(&dev->dev), sizeof(serio->phys));
serio->port_data = ps2if;
serio->dev.parent = &dev->dev;
ps2if->io = serio;
diff --git a/drivers/input/serio/ps2-gpio.c b/drivers/input/serio/ps2-gpio.c
index 8970b49ea09a..1e5e8d94220a 100644
--- a/drivers/input/serio/ps2-gpio.c
+++ b/drivers/input/serio/ps2-gpio.c
@@ -393,8 +393,8 @@ static int ps2_gpio_probe(struct platform_device *pdev)
serio->write = drvdata->write_enable ? ps2_gpio_write : NULL;
serio->port_data = drvdata;
serio->dev.parent = dev;
- strlcpy(serio->name, dev_name(dev), sizeof(serio->name));
- strlcpy(serio->phys, dev_name(dev), sizeof(serio->phys));
+ strscpy(serio->name, dev_name(dev), sizeof(serio->name));
+ strscpy(serio->phys, dev_name(dev), sizeof(serio->phys));

drvdata->serio = serio;
drvdata->dev = dev;
diff --git a/drivers/input/serio/ps2mult.c b/drivers/input/serio/ps2mult.c
index 0071dd5ebcc2..902e81826fbf 100644
--- a/drivers/input/serio/ps2mult.c
+++ b/drivers/input/serio/ps2mult.c
@@ -131,7 +131,7 @@ static int ps2mult_create_port(struct ps2mult *psm, int i)
if (!serio)
return -ENOMEM;

- strlcpy(serio->name, "TQC PS/2 Multiplexer", sizeof(serio->name));
+ strscpy(serio->name, "TQC PS/2 Multiplexer", sizeof(serio->name));
snprintf(serio->phys, sizeof(serio->phys),
"%s/port%d", mx_serio->phys, i);
serio->id.type = SERIO_8042;
diff --git a/drivers/input/serio/q40kbd.c b/drivers/input/serio/q40kbd.c
index bd248398556a..a1c61f5de047 100644
--- a/drivers/input/serio/q40kbd.c
+++ b/drivers/input/serio/q40kbd.c
@@ -126,8 +126,8 @@ static int q40kbd_probe(struct platform_device *pdev)
port->close = q40kbd_close;
port->port_data = q40kbd;
port->dev.parent = &pdev->dev;
- strlcpy(port->name, "Q40 Kbd Port", sizeof(port->name));
- strlcpy(port->phys, "Q40", sizeof(port->phys));
+ strscpy(port->name, "Q40 Kbd Port", sizeof(port->name));
+ strscpy(port->phys, "Q40", sizeof(port->phys));

q40kbd_stop();

diff --git a/drivers/input/serio/rpckbd.c b/drivers/input/serio/rpckbd.c
index 37fe6a5711ea..7008bc101415 100644
--- a/drivers/input/serio/rpckbd.c
+++ b/drivers/input/serio/rpckbd.c
@@ -128,8 +128,8 @@ static int rpckbd_probe(struct platform_device *dev)
serio->close = rpckbd_close;
serio->dev.parent = &dev->dev;
serio->port_data = rpckbd;
- strlcpy(serio->name, "RiscPC PS/2 kbd port", sizeof(serio->name));
- strlcpy(serio->phys, "rpckbd/serio0", sizeof(serio->phys));
+ strscpy(serio->name, "RiscPC PS/2 kbd port", sizeof(serio->name));
+ strscpy(serio->phys, "rpckbd/serio0", sizeof(serio->phys));

platform_set_drvdata(dev, serio);
serio_register_port(serio);
diff --git a/drivers/input/serio/sa1111ps2.c b/drivers/input/serio/sa1111ps2.c
index 68fac4801e2e..2724c3aa512c 100644
--- a/drivers/input/serio/sa1111ps2.c
+++ b/drivers/input/serio/sa1111ps2.c
@@ -267,8 +267,8 @@ static int ps2_probe(struct sa1111_dev *dev)
serio->write = ps2_write;
serio->open = ps2_open;
serio->close = ps2_close;
- strlcpy(serio->name, dev_name(&dev->dev), sizeof(serio->name));
- strlcpy(serio->phys, dev_name(&dev->dev), sizeof(serio->phys));
+ strscpy(serio->name, dev_name(&dev->dev), sizeof(serio->name));
+ strscpy(serio->phys, dev_name(&dev->dev), sizeof(serio->phys));
serio->port_data = ps2if;
serio->dev.parent = &dev->dev;
ps2if->io = serio;
diff --git a/drivers/input/serio/serport.c b/drivers/input/serio/serport.c
index 7fbbe00e3553..50295b17cca4 100644
--- a/drivers/input/serio/serport.c
+++ b/drivers/input/serio/serport.c
@@ -171,7 +171,7 @@ static ssize_t serport_ldisc_read(struct tty_struct * tty, struct file * file,
if (!serio)
return -ENOMEM;

- strlcpy(serio->name, "Serial port", sizeof(serio->name));
+ strscpy(serio->name, "Serial port", sizeof(serio->name));
snprintf(serio->phys, sizeof(serio->phys), "%s/serio0", tty_name(tty));
serio->id = serport->id;
serio->id.type = SERIO_RS232;
diff --git a/drivers/input/serio/sun4i-ps2.c b/drivers/input/serio/sun4i-ps2.c
index f15ed3dcdb9b..eb262640192e 100644
--- a/drivers/input/serio/sun4i-ps2.c
+++ b/drivers/input/serio/sun4i-ps2.c
@@ -256,8 +256,8 @@ static int sun4i_ps2_probe(struct platform_device *pdev)
serio->close = sun4i_ps2_close;
serio->port_data = drvdata;
serio->dev.parent = dev;
- strlcpy(serio->name, dev_name(dev), sizeof(serio->name));
- strlcpy(serio->phys, dev_name(dev), sizeof(serio->phys));
+ strscpy(serio->name, dev_name(dev), sizeof(serio->name));
+ strscpy(serio->phys, dev_name(dev), sizeof(serio->phys));

/* shutoff interrupt */
writel(0, drvdata->reg_base + PS2_REG_GCTL);
--
2.25.1