* [PATCH] Fix a potential abuse of seq_printf() format string in drivers
@ 2024-11-20 5:30 David Wang
2024-11-20 7:35 ` Linus Walleij
2024-11-22 19:54 ` Thomas Gleixner
0 siblings, 2 replies; 8+ messages in thread
From: David Wang @ 2024-11-20 5:30 UTC (permalink / raw)
To: linus.walleij, brgl, tglx; +Cc: linux-kernel, linux-gpio, geert, David Wang
Using device name as format string of seq_printf() is proned to
"Format string attack", opens possibility for exploitation.
Seq_puts() is safer and more efficient.
Signed-off-by: David Wang <00107082@163.com>
---
drivers/gpio/gpio-aspeed-sgpio.c | 2 +-
drivers/gpio/gpio-aspeed.c | 2 +-
drivers/gpio/gpio-ep93xx.c | 2 +-
drivers/gpio/gpio-hlwd.c | 2 +-
drivers/gpio/gpio-mlxbf2.c | 2 +-
drivers/gpio/gpio-omap.c | 2 +-
drivers/gpio/gpio-pca953x.c | 2 +-
drivers/gpio/gpio-pl061.c | 2 +-
drivers/gpio/gpio-tegra.c | 2 +-
drivers/gpio/gpio-tegra186.c | 2 +-
drivers/gpio/gpio-tqmx86.c | 2 +-
drivers/gpio/gpio-visconti.c | 2 +-
drivers/gpio/gpio-xgs-iproc.c | 2 +-
drivers/irqchip/irq-gic.c | 2 +-
drivers/irqchip/irq-mvebu-pic.c | 2 +-
drivers/irqchip/irq-versatile-fpga.c | 2 +-
drivers/pinctrl/bcm/pinctrl-iproc-gpio.c | 2 +-
drivers/pinctrl/mvebu/pinctrl-armada-37xx.c | 2 +-
drivers/pinctrl/pinctrl-mcp23s08.c | 2 +-
drivers/pinctrl/pinctrl-stmfx.c | 2 +-
drivers/pinctrl/pinctrl-sx150x.c | 2 +-
drivers/pinctrl/renesas/pinctrl-rzg2l.c | 2 +-
22 files changed, 22 insertions(+), 22 deletions(-)
diff --git a/drivers/gpio/gpio-aspeed-sgpio.c b/drivers/gpio/gpio-aspeed-sgpio.c
index 72755fee6478..34eb26298e32 100644
--- a/drivers/gpio/gpio-aspeed-sgpio.c
+++ b/drivers/gpio/gpio-aspeed-sgpio.c
@@ -420,7 +420,7 @@ static void aspeed_sgpio_irq_print_chip(struct irq_data *d, struct seq_file *p)
int offset;
irqd_to_aspeed_sgpio_data(d, &gpio, &bank, &bit, &offset);
- seq_printf(p, dev_name(gpio->dev));
+ seq_puts(p, dev_name(gpio->dev));
}
static const struct irq_chip aspeed_sgpio_irq_chip = {
diff --git a/drivers/gpio/gpio-aspeed.c b/drivers/gpio/gpio-aspeed.c
index ea40ad43a79b..7f3292d9f016 100644
--- a/drivers/gpio/gpio-aspeed.c
+++ b/drivers/gpio/gpio-aspeed.c
@@ -1102,7 +1102,7 @@ static void aspeed_gpio_irq_print_chip(struct irq_data *d, struct seq_file *p)
if (rc)
return;
- seq_printf(p, dev_name(gpio->dev));
+ seq_puts(p, dev_name(gpio->dev));
}
static const struct irq_chip aspeed_gpio_irq_chip = {
diff --git a/drivers/gpio/gpio-ep93xx.c b/drivers/gpio/gpio-ep93xx.c
index ab798c848215..58d2464c07bc 100644
--- a/drivers/gpio/gpio-ep93xx.c
+++ b/drivers/gpio/gpio-ep93xx.c
@@ -249,7 +249,7 @@ static void ep93xx_irq_print_chip(struct irq_data *data, struct seq_file *p)
{
struct gpio_chip *gc = irq_data_get_irq_chip_data(data);
- seq_printf(p, dev_name(gc->parent));
+ seq_puts(p, dev_name(gc->parent));
}
static const struct irq_chip gpio_eic_irq_chip = {
diff --git a/drivers/gpio/gpio-hlwd.c b/drivers/gpio/gpio-hlwd.c
index 1bcfc1835dae..0580f6712bea 100644
--- a/drivers/gpio/gpio-hlwd.c
+++ b/drivers/gpio/gpio-hlwd.c
@@ -210,7 +210,7 @@ static void hlwd_gpio_irq_print_chip(struct irq_data *data, struct seq_file *p)
struct hlwd_gpio *hlwd =
gpiochip_get_data(irq_data_get_irq_chip_data(data));
- seq_printf(p, dev_name(hlwd->dev));
+ seq_puts(p, dev_name(hlwd->dev));
}
static const struct irq_chip hlwd_gpio_irq_chip = {
diff --git a/drivers/gpio/gpio-mlxbf2.c b/drivers/gpio/gpio-mlxbf2.c
index 6abe01bc39c3..6f3dda6b635f 100644
--- a/drivers/gpio/gpio-mlxbf2.c
+++ b/drivers/gpio/gpio-mlxbf2.c
@@ -331,7 +331,7 @@ static void mlxbf2_gpio_irq_print_chip(struct irq_data *irqd,
struct gpio_chip *gc = irq_data_get_irq_chip_data(irqd);
struct mlxbf2_gpio_context *gs = gpiochip_get_data(gc);
- seq_printf(p, dev_name(gs->dev));
+ seq_puts(p, dev_name(gs->dev));
}
static const struct irq_chip mlxbf2_gpio_irq_chip = {
diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c
index 76d5d87e9681..279524b640ae 100644
--- a/drivers/gpio/gpio-omap.c
+++ b/drivers/gpio/gpio-omap.c
@@ -715,7 +715,7 @@ static void omap_gpio_irq_print_chip(struct irq_data *d, struct seq_file *p)
{
struct gpio_bank *bank = omap_irq_data_get_bank(d);
- seq_printf(p, dev_name(bank->dev));
+ seq_puts(p, dev_name(bank->dev));
}
static const struct irq_chip omap_gpio_irq_chip = {
diff --git a/drivers/gpio/gpio-pca953x.c b/drivers/gpio/gpio-pca953x.c
index 3f2d33ee20cc..272febc3230e 100644
--- a/drivers/gpio/gpio-pca953x.c
+++ b/drivers/gpio/gpio-pca953x.c
@@ -815,7 +815,7 @@ static void pca953x_irq_print_chip(struct irq_data *data, struct seq_file *p)
{
struct gpio_chip *gc = irq_data_get_irq_chip_data(data);
- seq_printf(p, dev_name(gc->parent));
+ seq_puts(p, dev_name(gc->parent));
}
static const struct irq_chip pca953x_irq_chip = {
diff --git a/drivers/gpio/gpio-pl061.c b/drivers/gpio/gpio-pl061.c
index a211a02d4b4a..1c273727ffa3 100644
--- a/drivers/gpio/gpio-pl061.c
+++ b/drivers/gpio/gpio-pl061.c
@@ -291,7 +291,7 @@ static void pl061_irq_print_chip(struct irq_data *data, struct seq_file *p)
{
struct gpio_chip *gc = irq_data_get_irq_chip_data(data);
- seq_printf(p, dev_name(gc->parent));
+ seq_puts(p, dev_name(gc->parent));
}
static const struct irq_chip pl061_irq_chip = {
diff --git a/drivers/gpio/gpio-tegra.c b/drivers/gpio/gpio-tegra.c
index 6d3a39a03f58..9ad286adf263 100644
--- a/drivers/gpio/gpio-tegra.c
+++ b/drivers/gpio/gpio-tegra.c
@@ -600,7 +600,7 @@ static void tegra_gpio_irq_print_chip(struct irq_data *d, struct seq_file *s)
{
struct gpio_chip *chip = irq_data_get_irq_chip_data(d);
- seq_printf(s, dev_name(chip->parent));
+ seq_puts(s, dev_name(chip->parent));
}
static const struct irq_chip tegra_gpio_irq_chip = {
diff --git a/drivers/gpio/gpio-tegra186.c b/drivers/gpio/gpio-tegra186.c
index 1ecb733a5e88..6895b65c86af 100644
--- a/drivers/gpio/gpio-tegra186.c
+++ b/drivers/gpio/gpio-tegra186.c
@@ -610,7 +610,7 @@ static void tegra186_irq_print_chip(struct irq_data *data, struct seq_file *p)
{
struct gpio_chip *gc = irq_data_get_irq_chip_data(data);
- seq_printf(p, dev_name(gc->parent));
+ seq_puts(p, dev_name(gc->parent));
}
static const struct irq_chip tegra186_gpio_irq_chip = {
diff --git a/drivers/gpio/gpio-tqmx86.c b/drivers/gpio/gpio-tqmx86.c
index f2e7e8754d95..5e26eb3adabb 100644
--- a/drivers/gpio/gpio-tqmx86.c
+++ b/drivers/gpio/gpio-tqmx86.c
@@ -275,7 +275,7 @@ static void tqmx86_gpio_irq_print_chip(struct irq_data *d, struct seq_file *p)
{
struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
- seq_printf(p, gc->label);
+ seq_puts(p, gc->label);
}
static const struct irq_chip tqmx86_gpio_irq_chip = {
diff --git a/drivers/gpio/gpio-visconti.c b/drivers/gpio/gpio-visconti.c
index ebc71ecdb6cf..5bd965c18a46 100644
--- a/drivers/gpio/gpio-visconti.c
+++ b/drivers/gpio/gpio-visconti.c
@@ -142,7 +142,7 @@ static void visconti_gpio_irq_print_chip(struct irq_data *d, struct seq_file *p)
struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
struct visconti_gpio *priv = gpiochip_get_data(gc);
- seq_printf(p, dev_name(priv->dev));
+ seq_puts(p, dev_name(priv->dev));
}
static const struct irq_chip visconti_gpio_irq_chip = {
diff --git a/drivers/gpio/gpio-xgs-iproc.c b/drivers/gpio/gpio-xgs-iproc.c
index d445eea03687..e9390f136b3c 100644
--- a/drivers/gpio/gpio-xgs-iproc.c
+++ b/drivers/gpio/gpio-xgs-iproc.c
@@ -198,7 +198,7 @@ static void iproc_gpio_irq_print_chip(struct irq_data *d, struct seq_file *p)
struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
struct iproc_gpio_chip *chip = to_iproc_gpio(gc);
- seq_printf(p, dev_name(chip->dev));
+ seq_puts(p, dev_name(chip->dev));
}
static const struct irq_chip iproc_gpio_irq_chip = {
diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c
index 3be7bd8cd8cd..8fae6dc01024 100644
--- a/drivers/irqchip/irq-gic.c
+++ b/drivers/irqchip/irq-gic.c
@@ -400,7 +400,7 @@ static void gic_irq_print_chip(struct irq_data *d, struct seq_file *p)
struct gic_chip_data *gic = irq_data_get_irq_chip_data(d);
if (gic->domain->pm_dev)
- seq_printf(p, gic->domain->pm_dev->of_node->name);
+ seq_puts(p, gic->domain->pm_dev->of_node->name);
else
seq_printf(p, "GIC-%d", (int)(gic - &gic_data[0]));
}
diff --git a/drivers/irqchip/irq-mvebu-pic.c b/drivers/irqchip/irq-mvebu-pic.c
index 08b0cc862adf..b815a60f930c 100644
--- a/drivers/irqchip/irq-mvebu-pic.c
+++ b/drivers/irqchip/irq-mvebu-pic.c
@@ -71,7 +71,7 @@ static void mvebu_pic_print_chip(struct irq_data *d, struct seq_file *p)
{
struct mvebu_pic *pic = irq_data_get_irq_chip_data(d);
- seq_printf(p, dev_name(&pic->pdev->dev));
+ seq_puts(p, dev_name(&pic->pdev->dev));
}
static const struct irq_chip mvebu_pic_chip = {
diff --git a/drivers/irqchip/irq-versatile-fpga.c b/drivers/irqchip/irq-versatile-fpga.c
index ca471c6fee99..0abc8934c2ee 100644
--- a/drivers/irqchip/irq-versatile-fpga.c
+++ b/drivers/irqchip/irq-versatile-fpga.c
@@ -69,7 +69,7 @@ static void fpga_irq_print_chip(struct irq_data *d, struct seq_file *p)
{
struct fpga_irq_data *f = irq_data_get_irq_chip_data(d);
- seq_printf(p, irq_domain_get_of_node(f->domain)->name);
+ seq_puts(p, irq_domain_get_of_node(f->domain)->name);
}
static const struct irq_chip fpga_chip = {
diff --git a/drivers/pinctrl/bcm/pinctrl-iproc-gpio.c b/drivers/pinctrl/bcm/pinctrl-iproc-gpio.c
index fd5ce52d05b1..c9a3d3aa8c10 100644
--- a/drivers/pinctrl/bcm/pinctrl-iproc-gpio.c
+++ b/drivers/pinctrl/bcm/pinctrl-iproc-gpio.c
@@ -309,7 +309,7 @@ static void iproc_gpio_irq_print_chip(struct irq_data *d, struct seq_file *p)
struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
struct iproc_gpio *chip = gpiochip_get_data(gc);
- seq_printf(p, dev_name(chip->dev));
+ seq_puts(p, dev_name(chip->dev));
}
static const struct irq_chip iproc_gpio_irq_chip = {
diff --git a/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c b/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c
index 4c4ada06423d..335744ac8310 100644
--- a/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c
+++ b/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c
@@ -734,7 +734,7 @@ static void armada_37xx_irq_print_chip(struct irq_data *d, struct seq_file *p)
struct gpio_chip *chip = irq_data_get_irq_chip_data(d);
struct armada_37xx_pinctrl *info = gpiochip_get_data(chip);
- seq_printf(p, info->data->name);
+ seq_puts(p, info->data->name);
}
static const struct irq_chip armada_37xx_irqchip = {
diff --git a/drivers/pinctrl/pinctrl-mcp23s08.c b/drivers/pinctrl/pinctrl-mcp23s08.c
index 737d0ae3d0b6..d66c3a3e8429 100644
--- a/drivers/pinctrl/pinctrl-mcp23s08.c
+++ b/drivers/pinctrl/pinctrl-mcp23s08.c
@@ -569,7 +569,7 @@ static void mcp23s08_irq_print_chip(struct irq_data *d, struct seq_file *p)
struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
struct mcp23s08 *mcp = gpiochip_get_data(gc);
- seq_printf(p, dev_name(mcp->dev));
+ seq_puts(p, dev_name(mcp->dev));
}
static const struct irq_chip mcp23s08_irq_chip = {
diff --git a/drivers/pinctrl/pinctrl-stmfx.c b/drivers/pinctrl/pinctrl-stmfx.c
index d2c5321dd025..31d68183b743 100644
--- a/drivers/pinctrl/pinctrl-stmfx.c
+++ b/drivers/pinctrl/pinctrl-stmfx.c
@@ -599,7 +599,7 @@ static void stmfx_pinctrl_irq_print_chip(struct irq_data *d, struct seq_file *p)
struct gpio_chip *gpio_chip = irq_data_get_irq_chip_data(d);
struct stmfx_pinctrl *pctl = gpiochip_get_data(gpio_chip);
- seq_printf(p, dev_name(pctl->dev));
+ seq_puts(p, dev_name(pctl->dev));
}
static const struct irq_chip stmfx_pinctrl_irq_chip = {
diff --git a/drivers/pinctrl/pinctrl-sx150x.c b/drivers/pinctrl/pinctrl-sx150x.c
index fd0331a87cda..dbe14566e1f3 100644
--- a/drivers/pinctrl/pinctrl-sx150x.c
+++ b/drivers/pinctrl/pinctrl-sx150x.c
@@ -584,7 +584,7 @@ static void sx150x_irq_print_chip(struct irq_data *d, struct seq_file *p)
struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
struct sx150x_pinctrl *pctl = gpiochip_get_data(gc);
- seq_printf(p, pctl->client->name);
+ seq_puts(p, pctl->client->name);
}
static const struct irq_chip sx150x_irq_chip = {
diff --git a/drivers/pinctrl/renesas/pinctrl-rzg2l.c b/drivers/pinctrl/renesas/pinctrl-rzg2l.c
index 5a403915fed2..8742b440339e 100644
--- a/drivers/pinctrl/renesas/pinctrl-rzg2l.c
+++ b/drivers/pinctrl/renesas/pinctrl-rzg2l.c
@@ -2290,7 +2290,7 @@ static void rzg2l_gpio_irq_print_chip(struct irq_data *data, struct seq_file *p)
{
struct gpio_chip *gc = irq_data_get_irq_chip_data(data);
- seq_printf(p, dev_name(gc->parent));
+ seq_puts(p, dev_name(gc->parent));
}
static int rzg2l_gpio_irq_set_wake(struct irq_data *data, unsigned int on)
--
2.39.2
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] Fix a potential abuse of seq_printf() format string in drivers
2024-11-20 5:30 [PATCH] Fix a potential abuse of seq_printf() format string in drivers David Wang
@ 2024-11-20 7:35 ` Linus Walleij
2024-11-20 9:00 ` David Wang
2024-11-20 18:12 ` Kees Cook
2024-11-22 19:54 ` Thomas Gleixner
1 sibling, 2 replies; 8+ messages in thread
From: Linus Walleij @ 2024-11-20 7:35 UTC (permalink / raw)
To: David Wang, Kees Cook; +Cc: brgl, tglx, linux-kernel, linux-gpio, geert
On Wed, Nov 20, 2024 at 6:31 AM David Wang <00107082@163.com> wrote:
> Using device name as format string of seq_printf() is proned to
> "Format string attack", opens possibility for exploitation.
> Seq_puts() is safer and more efficient.
>
> Signed-off-by: David Wang <00107082@163.com>
Okay better get Kees' eye on this, he looks after string vulnerabilities.
(But I think you're right.)
> drivers/gpio/gpio-aspeed-sgpio.c | 2 +-
> drivers/gpio/gpio-aspeed.c | 2 +-
> drivers/gpio/gpio-ep93xx.c | 2 +-
> drivers/gpio/gpio-hlwd.c | 2 +-
> drivers/gpio/gpio-mlxbf2.c | 2 +-
> drivers/gpio/gpio-omap.c | 2 +-
> drivers/gpio/gpio-pca953x.c | 2 +-
> drivers/gpio/gpio-pl061.c | 2 +-
> drivers/gpio/gpio-tegra.c | 2 +-
> drivers/gpio/gpio-tegra186.c | 2 +-
> drivers/gpio/gpio-tqmx86.c | 2 +-
> drivers/gpio/gpio-visconti.c | 2 +-
> drivers/gpio/gpio-xgs-iproc.c | 2 +-
> drivers/irqchip/irq-gic.c | 2 +-
> drivers/irqchip/irq-mvebu-pic.c | 2 +-
> drivers/irqchip/irq-versatile-fpga.c | 2 +-
> drivers/pinctrl/bcm/pinctrl-iproc-gpio.c | 2 +-
> drivers/pinctrl/mvebu/pinctrl-armada-37xx.c | 2 +-
> drivers/pinctrl/pinctrl-mcp23s08.c | 2 +-
> drivers/pinctrl/pinctrl-stmfx.c | 2 +-
> drivers/pinctrl/pinctrl-sx150x.c | 2 +-
> drivers/pinctrl/renesas/pinctrl-rzg2l.c | 2 +-
Can you split this in three patches per-subsystem?
One for gpio, one for irqchip and one for pinctrl?
Then send to each subsystem maintainer and CC kees on
each.
I'm just the pinctrl maintainer. The rest can be found with
scripts/get_maintainer.pl.
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] Fix a potential abuse of seq_printf() format string in drivers
2024-11-20 7:35 ` Linus Walleij
@ 2024-11-20 9:00 ` David Wang
2024-11-20 18:12 ` Kees Cook
1 sibling, 0 replies; 8+ messages in thread
From: David Wang @ 2024-11-20 9:00 UTC (permalink / raw)
To: Linus Walleij; +Cc: Kees Cook, brgl, tglx, linux-kernel, linux-gpio, geert
At 2024-11-20 15:35:38, "Linus Walleij" <linus.walleij@linaro.org> wrote:
>On Wed, Nov 20, 2024 at 6:31 AM David Wang <00107082@163.com> wrote:
>
>> Using device name as format string of seq_printf() is proned to
>> "Format string attack", opens possibility for exploitation.
>> Seq_puts() is safer and more efficient.
>>
>> Signed-off-by: David Wang <00107082@163.com>
>
>Okay better get Kees' eye on this, he looks after string vulnerabilities.
>(But I think you're right.)
>
>> drivers/gpio/gpio-aspeed-sgpio.c | 2 +-
>> drivers/gpio/gpio-aspeed.c | 2 +-
>> drivers/gpio/gpio-ep93xx.c | 2 +-
>> drivers/gpio/gpio-hlwd.c | 2 +-
>> drivers/gpio/gpio-mlxbf2.c | 2 +-
>> drivers/gpio/gpio-omap.c | 2 +-
>> drivers/gpio/gpio-pca953x.c | 2 +-
>> drivers/gpio/gpio-pl061.c | 2 +-
>> drivers/gpio/gpio-tegra.c | 2 +-
>> drivers/gpio/gpio-tegra186.c | 2 +-
>> drivers/gpio/gpio-tqmx86.c | 2 +-
>> drivers/gpio/gpio-visconti.c | 2 +-
>> drivers/gpio/gpio-xgs-iproc.c | 2 +-
>> drivers/irqchip/irq-gic.c | 2 +-
>> drivers/irqchip/irq-mvebu-pic.c | 2 +-
>> drivers/irqchip/irq-versatile-fpga.c | 2 +-
>> drivers/pinctrl/bcm/pinctrl-iproc-gpio.c | 2 +-
>> drivers/pinctrl/mvebu/pinctrl-armada-37xx.c | 2 +-
>> drivers/pinctrl/pinctrl-mcp23s08.c | 2 +-
>> drivers/pinctrl/pinctrl-stmfx.c | 2 +-
>> drivers/pinctrl/pinctrl-sx150x.c | 2 +-
>> drivers/pinctrl/renesas/pinctrl-rzg2l.c | 2 +-
>
>Can you split this in three patches per-subsystem?
>One for gpio, one for irqchip and one for pinctrl?
>
>Then send to each subsystem maintainer and CC kees on
>each.
>
>I'm just the pinctrl maintainer. The rest can be found with
>scripts/get_maintainer.pl.
Thanks for the review, I will send a patchset later.
>
>Yours,
>Linus Walleij
Thanks
David
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] Fix a potential abuse of seq_printf() format string in drivers
2024-11-20 7:35 ` Linus Walleij
2024-11-20 9:00 ` David Wang
@ 2024-11-20 18:12 ` Kees Cook
2024-11-20 19:28 ` Greg Kroah-Hartman
1 sibling, 1 reply; 8+ messages in thread
From: Kees Cook @ 2024-11-20 18:12 UTC (permalink / raw)
To: Linus Walleij
Cc: David Wang, brgl, tglx, linux-kernel, linux-gpio, geert,
linux-hardening, Greg Kroah-Hartman, Rafael J. Wysocki
On Wed, Nov 20, 2024 at 08:35:38AM +0100, Linus Walleij wrote:
> On Wed, Nov 20, 2024 at 6:31 AM David Wang <00107082@163.com> wrote:
>
> > Using device name as format string of seq_printf() is proned to
> > "Format string attack", opens possibility for exploitation.
> > Seq_puts() is safer and more efficient.
> >
> > Signed-off-by: David Wang <00107082@163.com>
>
> Okay better get Kees' eye on this, he looks after string vulnerabilities.
> (But I think you're right.)
Agreed, this may lead to kernel memory content exposures. seq_puts()
looks right.
Reviewed-by: Kees Cook <kees@kernel.org>
To defend against this, it might be interesting to detect
single-argument seq_printf() usage and aim it at seq_puts()
automatically...
>
> > drivers/gpio/gpio-aspeed-sgpio.c | 2 +-
> > drivers/gpio/gpio-aspeed.c | 2 +-
> > drivers/gpio/gpio-ep93xx.c | 2 +-
> > drivers/gpio/gpio-hlwd.c | 2 +-
> > drivers/gpio/gpio-mlxbf2.c | 2 +-
> > drivers/gpio/gpio-omap.c | 2 +-
> > drivers/gpio/gpio-pca953x.c | 2 +-
> > drivers/gpio/gpio-pl061.c | 2 +-
> > drivers/gpio/gpio-tegra.c | 2 +-
> > drivers/gpio/gpio-tegra186.c | 2 +-
> > drivers/gpio/gpio-tqmx86.c | 2 +-
> > drivers/gpio/gpio-visconti.c | 2 +-
> > drivers/gpio/gpio-xgs-iproc.c | 2 +-
> > drivers/irqchip/irq-gic.c | 2 +-
> > drivers/irqchip/irq-mvebu-pic.c | 2 +-
> > drivers/irqchip/irq-versatile-fpga.c | 2 +-
> > drivers/pinctrl/bcm/pinctrl-iproc-gpio.c | 2 +-
> > drivers/pinctrl/mvebu/pinctrl-armada-37xx.c | 2 +-
> > drivers/pinctrl/pinctrl-mcp23s08.c | 2 +-
> > drivers/pinctrl/pinctrl-stmfx.c | 2 +-
> > drivers/pinctrl/pinctrl-sx150x.c | 2 +-
> > drivers/pinctrl/renesas/pinctrl-rzg2l.c | 2 +-
>
> Can you split this in three patches per-subsystem?
> One for gpio, one for irqchip and one for pinctrl?
>
> Then send to each subsystem maintainer and CC kees on
> each.
>
> I'm just the pinctrl maintainer. The rest can be found with
> scripts/get_maintainer.pl.
Oof. That's a lot of work for a mechanical change like this. Perhaps
Greg KH can take it directly to the drivers tree instead?
-Kees
--
Kees Cook
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] Fix a potential abuse of seq_printf() format string in drivers
2024-11-20 18:12 ` Kees Cook
@ 2024-11-20 19:28 ` Greg Kroah-Hartman
2024-11-22 4:38 ` Kees Cook
0 siblings, 1 reply; 8+ messages in thread
From: Greg Kroah-Hartman @ 2024-11-20 19:28 UTC (permalink / raw)
To: Kees Cook
Cc: Linus Walleij, David Wang, brgl, tglx, linux-kernel, linux-gpio,
geert, linux-hardening, Rafael J. Wysocki
On Wed, Nov 20, 2024 at 10:12:40AM -0800, Kees Cook wrote:
> On Wed, Nov 20, 2024 at 08:35:38AM +0100, Linus Walleij wrote:
> > On Wed, Nov 20, 2024 at 6:31 AM David Wang <00107082@163.com> wrote:
> >
> > > Using device name as format string of seq_printf() is proned to
> > > "Format string attack", opens possibility for exploitation.
> > > Seq_puts() is safer and more efficient.
> > >
> > > Signed-off-by: David Wang <00107082@163.com>
> >
> > Okay better get Kees' eye on this, he looks after string vulnerabilities.
> > (But I think you're right.)
>
> Agreed, this may lead to kernel memory content exposures. seq_puts()
> looks right.
>
> Reviewed-by: Kees Cook <kees@kernel.org>
Wait, userspace "shouldn't" be controlling a device name, but odds are
there are some paths/subsystems that do this, ugh.
> To defend against this, it might be interesting to detect
> single-argument seq_printf() usage and aim it at seq_puts()
> automatically...
Yeah, that would be good to squash this type of issue.
> > > drivers/gpio/gpio-aspeed-sgpio.c | 2 +-
> > > drivers/gpio/gpio-aspeed.c | 2 +-
> > > drivers/gpio/gpio-ep93xx.c | 2 +-
> > > drivers/gpio/gpio-hlwd.c | 2 +-
> > > drivers/gpio/gpio-mlxbf2.c | 2 +-
> > > drivers/gpio/gpio-omap.c | 2 +-
> > > drivers/gpio/gpio-pca953x.c | 2 +-
> > > drivers/gpio/gpio-pl061.c | 2 +-
> > > drivers/gpio/gpio-tegra.c | 2 +-
> > > drivers/gpio/gpio-tegra186.c | 2 +-
> > > drivers/gpio/gpio-tqmx86.c | 2 +-
> > > drivers/gpio/gpio-visconti.c | 2 +-
> > > drivers/gpio/gpio-xgs-iproc.c | 2 +-
> > > drivers/irqchip/irq-gic.c | 2 +-
> > > drivers/irqchip/irq-mvebu-pic.c | 2 +-
> > > drivers/irqchip/irq-versatile-fpga.c | 2 +-
> > > drivers/pinctrl/bcm/pinctrl-iproc-gpio.c | 2 +-
> > > drivers/pinctrl/mvebu/pinctrl-armada-37xx.c | 2 +-
> > > drivers/pinctrl/pinctrl-mcp23s08.c | 2 +-
> > > drivers/pinctrl/pinctrl-stmfx.c | 2 +-
> > > drivers/pinctrl/pinctrl-sx150x.c | 2 +-
> > > drivers/pinctrl/renesas/pinctrl-rzg2l.c | 2 +-
> >
> > Can you split this in three patches per-subsystem?
> > One for gpio, one for irqchip and one for pinctrl?
> >
> > Then send to each subsystem maintainer and CC kees on
> > each.
> >
> > I'm just the pinctrl maintainer. The rest can be found with
> > scripts/get_maintainer.pl.
>
> Oof. That's a lot of work for a mechanical change like this. Perhaps
> Greg KH can take it directly to the drivers tree instead?
I can take it all, as-is, right now, if you want me to. Just let me
know.
thanks,
greg k-h
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] Fix a potential abuse of seq_printf() format string in drivers
2024-11-20 19:28 ` Greg Kroah-Hartman
@ 2024-11-22 4:38 ` Kees Cook
2024-11-22 14:35 ` Greg Kroah-Hartman
0 siblings, 1 reply; 8+ messages in thread
From: Kees Cook @ 2024-11-22 4:38 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Linus Walleij, David Wang, brgl, tglx, linux-kernel, linux-gpio,
geert, linux-hardening, Rafael J. Wysocki
On November 20, 2024 11:28:35 AM PST, Greg Kroah-Hartman <gregkh@linuxfoundation.org> wrote:
>On Wed, Nov 20, 2024 at 10:12:40AM -0800, Kees Cook wrote:
>> On Wed, Nov 20, 2024 at 08:35:38AM +0100, Linus Walleij wrote:
>> > On Wed, Nov 20, 2024 at 6:31 AM David Wang <00107082@163.com> wrote:
>> >
>> > > Using device name as format string of seq_printf() is proned to
>> > > "Format string attack", opens possibility for exploitation.
>> > > Seq_puts() is safer and more efficient.
>> > >
>> > > Signed-off-by: David Wang <00107082@163.com>
>> >
>> > Okay better get Kees' eye on this, he looks after string vulnerabilities.
>> > (But I think you're right.)
>>
>> Agreed, this may lead to kernel memory content exposures. seq_puts()
>> looks right.
>>
>> Reviewed-by: Kees Cook <kees@kernel.org>
>
>Wait, userspace "shouldn't" be controlling a device name, but odds are
>there are some paths/subsystems that do this, ugh.
>
>> To defend against this, it might be interesting to detect
>> single-argument seq_printf() usage and aim it at seq_puts()
>> automatically...
>
>Yeah, that would be good to squash this type of issue.
>
>> > > drivers/gpio/gpio-aspeed-sgpio.c | 2 +-
>> > > drivers/gpio/gpio-aspeed.c | 2 +-
>> > > drivers/gpio/gpio-ep93xx.c | 2 +-
>> > > drivers/gpio/gpio-hlwd.c | 2 +-
>> > > drivers/gpio/gpio-mlxbf2.c | 2 +-
>> > > drivers/gpio/gpio-omap.c | 2 +-
>> > > drivers/gpio/gpio-pca953x.c | 2 +-
>> > > drivers/gpio/gpio-pl061.c | 2 +-
>> > > drivers/gpio/gpio-tegra.c | 2 +-
>> > > drivers/gpio/gpio-tegra186.c | 2 +-
>> > > drivers/gpio/gpio-tqmx86.c | 2 +-
>> > > drivers/gpio/gpio-visconti.c | 2 +-
>> > > drivers/gpio/gpio-xgs-iproc.c | 2 +-
>> > > drivers/irqchip/irq-gic.c | 2 +-
>> > > drivers/irqchip/irq-mvebu-pic.c | 2 +-
>> > > drivers/irqchip/irq-versatile-fpga.c | 2 +-
>> > > drivers/pinctrl/bcm/pinctrl-iproc-gpio.c | 2 +-
>> > > drivers/pinctrl/mvebu/pinctrl-armada-37xx.c | 2 +-
>> > > drivers/pinctrl/pinctrl-mcp23s08.c | 2 +-
>> > > drivers/pinctrl/pinctrl-stmfx.c | 2 +-
>> > > drivers/pinctrl/pinctrl-sx150x.c | 2 +-
>> > > drivers/pinctrl/renesas/pinctrl-rzg2l.c | 2 +-
>> >
>> > Can you split this in three patches per-subsystem?
>> > One for gpio, one for irqchip and one for pinctrl?
>> >
>> > Then send to each subsystem maintainer and CC kees on
>> > each.
>> >
>> > I'm just the pinctrl maintainer. The rest can be found with
>> > scripts/get_maintainer.pl.
>>
>> Oof. That's a lot of work for a mechanical change like this. Perhaps
>> Greg KH can take it directly to the drivers tree instead?
>
>I can take it all, as-is, right now, if you want me to. Just let me
>know.
Yes, please do. I will send a patch for making seq_printf more defensive separately.
Thanks!
-Kees
--
Kees Cook
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] Fix a potential abuse of seq_printf() format string in drivers
2024-11-22 4:38 ` Kees Cook
@ 2024-11-22 14:35 ` Greg Kroah-Hartman
0 siblings, 0 replies; 8+ messages in thread
From: Greg Kroah-Hartman @ 2024-11-22 14:35 UTC (permalink / raw)
To: Kees Cook
Cc: Linus Walleij, David Wang, brgl, tglx, linux-kernel, linux-gpio,
geert, linux-hardening, Rafael J. Wysocki
On Thu, Nov 21, 2024 at 08:38:27PM -0800, Kees Cook wrote:
>
>
> On November 20, 2024 11:28:35 AM PST, Greg Kroah-Hartman <gregkh@linuxfoundation.org> wrote:
> >On Wed, Nov 20, 2024 at 10:12:40AM -0800, Kees Cook wrote:
> >> On Wed, Nov 20, 2024 at 08:35:38AM +0100, Linus Walleij wrote:
> >> > On Wed, Nov 20, 2024 at 6:31 AM David Wang <00107082@163.com> wrote:
> >> >
> >> > > Using device name as format string of seq_printf() is proned to
> >> > > "Format string attack", opens possibility for exploitation.
> >> > > Seq_puts() is safer and more efficient.
> >> > >
> >> > > Signed-off-by: David Wang <00107082@163.com>
> >> >
> >> > Okay better get Kees' eye on this, he looks after string vulnerabilities.
> >> > (But I think you're right.)
> >>
> >> Agreed, this may lead to kernel memory content exposures. seq_puts()
> >> looks right.
> >>
> >> Reviewed-by: Kees Cook <kees@kernel.org>
> >
> >Wait, userspace "shouldn't" be controlling a device name, but odds are
> >there are some paths/subsystems that do this, ugh.
> >
> >> To defend against this, it might be interesting to detect
> >> single-argument seq_printf() usage and aim it at seq_puts()
> >> automatically...
> >
> >Yeah, that would be good to squash this type of issue.
> >
> >> > > drivers/gpio/gpio-aspeed-sgpio.c | 2 +-
> >> > > drivers/gpio/gpio-aspeed.c | 2 +-
> >> > > drivers/gpio/gpio-ep93xx.c | 2 +-
> >> > > drivers/gpio/gpio-hlwd.c | 2 +-
> >> > > drivers/gpio/gpio-mlxbf2.c | 2 +-
> >> > > drivers/gpio/gpio-omap.c | 2 +-
> >> > > drivers/gpio/gpio-pca953x.c | 2 +-
> >> > > drivers/gpio/gpio-pl061.c | 2 +-
> >> > > drivers/gpio/gpio-tegra.c | 2 +-
> >> > > drivers/gpio/gpio-tegra186.c | 2 +-
> >> > > drivers/gpio/gpio-tqmx86.c | 2 +-
> >> > > drivers/gpio/gpio-visconti.c | 2 +-
> >> > > drivers/gpio/gpio-xgs-iproc.c | 2 +-
> >> > > drivers/irqchip/irq-gic.c | 2 +-
> >> > > drivers/irqchip/irq-mvebu-pic.c | 2 +-
> >> > > drivers/irqchip/irq-versatile-fpga.c | 2 +-
> >> > > drivers/pinctrl/bcm/pinctrl-iproc-gpio.c | 2 +-
> >> > > drivers/pinctrl/mvebu/pinctrl-armada-37xx.c | 2 +-
> >> > > drivers/pinctrl/pinctrl-mcp23s08.c | 2 +-
> >> > > drivers/pinctrl/pinctrl-stmfx.c | 2 +-
> >> > > drivers/pinctrl/pinctrl-sx150x.c | 2 +-
> >> > > drivers/pinctrl/renesas/pinctrl-rzg2l.c | 2 +-
> >> >
> >> > Can you split this in three patches per-subsystem?
> >> > One for gpio, one for irqchip and one for pinctrl?
> >> >
> >> > Then send to each subsystem maintainer and CC kees on
> >> > each.
> >> >
> >> > I'm just the pinctrl maintainer. The rest can be found with
> >> > scripts/get_maintainer.pl.
> >>
> >> Oof. That's a lot of work for a mechanical change like this. Perhaps
> >> Greg KH can take it directly to the drivers tree instead?
> >
> >I can take it all, as-is, right now, if you want me to. Just let me
> >know.
>
> Yes, please do. I will send a patch for making seq_printf more defensive separately.
Great, now queued up, let's make sure 0-day is ok with it...
greg k-h
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] Fix a potential abuse of seq_printf() format string in drivers
2024-11-20 5:30 [PATCH] Fix a potential abuse of seq_printf() format string in drivers David Wang
2024-11-20 7:35 ` Linus Walleij
@ 2024-11-22 19:54 ` Thomas Gleixner
1 sibling, 0 replies; 8+ messages in thread
From: Thomas Gleixner @ 2024-11-22 19:54 UTC (permalink / raw)
To: David Wang, linus.walleij, brgl
Cc: linux-kernel, linux-gpio, geert, David Wang
On Wed, Nov 20 2024 at 13:30, David Wang wrote:
> Using device name as format string of seq_printf() is proned to
> "Format string attack", opens possibility for exploitation.
> Seq_puts() is safer and more efficient.
>
> Signed-off-by: David Wang <00107082@163.com>
> ---
> drivers/gpio/gpio-aspeed-sgpio.c | 2 +-
> drivers/gpio/gpio-aspeed.c | 2 +-
> drivers/gpio/gpio-ep93xx.c | 2 +-
> drivers/gpio/gpio-hlwd.c | 2 +-
> drivers/gpio/gpio-mlxbf2.c | 2 +-
> drivers/gpio/gpio-omap.c | 2 +-
> drivers/gpio/gpio-pca953x.c | 2 +-
> drivers/gpio/gpio-pl061.c | 2 +-
> drivers/gpio/gpio-tegra.c | 2 +-
> drivers/gpio/gpio-tegra186.c | 2 +-
> drivers/gpio/gpio-tqmx86.c | 2 +-
> drivers/gpio/gpio-visconti.c | 2 +-
> drivers/gpio/gpio-xgs-iproc.c | 2 +-
> drivers/irqchip/irq-gic.c | 2 +-
> drivers/irqchip/irq-mvebu-pic.c | 2 +-
> drivers/irqchip/irq-versatile-fpga.c | 2 +-
> drivers/pinctrl/bcm/pinctrl-iproc-gpio.c | 2 +-
> drivers/pinctrl/mvebu/pinctrl-armada-37xx.c | 2 +-
> drivers/pinctrl/pinctrl-mcp23s08.c | 2 +-
> drivers/pinctrl/pinctrl-stmfx.c | 2 +-
> drivers/pinctrl/pinctrl-sx150x.c | 2 +-
> drivers/pinctrl/renesas/pinctrl-rzg2l.c | 2 +-
Can you please split these patches per subsystem (gpio, irqchip,
pinctrl) so that the respective maintainers can pick them up
individually?
Thanks,
tglx
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2024-11-22 19:54 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-11-20 5:30 [PATCH] Fix a potential abuse of seq_printf() format string in drivers David Wang
2024-11-20 7:35 ` Linus Walleij
2024-11-20 9:00 ` David Wang
2024-11-20 18:12 ` Kees Cook
2024-11-20 19:28 ` Greg Kroah-Hartman
2024-11-22 4:38 ` Kees Cook
2024-11-22 14:35 ` Greg Kroah-Hartman
2024-11-22 19:54 ` Thomas Gleixner
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).