* [GIT PULL] SPI fixes for v5.17-rc7 @ 2022-03-10 12:13 Mark Brown 2022-03-10 12:25 ` Linus Torvalds 2022-03-10 12:27 ` pr-tracker-bot 0 siblings, 2 replies; 9+ messages in thread From: Mark Brown @ 2022-03-10 12:13 UTC (permalink / raw) To: Linus Torvalds; +Cc: linux-spi, linux-kernel, Mark Brown The following changes since commit 80808768e41324d2e23de89972b5406c1020e6e4: spi: rockchip: terminate dma transmission when slave abort (2022-02-17 15:33:18 +0000) are available in the Git repository at: https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git tags/spi-fix-v5.17-rc7 for you to fetch changes up to 1a4e53d2fc4f68aa654ad96d13ad042e1a8e8a7d: spi: Fix invalid sgs value (2022-03-08 12:27:33 +0000) ---------------------------------------------------------------- spi: Fix for v5.17 One fix for type conversion issues when working out maximum scatter/gather segment sizes which caused problems for some systems which where the limits overflow due to the type conversion. ---------------------------------------------------------------- Biju Das (1): spi: Fix invalid sgs value drivers/spi/spi.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [GIT PULL] SPI fixes for v5.17-rc7 2022-03-10 12:13 [GIT PULL] SPI fixes for v5.17-rc7 Mark Brown @ 2022-03-10 12:25 ` Linus Torvalds 2022-03-10 12:47 ` Mark Brown 2022-03-10 12:27 ` pr-tracker-bot 1 sibling, 1 reply; 9+ messages in thread From: Linus Torvalds @ 2022-03-10 12:25 UTC (permalink / raw) To: Mark Brown; +Cc: linux-spi, Linux Kernel Mailing List On Thu, Mar 10, 2022 at 4:13 AM Mark Brown <broonie@kernel.org> wrote: > > One fix for type conversion issues when working out maximum > scatter/gather segment sizes which caused problems for some systems > which where the limits overflow due to the type conversion. Side note: 'ctrl->max_dma_len' is a 'size_t', so even 'unsigned int' isn't necessarily a sufficient type. Not that I see anything setting it to values that would cause problems, but it looks conceptually like a similar problem as that 'max_seg_size' thing was. Linus ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [GIT PULL] SPI fixes for v5.17-rc7 2022-03-10 12:25 ` Linus Torvalds @ 2022-03-10 12:47 ` Mark Brown 2022-03-15 9:08 ` Geert Uytterhoeven 0 siblings, 1 reply; 9+ messages in thread From: Mark Brown @ 2022-03-10 12:47 UTC (permalink / raw) To: Linus Torvalds; +Cc: linux-spi, Linux Kernel Mailing List [-- Attachment #1: Type: text/plain, Size: 487 bytes --] On Thu, Mar 10, 2022 at 04:25:34AM -0800, Linus Torvalds wrote: > On Thu, Mar 10, 2022 at 4:13 AM Mark Brown <broonie@kernel.org> wrote: > > One fix for type conversion issues when working out maximum > > scatter/gather segment sizes which caused problems for some systems > > which where the limits overflow due to the type conversion. > Side note: 'ctrl->max_dma_len' is a 'size_t', so even 'unsigned int' > isn't necessarily a sufficient type. Hrm, yes - thanks for spotting that. [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 488 bytes --] ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [GIT PULL] SPI fixes for v5.17-rc7 2022-03-10 12:47 ` Mark Brown @ 2022-03-15 9:08 ` Geert Uytterhoeven 2022-03-15 16:48 ` Linus Torvalds 0 siblings, 1 reply; 9+ messages in thread From: Geert Uytterhoeven @ 2022-03-15 9:08 UTC (permalink / raw) To: Mark Brown; +Cc: Linus Torvalds, linux-spi, Linux Kernel Mailing List Hi Mark, On Thu, Mar 10, 2022 at 3:46 PM Mark Brown <broonie@kernel.org> wrote: > On Thu, Mar 10, 2022 at 04:25:34AM -0800, Linus Torvalds wrote: > > On Thu, Mar 10, 2022 at 4:13 AM Mark Brown <broonie@kernel.org> wrote: > > > One fix for type conversion issues when working out maximum > > > scatter/gather segment sizes which caused problems for some systems > > > which where the limits overflow due to the type conversion. > > > Side note: 'ctrl->max_dma_len' is a 'size_t', so even 'unsigned int' > > isn't necessarily a sufficient type. > > Hrm, yes - thanks for spotting that. I had noticed while reviewing the patch, but changing to size_t wouldn't help much, as other related code paths treat the value as unsigned int anyway. Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [GIT PULL] SPI fixes for v5.17-rc7 2022-03-15 9:08 ` Geert Uytterhoeven @ 2022-03-15 16:48 ` Linus Torvalds 2022-03-15 18:56 ` Geert Uytterhoeven 0 siblings, 1 reply; 9+ messages in thread From: Linus Torvalds @ 2022-03-15 16:48 UTC (permalink / raw) To: Geert Uytterhoeven; +Cc: Mark Brown, linux-spi, Linux Kernel Mailing List On Tue, Mar 15, 2022 at 2:08 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote: > > I had noticed while reviewing the patch, but changing to size_t wouldn't > help much, as other related code paths treat the value as unsigned int > anyway. .. but it really would. Note that the paths *after* this code don't matter. Because the result is guaranteed to fit in 'unsigned int' anyway. Put another way: min_t(unsigned int,x,y) is buggy if one of x/y is 'size_t'. Why? Because if that one gets truncated, you're doing 'min()' with a value that may be artificially much too small (that was exactly the problem commit 1a4e53d2fc4f: "spi: Fix invalid sgs value")fixed). But the situation is _not_ true in the reverse. Look: min(size_t,x,y) is guaranteed to fit in 'unsigned int' as long as _one_ of x,y fits in 'unsigned int' - even if the other doesn't. Because then 'min()' will just pick the one that already had the right size. To make it really concrete, compare min_t(unsigned int, 5, 0x100000001); min_t(size_t, 5, 0x100000001); on a 64-bit machine (ie size_t is 64-bits, and unsigned int is 32-bit). One returns 1. The other returns 5. Both fit the result in 'unsigned int', but one of them is wrong. Linus ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [GIT PULL] SPI fixes for v5.17-rc7 2022-03-15 16:48 ` Linus Torvalds @ 2022-03-15 18:56 ` Geert Uytterhoeven 2022-03-16 16:26 ` Mark Brown 0 siblings, 1 reply; 9+ messages in thread From: Geert Uytterhoeven @ 2022-03-15 18:56 UTC (permalink / raw) To: Linus Torvalds; +Cc: Mark Brown, linux-spi, Linux Kernel Mailing List, Biju Das Hi Linus, On Tue, Mar 15, 2022 at 5:48 PM Linus Torvalds <torvalds@linux-foundation.org> wrote: > On Tue, Mar 15, 2022 at 2:08 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote: > > I had noticed while reviewing the patch, but changing to size_t wouldn't > > help much, as other related code paths treat the value as unsigned int > > anyway. > > .. but it really would. > > Note that the paths *after* this code don't matter. Because the result > is guaranteed to fit in 'unsigned int' anyway. > > Put another way: > > min_t(unsigned int,x,y) > > is buggy if one of x/y is 'size_t'. Why? Because if that one gets > truncated, you're doing 'min()' with a value that may be artificially > much too small (that was exactly the problem commit 1a4e53d2fc4f: > "spi: Fix invalid sgs value")fixed). > > But the situation is _not_ true in the reverse. Look: > > min(size_t,x,y) > > is guaranteed to fit in 'unsigned int' as long as _one_ of x,y fits in > 'unsigned int' - even if the other doesn't. Because then 'min()' will > just pick the one that already had the right size. > > To make it really concrete, compare > > min_t(unsigned int, 5, 0x100000001); > min_t(size_t, 5, 0x100000001); > > on a 64-bit machine (ie size_t is 64-bits, and unsigned int is 32-bit). > > One returns 1. The other returns 5. Both fit the result in 'unsigned > int', but one of them is wrong. You're absolutely right. So the code should be changed to: if (vmalloced_buf || kmap_buf) { - desc_len = min_t(unsigned int, max_seg_size, PAGE_SIZE); + desc_len = min_t(unsigned long, max_seg_size, PAGE_SIZE); sgs = DIV_ROUND_UP(len + offset_in_page(buf), desc_len); } else if (virt_addr_valid(buf)) { - desc_len = min_t(unsigned int, max_seg_size, ctlr->max_dma_len); + desc_len = min_t(size_t, max_seg_size, ctlr->max_dma_len); sgs = DIV_ROUND_UP(len, desc_len); } else { return -EINVAL; } Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [GIT PULL] SPI fixes for v5.17-rc7 2022-03-15 18:56 ` Geert Uytterhoeven @ 2022-03-16 16:26 ` Mark Brown 2022-03-16 17:54 ` Biju Das 0 siblings, 1 reply; 9+ messages in thread From: Mark Brown @ 2022-03-16 16:26 UTC (permalink / raw) To: Geert Uytterhoeven Cc: Linus Torvalds, linux-spi, Linux Kernel Mailing List, Biju Das [-- Attachment #1: Type: text/plain, Size: 773 bytes --] On Tue, Mar 15, 2022 at 07:56:13PM +0100, Geert Uytterhoeven wrote: > You're absolutely right. So the code should be changed to: > > if (vmalloced_buf || kmap_buf) { > - desc_len = min_t(unsigned int, max_seg_size, PAGE_SIZE); > + desc_len = min_t(unsigned long, max_seg_size, PAGE_SIZE); > sgs = DIV_ROUND_UP(len + offset_in_page(buf), desc_len); > } else if (virt_addr_valid(buf)) { > - desc_len = min_t(unsigned int, max_seg_size, ctlr->max_dma_len); > + desc_len = min_t(size_t, max_seg_size, ctlr->max_dma_len); > sgs = DIV_ROUND_UP(len, desc_len); > } else { > return -EINVAL; > } Can you send a patch please? [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 488 bytes --] ^ permalink raw reply [flat|nested] 9+ messages in thread
* RE: [GIT PULL] SPI fixes for v5.17-rc7 2022-03-16 16:26 ` Mark Brown @ 2022-03-16 17:54 ` Biju Das 0 siblings, 0 replies; 9+ messages in thread From: Biju Das @ 2022-03-16 17:54 UTC (permalink / raw) To: Mark Brown, Geert Uytterhoeven Cc: Linus Torvalds, linux-spi, Linux Kernel Mailing List Hi Mark, > Subject: Re: [GIT PULL] SPI fixes for v5.17-rc7 > > On Tue, Mar 15, 2022 at 07:56:13PM +0100, Geert Uytterhoeven wrote: > > > You're absolutely right. So the code should be changed to: > > > > if (vmalloced_buf || kmap_buf) { > > - desc_len = min_t(unsigned int, max_seg_size, > PAGE_SIZE); > > + desc_len = min_t(unsigned long, max_seg_size, > PAGE_SIZE); > > sgs = DIV_ROUND_UP(len + offset_in_page(buf), desc_len); > > } else if (virt_addr_valid(buf)) { > > - desc_len = min_t(unsigned int, max_seg_size, ctlr- > >max_dma_len); > > + desc_len = min_t(size_t, max_seg_size, ctlr- > >max_dma_len); > > sgs = DIV_ROUND_UP(len, desc_len); > > } else { > > return -EINVAL; > > } > > Can you send a patch please? I have send the patch[1] [1] https://patchwork.kernel.org/project/linux-renesas-soc/patch/20220316175317.465-1-biju.das.jz@bp.renesas.com/ Cheers, Biju ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [GIT PULL] SPI fixes for v5.17-rc7 2022-03-10 12:13 [GIT PULL] SPI fixes for v5.17-rc7 Mark Brown 2022-03-10 12:25 ` Linus Torvalds @ 2022-03-10 12:27 ` pr-tracker-bot 1 sibling, 0 replies; 9+ messages in thread From: pr-tracker-bot @ 2022-03-10 12:27 UTC (permalink / raw) To: Mark Brown; +Cc: Linus Torvalds, linux-spi, linux-kernel, Mark Brown The pull request you sent on Thu, 10 Mar 2022 12:13:11 +0000: > https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git tags/spi-fix-v5.17-rc7 has been merged into torvalds/linux.git: https://git.kernel.org/torvalds/c/1db333d9a51f3459fba1bcaa564d95befe79f0b3 Thank you! -- Deet-doot-dot, I am a bot. https://korg.docs.kernel.org/prtracker.html ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2022-03-16 17:54 UTC | newest] Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-03-10 12:13 [GIT PULL] SPI fixes for v5.17-rc7 Mark Brown 2022-03-10 12:25 ` Linus Torvalds 2022-03-10 12:47 ` Mark Brown 2022-03-15 9:08 ` Geert Uytterhoeven 2022-03-15 16:48 ` Linus Torvalds 2022-03-15 18:56 ` Geert Uytterhoeven 2022-03-16 16:26 ` Mark Brown 2022-03-16 17:54 ` Biju Das 2022-03-10 12:27 ` pr-tracker-bot
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).