[PATCH] input/serio: Prefer strscpy over strlcpy

From: Len Baker
Date: Tue Aug 17 2021 - 12:19:23 EST

Next message: Song Liu: "Re: [PATCH] bpf: Allow bpf_get_netns_cookie in BPF_PROG_TYPE_SOCK_OPS"
Previous message: gregkh@xxxxxxxxxxxxxxxxxxx: "Re: [PATCH] fs: sysfs: do not remove files if group is null"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

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 <len.baker@xxxxxxx>
---
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/YQbXiwie4YPzPWKK@xxxxxxxxxx/

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

Next message: Song Liu: "Re: [PATCH] bpf: Allow bpf_get_netns_cookie in BPF_PROG_TYPE_SOCK_OPS"
Previous message: gregkh@xxxxxxxxxxxxxxxxxxx: "Re: [PATCH] fs: sysfs: do not remove files if group is null"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]