Skip to content

SPI: enable compiling upstream spi-bcm2835 driver and add overlay to allow us to load the driver #866

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged

Conversation

msperl
Copy link
Contributor

@msperl msperl commented Mar 2, 2015

note that when we move to generic GPIOs we will need to modify the brcm,function to 1 in spi0_cs_pins
Some of this (especially the separation between spi0_pins and spin0_cs_pins) might get generalized into the "core" device-tree.

@msperl
Copy link
Contributor Author

msperl commented Mar 2, 2015

See #864 for the discussions around this.

@msperl
Copy link
Contributor Author

msperl commented Mar 2, 2015

@pelwell: you want the overlay named differently? See the discussion around it in #864

@msperl msperl changed the title enable compiling spi-bcm2835 and add overlay to allow us to load the up-stream driver SPI: enable compiling upstream spi-bcm2835 driver and add overlay to allow us to load the driver Mar 2, 2015
@pelwell
Copy link
Contributor

pelwell commented Mar 2, 2015

That is a "Line comment" attached the line that says:

 * Device tree overlay for spm-2835

@msperl
Copy link
Contributor Author

msperl commented Mar 2, 2015

fixed

@pelwell
Copy link
Contributor

pelwell commented Mar 2, 2015

I'll merge if you change the comment to either spi-bcm2835 or bcm2835-spi, and then squash the commits down to one.

@msperl msperl force-pushed the rpi-3.18.y-enable-spi-bcm2835 branch from 4619a06 to 2bb9e24 Compare March 2, 2015 11:02
@msperl
Copy link
Contributor Author

msperl commented Mar 2, 2015

Done - forced pushing a squash upstream requires lots of memory - the default swap settings of raspbian are not enough...

@notro
Copy link
Contributor

notro commented Mar 2, 2015

cs-gpios = <&gpio 8 0>, <&gpio 7 0>;

Why this? It's not used by the driver. And also why splitting out the cs pins?

@msperl
Copy link
Contributor Author

msperl commented Mar 2, 2015

well - the reason is that at some point in the future we will have generic GPIOs acting as CS (I got a working prototype), so IMO it makes sense to separate them already...
finally this makes it more easily configurable - the CS are different from the SCK, MISO, MOSI pins.

@notro
Copy link
Contributor

notro commented Mar 2, 2015

I don't know what Phil's policy is, but changes like this isn't accepted in mainline. Having code lying around that might or will get used in the future isn't good. Someone reading the overlay will assume that cs-gpios actually works.
Better add this in when it's needed.

@msperl
Copy link
Contributor Author

msperl commented Mar 2, 2015

I shall remove those cs-gpios then, but I would leave the separation of the GPIOs.
As said somewhere: IMO it actually would make sense to split those on the global device-tree...

@msperl msperl force-pushed the rpi-3.18.y-enable-spi-bcm2835 branch from 2bb9e24 to bc61074 Compare March 2, 2015 11:24
@msperl
Copy link
Contributor Author

msperl commented Mar 2, 2015

better now?

@notro
Copy link
Contributor

notro commented Mar 2, 2015

Yes, better.
I have to go now, but can you point me to your cs pin split argument? I can't see how this change in the DT provides any benefit, at the present at least.

@pelwell
Copy link
Contributor

pelwell commented Mar 2, 2015

Yes, I would also like to understand that.

@msperl
Copy link
Contributor Author

msperl commented Mar 2, 2015

the split is there to avoid changes in the future.
actually there is a risk of merging a later version of the driver from upstream that then only supports gpio-cs and people will expect it to work, that is why I started to include all those things that right now do not make a difference, but will have people complaining if things stop to work in the future.

@msperl
Copy link
Contributor Author

msperl commented Mar 2, 2015

but OK, I will remove also that...

@msperl msperl force-pushed the rpi-3.18.y-enable-spi-bcm2835 branch from bc61074 to 6597808 Compare March 2, 2015 11:55
@msperl
Copy link
Contributor Author

msperl commented Mar 2, 2015

done - now it is very minimal

@pelwell
Copy link
Contributor

pelwell commented Mar 2, 2015

We like minimalism, provided it is functional minimalism.

pelwell added a commit that referenced this pull request Mar 2, 2015
SPI: enable compiling upstream spi-bcm2835 driver and add overlay to allow us to load the driver
@pelwell pelwell merged commit 11bfe08 into raspberrypi:rpi-3.18.y Mar 2, 2015
popcornmix added a commit to raspberrypi/firmware that referenced this pull request Mar 3, 2015
firmware: arm_loader: Support HATs on all boards with the bplus layout

