Skip to content

[ciqlts8_8] writeback: avoid use-after-free after removing device #144

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
merged 1 commit into from
Feb 25, 2025

Conversation

thefossguy-ciq
Copy link

@thefossguy-ciq thefossguy-ciq commented Feb 21, 2025

  • Commit Message Requirements
  • Built against Vault/LTS Environment
  • kABI Check Passed, where Valid (Pre 9.4 RT does not have kABI stability)
  • Boot Test
  • Kernel SelfTest results
  • Additional Tests as determined relevant

Commit message

jira VULN-6836
cve CVE-2024-0562
commit-author Khazhismel Kumykov <[email protected]> commit f87904c075515f3e1d8f4a7115869d3b914674fd

When a disk is removed, bdi_unregister gets called to stop further writeback and wait for associated delayed work to complete.  However, wb_inode_writeback_end() may schedule bandwidth estimation dwork after this has completed, which can result in the timer attempting to access the just freed bdi_writeback.

Fix this by checking if the bdi_writeback is alive, similar to when scheduling writeback work.

Since this requires wb->work_lock, and wb_inode_writeback_end() may get called from interrupt, switch wb->work_lock to an irqsafe lock.

Link: https://lkml.kernel.org/r/[email protected] Fixes: 45a2966fd641 ("writeback: fix bandwidth estimate for spiky workload")
	Signed-off-by: Khazhismel Kumykov <[email protected]>
	Reviewed-by: Jan Kara <[email protected]>
	Cc: Michael Stapelberg <[email protected]>
	Cc: Wu Fengguang <[email protected]>
	Cc: Alexander Viro <[email protected]>
	Cc: <[email protected]>
	Signed-off-by: Andrew Morton <[email protected]>
(cherry picked from commit f87904c075515f3e1d8f4a7115869d3b914674fd)
	Signed-off-by: Pratham Patel <[email protected]>

kABI check

[ciq@lts-8-8 kernel-src-tree]$ ../kernel-dist-git/SOURCES/check-kabi -k ../kernel-dist-git/SOURCES/Module.kabi_x86_64 -s Module.symvers
[ciq@lts-8-8 kernel-src-tree]$ echo $?
0

Kselftests

$ grep '^ok ' kselftest-before.log | wc -l
     230

$ grep '^ok ' kselftest-after.log | wc -l
     230

$ grep '^not ok ' kselftest-before.log | wc -l
      62

$ grep '^not ok ' kselftest-after.log | wc -l
      62

kselftest-after.log
kselftest-before.log

jira VULN-6836
cve CVE-2024-0562
commit-author Khazhismel Kumykov <[email protected]>
commit f87904c

When a disk is removed, bdi_unregister gets called to stop further
writeback and wait for associated delayed work to complete.  However,
wb_inode_writeback_end() may schedule bandwidth estimation dwork after
this has completed, which can result in the timer attempting to access the
just freed bdi_writeback.

Fix this by checking if the bdi_writeback is alive, similar to when
scheduling writeback work.

Since this requires wb->work_lock, and wb_inode_writeback_end() may get
called from interrupt, switch wb->work_lock to an irqsafe lock.

Link: https://lkml.kernel.org/r/[email protected]
Fixes: 45a2966 ("writeback: fix bandwidth estimate for spiky workload")
	Signed-off-by: Khazhismel Kumykov <[email protected]>
	Reviewed-by: Jan Kara <[email protected]>
	Cc: Michael Stapelberg <[email protected]>
	Cc: Wu Fengguang <[email protected]>
	Cc: Alexander Viro <[email protected]>
	Cc: <[email protected]>
	Signed-off-by: Andrew Morton <[email protected]>
(cherry picked from commit f87904c)
	Signed-off-by: Pratham Patel <[email protected]>
Copy link

@gvrose8192 gvrose8192 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM - Thanks!

Copy link
Collaborator

@bmastbergen bmastbergen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🥌 Nice work Pratham

@thefossguy-ciq
Copy link
Author

Thank you for the review Greg and Brett!

@thefossguy-ciq thefossguy-ciq changed the title writeback: avoid use-after-free after removing device [ciqlts8_8] writeback: avoid use-after-free after removing device Feb 22, 2025
@thefossguy-ciq thefossguy-ciq merged commit 4099e34 into ciqlts8_8 Feb 25, 2025
3 checks passed
github-actions bot pushed a commit that referenced this pull request Apr 26, 2025
JIRA: https://issues.redhat.com/browse/RHEL-78828

commit a4faee0
Author: Xu Yang <[email protected]>
Date: Tue, 26 Nov 2024 11:28:41 +0800

  When unbind and bind the device again, kernel will dump below warning:

  [  173.972130] sysfs: cannot create duplicate filename '/devices/platform/soc/4c010010.usb/software_node'
  [  173.981564] CPU: 2 UID: 0 PID: 536 Comm: sh Not tainted 6.12.0-rc6-06344-g2aed7c4a5c56 #144
  [  173.989923] Hardware name: NXP i.MX95 15X15 board (DT)
  [  173.995062] Call trace:
  [  173.997509]  dump_backtrace+0x90/0xe8
  [  174.001196]  show_stack+0x18/0x24
  [  174.004524]  dump_stack_lvl+0x74/0x8c
  [  174.008198]  dump_stack+0x18/0x24
  [  174.011526]  sysfs_warn_dup+0x64/0x80
  [  174.015201]  sysfs_do_create_link_sd+0xf0/0xf8
  [  174.019656]  sysfs_create_link+0x20/0x40
  [  174.023590]  software_node_notify+0x90/0x100
  [  174.027872]  device_create_managed_software_node+0xec/0x108
  ...

  The '4c010010.usb' device is a platform device created during the initcall
  and is never removed, which causes its associated software node to persist
  indefinitely.

  The existing device_create_managed_software_node() does not provide a
  corresponding removal function.

  Replace device_create_managed_software_node() with the
  device_add_software_node() and device_remove_software_node() pair to ensure
  proper addition and removal of software nodes, addressing this issue.

  Fixes: a9400f1 ("usb: dwc3: imx8mp: add 2 software managed quirk properties for host mode")
  Cc: [email protected]
  Reviewed-by: Frank Li <[email protected]>
  Signed-off-by: Xu Yang <[email protected]>
  Acked-by: Thinh Nguyen <[email protected]>
  Link: https://lore.kernel.org/r/[email protected]
  Signed-off-by: Greg Kroah-Hartman <[email protected]>

Signed-off-by: Desnes Nunes <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants