From: Mark Brown <broonie@kernel.org> To: Joe Burmeister <joe.burmeister@devtank.co.uk> Cc: Florian Fainelli <f.fainelli@gmail.com>, Ray Jui <rjui@broadcom.com>, Scott Branden <sbranden@broadcom.com>, bcm-kernel-feedback-list@broadcom.com, linux-spi@vger.kernel.org, linux-rpi-kernel@lists.infradead.org, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, nsaenz@kernel.org Subject: Re: [PATCH] spi: bcm2835: Fix buffer overflow with CS able to go beyond limit. Date: Fri, 23 Apr 2021 12:57:24 +0100 [thread overview] Message-ID: <20210423115724.GB5507@sirena.org.uk> (raw) In-Reply-To: <380624c4-82f3-0e6e-8cdb-8a9732636db8@devtank.co.uk> [-- Attachment #1: Type: text/plain, Size: 1909 bytes --] On Fri, Apr 23, 2021 at 11:03:22AM +0100, Joe Burmeister wrote: > On 23/04/2021 00:49, Florian Fainelli wrote: > > Right, which means that we should probably seek a solution within the > > SPI core itself, even if you can only test with spi-bcm2835.c chances > > are that the fix would be applicable for other controllers if done in > > the core. > I'm not sure it's possible to do in the core alone. The numb of the > issue is the core changes ctlr->num_chipselect to what is in the device > tree and some drivers are cool with that overs quietly stomp memory. I wouldn't expect any controller to be OK with that? Drivers can store per-client data in spi_device->controller_data which doesn't need scaling (but is also not so helpful if you need to look at clients other than the one you're currently controlling). > I've got a simple little patch to warn when the core expands > ctlr->num_chipselect. This warning won't go off in bcm2835 with my patch > because I am also extending ctlr->num_chipselect to the amount in the > device tree before the core does that expansion. Hopefully that new > warning would make people investigate and fix problem drivers. > >> There is protection in spi_add_device, which will catch extra added > >> later, but not ones in the device tree when the spi controller was > >> registered. > > Not sure I follow you, if we have the overlay before > > spi_register_controller() is called, how can the check there not > > trigger? And if we load the overlay later when the SPI controller is > > already registered, why does not spi_add_device()'s check work? > I think it might be a RPI thing. I think it is merging in the overlay > and giving Linux one already merged. If the overlay is handled by the bootloader then from the point of view of Linux there is no overlay - sounds like there's an issue in the overlay, it should be overriding something that it doesn't? [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 488 bytes --]
WARNING: multiple messages have this Message-ID (diff)
From: Mark Brown <broonie@kernel.org> To: Joe Burmeister <joe.burmeister@devtank.co.uk> Cc: Florian Fainelli <f.fainelli@gmail.com>, Ray Jui <rjui@broadcom.com>, Scott Branden <sbranden@broadcom.com>, bcm-kernel-feedback-list@broadcom.com, linux-spi@vger.kernel.org, linux-rpi-kernel@lists.infradead.org, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, nsaenz@kernel.org Subject: Re: [PATCH] spi: bcm2835: Fix buffer overflow with CS able to go beyond limit. Date: Fri, 23 Apr 2021 12:57:24 +0100 [thread overview] Message-ID: <20210423115724.GB5507@sirena.org.uk> (raw) In-Reply-To: <380624c4-82f3-0e6e-8cdb-8a9732636db8@devtank.co.uk> [-- Attachment #1.1: Type: text/plain, Size: 1909 bytes --] On Fri, Apr 23, 2021 at 11:03:22AM +0100, Joe Burmeister wrote: > On 23/04/2021 00:49, Florian Fainelli wrote: > > Right, which means that we should probably seek a solution within the > > SPI core itself, even if you can only test with spi-bcm2835.c chances > > are that the fix would be applicable for other controllers if done in > > the core. > I'm not sure it's possible to do in the core alone. The numb of the > issue is the core changes ctlr->num_chipselect to what is in the device > tree and some drivers are cool with that overs quietly stomp memory. I wouldn't expect any controller to be OK with that? Drivers can store per-client data in spi_device->controller_data which doesn't need scaling (but is also not so helpful if you need to look at clients other than the one you're currently controlling). > I've got a simple little patch to warn when the core expands > ctlr->num_chipselect. This warning won't go off in bcm2835 with my patch > because I am also extending ctlr->num_chipselect to the amount in the > device tree before the core does that expansion. Hopefully that new > warning would make people investigate and fix problem drivers. > >> There is protection in spi_add_device, which will catch extra added > >> later, but not ones in the device tree when the spi controller was > >> registered. > > Not sure I follow you, if we have the overlay before > > spi_register_controller() is called, how can the check there not > > trigger? And if we load the overlay later when the SPI controller is > > already registered, why does not spi_add_device()'s check work? > I think it might be a RPI thing. I think it is merging in the overlay > and giving Linux one already merged. If the overlay is handled by the bootloader then from the point of view of Linux there is no overlay - sounds like there's an issue in the overlay, it should be overriding something that it doesn't? [-- Attachment #1.2: signature.asc --] [-- Type: application/pgp-signature, Size: 488 bytes --] [-- Attachment #2: Type: text/plain, Size: 176 bytes --] _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2021-04-23 11:57 UTC|newest] Thread overview: 24+ messages / expand[flat|nested] mbox.gz Atom feed top 2021-04-20 8:34 [PATCH] spi: bcm2835: Fix buffer overflow with CS able to go beyond limit Joe Burmeister 2021-04-20 8:34 ` Joe Burmeister 2021-04-22 16:42 ` Mark Brown 2021-04-22 16:42 ` Mark Brown 2021-04-22 16:50 ` Florian Fainelli 2021-04-22 16:50 ` Florian Fainelli 2021-04-22 20:10 ` Joe Burmeister 2021-04-22 20:10 ` Joe Burmeister 2021-04-22 23:49 ` Florian Fainelli 2021-04-22 23:49 ` Florian Fainelli 2021-04-23 10:03 ` Joe Burmeister 2021-04-23 10:03 ` Joe Burmeister 2021-04-23 11:57 ` Mark Brown [this message] 2021-04-23 11:57 ` Mark Brown 2021-04-23 14:12 ` Joe Burmeister 2021-04-23 14:12 ` Joe Burmeister 2021-04-23 16:20 ` Mark Brown 2021-04-23 16:20 ` Mark Brown 2021-04-23 17:34 ` Nicolas Saenz Julienne 2021-04-23 17:34 ` Nicolas Saenz Julienne 2021-05-01 19:51 ` Lukas Wunner 2021-05-04 11:51 ` Mark Brown 2021-05-04 11:51 ` Mark Brown 2021-05-04 13:53 ` Lukas Wunner
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20210423115724.GB5507@sirena.org.uk \ --to=broonie@kernel.org \ --cc=bcm-kernel-feedback-list@broadcom.com \ --cc=f.fainelli@gmail.com \ --cc=joe.burmeister@devtank.co.uk \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-rpi-kernel@lists.infradead.org \ --cc=linux-spi@vger.kernel.org \ --cc=nsaenz@kernel.org \ --cc=rjui@broadcom.com \ --cc=sbranden@broadcom.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.