firmware: dtblob: Remove the static i2c0 pin assignments for B+ and 2B

firmware: arm_loader: Use bcm2708-rpi-cm.dtb on a Compute Module

kernel: enable mcp2515 CAN controller module plus the corrresponding overlay
See: raspberrypi/linux#868

kernel: Built-in serial port losing characters
See: raspberrypi/linux#854

kernel: SPI: enable compiling upstream spi-bcm2835 driver and add overlay to allow us to load the driver
See: raspberrypi/linux#866
popcornmix added a commit to Hexxeh/rpi-firmware that referenced this pull request Mar 3, 2015
firmware: arm_loader: Support HATs on all boards with the bplus layout

firmware: dtblob: Remove the static i2c0 pin assignments for B+ and 2B

firmware: arm_loader: Use bcm2708-rpi-cm.dtb on a Compute Module

kernel: enable mcp2515 CAN controller module plus the corrresponding overlay
See: raspberrypi/linux#868

kernel: Built-in serial port losing characters
See: raspberrypi/linux#854

kernel: SPI: enable compiling upstream spi-bcm2835 driver and add overlay to allow us to load the driver
See: raspberrypi/linux#866
neuschaefer pushed a commit to neuschaefer/raspi-binary-firmware that referenced this pull request Feb 27, 2017
firmware: arm_loader: Support HATs on all boards with the bplus layout

firmware: dtblob: Remove the static i2c0 pin assignments for B+ and 2B

firmware: arm_loader: Use bcm2708-rpi-cm.dtb on a Compute Module

kernel: enable mcp2515 CAN controller module plus the corrresponding overlay
See: raspberrypi/linux#868

kernel: Built-in serial port losing characters
See: raspberrypi/linux#854

kernel: SPI: enable compiling upstream spi-bcm2835 driver and add overlay to allow us to load the driver
See: raspberrypi/linux#866
popcornmix pushed a commit that referenced this pull request Jan 20, 2020
In current driver, locks can be taken as follows:
- Register access: take a lock on regmap config and then on clock.
- Master clock provider: take a lock on clock and then on regmap config.
This can lead to the circular locking summarized below.

Remove peripheral clock management through regmap framework, and manage
peripheral clock in driver instead. On register access, lock on clock
is taken first, which allows to avoid possible locking issue.

[ 6696.561513] ======================================================
[ 6696.567670] WARNING: possible circular locking dependency detected
[ 6696.573842] 4.19.49 #866 Not tainted
[ 6696.577397] ------------------------------------------------------
[ 6696.583566] pulseaudio/6439 is trying to acquire lock:
[ 6696.588697] 87b0a25b (enable_lock){..-.}, at: clk_enable_lock+0x64/0x128
[ 6696.595377]
[ 6696.595377] but task is already holding lock:
[ 6696.601197] d858f825 (stm32_sai_sub:1342:(sai->regmap_config)->lock){....}
...
[ 6696.812513]  Possible unsafe locking scenario:
[ 6696.812513]
[ 6696.818418]        CPU0                    CPU1
[ 6696.822935]        ----                    ----
[ 6696.827451]   lock(stm32_sai_sub:1342:(sai->regmap_config)->lock);
[ 6696.833618]                                lock(enable_lock);
[ 6696.839350]                                lock(stm32_sai_sub:1342:
                                              (sai->regmap_config)->lock);
[ 6696.848035]   lock(enable_lock);

Fixes: 03e78a2 ("ASoC: stm32: sai: add h7 support")

Signed-off-by: Olivier Moysan <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Mark Brown <[email protected]>
popcornmix pushed a commit that referenced this pull request Jan 30, 2020
commit a14bf98 upstream.

In current driver, locks can be taken as follows:
- Register access: take a lock on regmap config and then on clock.
- Master clock provider: take a lock on clock and then on regmap config.
This can lead to the circular locking summarized below.

Remove peripheral clock management through regmap framework, and manage
peripheral clock in driver instead. On register access, lock on clock
is taken first, which allows to avoid possible locking issue.

