* 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