Skip to content

vc4-kms-v3d overlay breaks bcm2835-i2s memory acquisition #1746

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.

Sign up for GitHub

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

Jump to bottom
Closed
flatmax opened this issue Dec 4, 2016 · 16 comments
Closed

vc4-kms-v3d overlay breaks bcm2835-i2s memory acquisition #1746

flatmax opened this issue Dec 4, 2016 · 16 comments

Comments

@flatmax
Copy link
Contributor

flatmax commented Dec 4, 2016

It appears that the i2s driver will not load when the vc4-kms-v3d overlay is loaded.
The reported reason appears to be because of a memory reservation conflict :
http://www.flatmax.org/phpbb/viewtopic.php?f=5&t=28
[ 9.831233] bcm2835-i2s 3f203000.i2s: can't request region for resource [mem 0x3f101098-0x3f10109f]
[ 9.840558] bcm2835-i2s: probe of 3f203000.i2s failed with error -16

Matt
@flatmax
Copy link
Contributor Author

flatmax commented Dec 4, 2016 •
edited
Loading

This is related to this issue :
#1420

Apparently due to clock frame work issues.

@pelwell
Copy link
Contributor

pelwell commented Dec 4, 2016 •
edited
Loading

Yes - it is the same issue. The I2S driver tries to directly manipulate the clock hardware, but the vc4-kms-v3d overlay enables the new clock driver which reserves the same I/O space as its domain. I don't think we'll be enabling the clock driver by default in the 4.4 tree, so in order to work with and without the vc4 overlay the I2S driver will need to support both access methods, presumably with a DT property to switch between the two modes. The obvious indication would be the absence of the second I/O region from the i2s node, which we can achieve by adding the shortened i2s "reg" property to the vc4-kms-v3d overlay.

@clivem
Copy link

clivem commented Dec 4, 2016

I don't think we'll be enabling the clock driver by default in the 4.4 tree, so in order to work with and without the vc4 overlay the I2S driver will need to support both access methods, presumably with a DT property to switch between the two modes.

I can't help feeling that this is more goofy than just having accepted the complete dma/clock/i2s backport into the rpi-4.4.y tree to start with..... Oh well!

@pelwell
Copy link
Contributor

pelwell commented Dec 4, 2016

Well, you know what they say about opinions.

@popcornmix I'm midway through a partial backport of this upstream commit. The options are:

  1. Continue the backport, so that the clock driver support in the I2S driver is only used with the vc4-kms-v3d overlay.
  2. Always use the clock driver.
  3. Do nothing.

What are your thoughts?

@clivem
Copy link

clivem commented Dec 4, 2016

Well, you know what they say about opinions.

That you are not interested in mine? ;)

No worries. I've been maintaining it out-of-tree for the audio guys that wanted to stick with 4.4.y for many months, so that's what I'll carry on doing.

@popcornmix
Copy link
Collaborator

@pelwell
Are you aware of any breakage by always using the clock driver?
No objection to always using it if it doesn't break anything.

The fourth option is to move to a newer kernel (4.8/4.9) where this isn't an issue.

@pelwell
Copy link
Contributor

pelwell commented Dec 4, 2016

The only potential breakage I'm aware of is with the MIDI/DMX users who have been getting the kernel to use non-standard baudrates by lying about the UART clockrate. They will need an overlay to create a fixed-factor clock in order to achieve the same effect.

You have more exposure to feedback from Kodi's use of the 4.8 tree - how do you feel about either a full backport in 4.4 or switching up to 4.8?

@popcornmix
Copy link
Collaborator

Not aware of any problems with either clock driver or a move to 4.8 kernel.
All is stable in the kodi world.

I have a slight preference for 4.9 as the next stable kernel, due to the cleaner downstream patching, and it's rumoured to be LTS, but there's some argument to using 4.8 for rpi-update for a while as a stepping stone to 4.9 (may be a useful data point in regression testing).

@pelwell
Copy link
Contributor

pelwell commented Dec 4, 2016

Do you have a preferred schedule for the update?

@popcornmix
Copy link
Collaborator

If it fixes this issue (avoiding the work of backporting) and we're not aware of any downsides then I don't mind doing this soon (e.g. this week). We've just had a stable bump to raspbian, so we have a while before the next stable release.

One thing I remember - vc4-kms-v3d / vc4-fkms-v3d is not working on 4.8 but is on 4.9.
I think it may just need some copy/pasting of DT nodes to make it work on 4.8, but we should either fix that or go straight to 4.9.

@clivem
Copy link

clivem commented Dec 4, 2016

....avoiding the work of backporting....

FYI, the "work" was done back in August. #1592

@flatmax
Copy link
Contributor Author

flatmax commented Dec 5, 2016 via email

@pelwell
Copy link
Contributor

pelwell commented Dec 9, 2016

The BRANCH=next kernel has been moved to 4.8 - see https://www.raspberrypi.org/forums/viewtopic.php?f=29&t=167934 - having already been used succesfully by LibreELEC for a while. I doubt it will stay there for long before hitting the master branch.

@Ruffio
Copy link

Ruffio commented Feb 5, 2017

@flatmax can this be closed? It looks like it has been resolved...

@flatmax
Copy link
Contributor Author

flatmax commented Feb 5, 2017 via email

@anholt
Copy link
Contributor

anholt commented Mar 2, 2017

Now that the default is 4.9, I think this can be closed.

@pelwell pelwell closed this as completed Mar 2, 2017
@jstirzaker jstirzaker mentioned this issue Mar 27, 2024
Open
popcornmix pushed a commit that referenced this issue Dec 7, 2024
@gregkh
wifi: ath12k: fix use-after-free in ath12k_dp_cc_cleanup()
35be501 
[ Upstream commit bdb2811 ]

During ath12k module removal, in ath12k_core_deinit(),
ath12k_mac_destroy() un-registers ah->hw from mac80211 and frees
the ah->hw as well as all the ar's in it. After this
ath12k_core_soc_destroy()-> ath12k_dp_free()-> ath12k_dp_cc_cleanup()
tries to access one of the freed ar's from pending skb.

This is because during mac destroy, driver failed to flush few
data packets, which were accessed later in ath12k_dp_cc_cleanup()
and freed, but using ar from the packet led to this use-after-free.

BUG: KASAN: use-after-free in ath12k_dp_cc_cleanup.part.0+0x5e2/0xd40 [ath12k]
Write of size 4 at addr ffff888150bd3514 by task modprobe/8926
CPU: 0 UID: 0 PID: 8926 Comm: modprobe Not tainted
6.11.0-rc2-wt-ath+ #1746
Hardware name: Intel(R) Client Systems NUC8i7HVK/NUC8i7HVB, BIOS
HNKBLi70.86A.0067.2021.0528.1339 05/28/2021

Call Trace:
  <TASK>
  dump_stack_lvl+0x7d/0xe0
  print_address_description.constprop.0+0x33/0x3a0
  print_report+0xb5/0x260
  ? kasan_addr_to_slab+0x24/0x80
  kasan_report+0xd8/0x110
  ? ath12k_dp_cc_cleanup.part.0+0x5e2/0xd40 [ath12k]
  ? ath12k_dp_cc_cleanup.part.0+0x5e2/0xd40 [ath12k]
  kasan_check_range+0xf3/0x1a0
  __kasan_check_write+0x14/0x20
  ath12k_dp_cc_cleanup.part.0+0x5e2/0xd40 [ath12k]
  ath12k_dp_free+0x178/0x420 [ath12k]
  ath12k_core_stop+0x176/0x200 [ath12k]
  ath12k_core_deinit+0x13f/0x210 [ath12k]
  ath12k_pci_remove+0xad/0x1c0 [ath12k]
  pci_device_remove+0x9b/0x1b0
  device_remove+0xbf/0x150
  device_release_driver_internal+0x3c3/0x580
  ? __kasan_check_read+0x11/0x20
  driver_detach+0xc4/0x190
  bus_remove_driver+0x130/0x2a0
  driver_unregister+0x68/0x90
  pci_unregister_driver+0x24/0x240
  ? find_module_all+0x13e/0x1e0
  ath12k_pci_exit+0x10/0x20 [ath12k]
  __do_sys_delete_module+0x32c/0x580
  ? module_flags+0x2f0/0x2f0
  ? kmem_cache_free+0xf0/0x410
  ? __fput+0x56f/0xab0
  ? __fput+0x56f/0xab0
  ? debug_smp_processor_id+0x17/0x20
  __x64_sys_delete_module+0x4f/0x70
  x64_sys_call+0x522/0x9f0
  do_syscall_64+0x64/0x130
  entry_SYSCALL_64_after_hwframe+0x4b/0x53
RIP: 0033:0x7f8182c6ac8b

Commit 24de1b7 ("wifi: ath12k: fix flush failure in recovery
scenarios") added the change to decrement the pending packets count
in case of recovery which make sense as ah->hw as well all
ar's in it are intact during recovery, but during core deinit there
is no use in decrementing packets count or waking up the empty waitq
as the module is going to be removed also ar's from pending skb's
can't be used and the packets should just be released back.

To fix this, avoid accessing ar from skb->cb when driver is being
unregistered.

Tested-on: QCN9274 hw2.0 PCI WLAN.WBE.1.1.1-00214-QCAHKSWPL_SILICONZ-1
Tested-on: WCN7850 hw2.0 PCI WLAN.HMT.1.0.c5-00481-QCAHMTSWPL_V1.0_V2.0_SILICONZ-3

Fixes: 24de1b7 ("wifi: ath12k: fix flush failure in recovery scenarios")
Signed-off-by: Rameshkumar Sundaram <[email protected]>
Acked-by: Jeff Johnson <[email protected]>
Link: https://patch.msgid.link/[email protected]
Signed-off-by: Jeff Johnson <[email protected]>
Signed-off-by: Sasha Levin <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
6 participants
@flatmax @popcornmix @clivem @Ruffio @anholt @pelwell