[ 6696.561513] ======================================================
[ 6696.567670] WARNING: possible circular locking dependency detected
[ 6696.573842] 4.19.49 #866 Not tainted
[ 6696.577397] ------------------------------------------------------
[ 6696.583566] pulseaudio/6439 is trying to acquire lock:
[ 6696.588697] 87b0a25b (enable_lock){..-.}, at: clk_enable_lock+0x64/0x128
[ 6696.595377]
[ 6696.595377] but task is already holding lock:
[ 6696.601197] d858f825 (stm32_sai_sub:1342:(sai->regmap_config)->lock){....}
...
[ 6696.812513]  Possible unsafe locking scenario:
[ 6696.812513]
[ 6696.818418]        CPU0                    CPU1
[ 6696.822935]        ----                    ----
[ 6696.827451]   lock(stm32_sai_sub:1342:(sai->regmap_config)->lock);
[ 6696.833618]                                lock(enable_lock);
[ 6696.839350]                                lock(stm32_sai_sub:1342:
                                              (sai->regmap_config)->lock);
[ 6696.848035]   lock(enable_lock);

Fixes: 03e78a2 ("ASoC: stm32: sai: add h7 support")

Signed-off-by: Olivier Moysan <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Mark Brown <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>
popcornmix pushed a commit that referenced this pull request Oct 10, 2024
commit a164f3a upstream.

As Ojaswin mentioned in Link, in ext4_ext_insert_extent(), if the path is
reallocated in ext4_ext_create_new_leaf(), we'll use the stale path and
cause UAF. Below is a sample trace with dummy values:

ext4_ext_insert_extent
  path = *ppath = 2000
  ext4_ext_create_new_leaf(ppath)
    ext4_find_extent(ppath)
      path = *ppath = 2000
      if (depth > path[0].p_maxdepth)
            kfree(path = 2000);
            *ppath = path = NULL;
      path = kcalloc() = 3000
      *ppath = 3000;
      return path;
  /* here path is still 2000, UAF! */
  eh = path[depth].p_hdr

==================================================================
BUG: KASAN: slab-use-after-free in ext4_ext_insert_extent+0x26d4/0x3330
Read of size 8 at addr ffff8881027bf7d0 by task kworker/u36:1/179
CPU: 3 UID: 0 PID: 179 Comm: kworker/u6:1 Not tainted 6.11.0-rc2-dirty #866
Call Trace:
 <TASK>
 ext4_ext_insert_extent+0x26d4/0x3330
 ext4_ext_map_blocks+0xe22/0x2d40
 ext4_map_blocks+0x71e/0x1700
 ext4_do_writepages+0x1290/0x2800
[...]

Allocated by task 179:
 ext4_find_extent+0x81c/0x1f70
 ext4_ext_map_blocks+0x146/0x2d40
 ext4_map_blocks+0x71e/0x1700
 ext4_do_writepages+0x1290/0x2800
 ext4_writepages+0x26d/0x4e0
 do_writepages+0x175/0x700
[...]

Freed by task 179:
 kfree+0xcb/0x240
 ext4_find_extent+0x7c0/0x1f70
 ext4_ext_insert_extent+0xa26/0x3330
 ext4_ext_map_blocks+0xe22/0x2d40
 ext4_map_blocks+0x71e/0x1700
 ext4_do_writepages+0x1290/0x2800
 ext4_writepages+0x26d/0x4e0
 do_writepages+0x175/0x700
[...]
==================================================================

So use *ppath to update the path to avoid the above problem.

Reported-by: Ojaswin Mujoo <[email protected]>
Closes: https://lore.kernel.org/r/[email protected]
Fixes: 10809df ("ext4: teach ext4_ext_find_extent() to realloc path if necessary")
Cc: [email protected]
Signed-off-by: Baokun Li <[email protected]>
Reviewed-by: Jan Kara <[email protected]>
Link: https://patch.msgid.link/[email protected]
Signed-off-by: Theodore Ts'o <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>
popcornmix pushed a commit that referenced this pull request Oct 10, 2024
commit a164f3a upstream.

As Ojaswin mentioned in Link, in ext4_ext_insert_extent(), if the path is
reallocated in ext4_ext_create_new_leaf(), we'll use the stale path and
cause UAF. Below is a sample trace with dummy values:

ext4_ext_insert_extent
  path = *ppath = 2000
  ext4_ext_create_new_leaf(ppath)
    ext4_find_extent(ppath)
      path = *ppath = 2000
      if (depth > path[0].p_maxdepth)
            kfree(path = 2000);
            *ppath = path = NULL;
      path = kcalloc() = 3000
      *ppath = 3000;
      return path;
  /* here path is still 2000, UAF! */
  eh = path[depth].p_hdr

==================================================================
BUG: KASAN: slab-use-after-free in ext4_ext_insert_extent+0x26d4/0x3330
Read of size 8 at addr ffff8881027bf7d0 by task kworker/u36:1/179
CPU: 3 UID: 0 PID: 179 Comm: kworker/u6:1 Not tainted 6.11.0-rc2-dirty #866
Call Trace:
 <TASK>
 ext4_ext_insert_extent+0x26d4/0x3330
 ext4_ext_map_blocks+0xe22/0x2d40
 ext4_map_blocks+0x71e/0x1700
 ext4_do_writepages+0x1290/0x2800
[...]

Allocated by task 179:
 ext4_find_extent+0x81c/0x1f70
 ext4_ext_map_blocks+0x146/0x2d40
 ext4_map_blocks+0x71e/0x1700
 ext4_do_writepages+0x1290/0x2800
 ext4_writepages+0x26d/0x4e0
 do_writepages+0x175/0x700
[...]

Freed by task 179:
 kfree+0xcb/0x240
 ext4_find_extent+0x7c0/0x1f70
 ext4_ext_insert_extent+0xa26/0x3330
 ext4_ext_map_blocks+0xe22/0x2d40
 ext4_map_blocks+0x71e/0x1700
 ext4_do_writepages+0x1290/0x2800
 ext4_writepages+0x26d/0x4e0
 do_writepages+0x175/0x700
[...]
==================================================================

So use *ppath to update the path to avoid the above problem.

Reported-by: Ojaswin Mujoo <[email protected]>
Closes: https://lore.kernel.org/r/[email protected]
Fixes: 10809df ("ext4: teach ext4_ext_find_extent() to realloc path if necessary")
Cc: [email protected]
Signed-off-by: Baokun Li <[email protected]>
Reviewed-by: Jan Kara <[email protected]>
Link: https://patch.msgid.link/[email protected]
Signed-off-by: Theodore Ts'o <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>
popcornmix pushed a commit that referenced this pull request Oct 10, 2024
commit a164f3a upstream.

As Ojaswin mentioned in Link, in ext4_ext_insert_extent(), if the path is
reallocated in ext4_ext_create_new_leaf(), we'll use the stale path and
cause UAF. Below is a sample trace with dummy values:

ext4_ext_insert_extent
  path = *ppath = 2000
  ext4_ext_create_new_leaf(ppath)
    ext4_find_extent(ppath)
      path = *ppath = 2000
      if (depth > path[0].p_maxdepth)
            kfree(path = 2000);
            *ppath = path = NULL;
      path = kcalloc() = 3000
      *ppath = 3000;
      return path;
  /* here path is still 2000, UAF! */
  eh = path[depth].p_hdr

==================================================================
BUG: KASAN: slab-use-after-free in ext4_ext_insert_extent+0x26d4/0x3330
Read of size 8 at addr ffff8881027bf7d0 by task kworker/u36:1/179
CPU: 3 UID: 0 PID: 179 Comm: kworker/u6:1 Not tainted 6.11.0-rc2-dirty #866
Call Trace:
 <TASK>
 ext4_ext_insert_extent+0x26d4/0x3330
 ext4_ext_map_blocks+0xe22/0x2d40
 ext4_map_blocks+0x71e/0x1700
 ext4_do_writepages+0x1290/0x2800
[...]

Allocated by task 179:
 ext4_find_extent+0x81c/0x1f70
 ext4_ext_map_blocks+0x146/0x2d40
 ext4_map_blocks+0x71e/0x1700
 ext4_do_writepages+0x1290/0x2800
 ext4_writepages+0x26d/0x4e0
 do_writepages+0x175/0x700
[...]

Freed by task 179:
 kfree+0xcb/0x240
 ext4_find_extent+0x7c0/0x1f70
 ext4_ext_insert_extent+0xa26/0x3330
 ext4_ext_map_blocks+0xe22/0x2d40
 ext4_map_blocks+0x71e/0x1700
 ext4_do_writepages+0x1290/0x2800
 ext4_writepages+0x26d/0x4e0
 do_writepages+0x175/0x700
[...]
==================================================================

So use *ppath to update the path to avoid the above problem.

Reported-by: Ojaswin Mujoo <[email protected]>
Closes: https://lore.kernel.org/r/[email protected]
Fixes: 10809df ("ext4: teach ext4_ext_find_extent() to realloc path if necessary")
Cc: [email protected]
Signed-off-by: Baokun Li <[email protected]>
Reviewed-by: Jan Kara <[email protected]>
Link: https://patch.msgid.link/[email protected]
Signed-off-by: Theodore Ts'o <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants