* Re: [PATCH] Fix a potential abuse of seq_printf() format string in drivers
[not found] ` <CACRpkdZ0zwn0908LDqrfQJtF7M-WRcKA4qdJdwSXZNzm0L47CA@mail.gmail.com>
@ 2024-11-20 18:12 ` Kees Cook
2024-11-20 19:28 ` Greg Kroah-Hartman
0 siblings, 1 reply; 4+ 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] 4+ messages in thread
* Re: [PATCH] Fix a potential abuse of seq_printf() format string in drivers
2024-11-20 18:12 ` [PATCH] Fix a potential abuse of seq_printf() format string in drivers Kees Cook
@ 2024-11-20 19:28 ` Greg Kroah-Hartman
2024-11-22 4:38 ` Kees Cook
0 siblings, 1 reply; 4+ 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] 4+ 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; 4+ 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] 4+ 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; 4+ 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] 4+ messages in thread
end of thread, other threads:[~2024-11-22 14:35 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20241120053055.225195-1-00107082@163.com>
[not found] ` <CACRpkdZ0zwn0908LDqrfQJtF7M-WRcKA4qdJdwSXZNzm0L47CA@mail.gmail.com>
2024-11-20 18:12 ` [PATCH] Fix a potential abuse of seq_printf() format string in drivers 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
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox