[26.04_linux-nvidia-bos] cxl: Backport Type-2 memdev lifetime fixes#483
Draft
nirmoy wants to merge 7 commits into
Draft
[26.04_linux-nvidia-bos] cxl: Backport Type-2 memdev lifetime fixes#483nirmoy wants to merge 7 commits into
nirmoy wants to merge 7 commits into
Conversation
Collaborator
Author
|
@JiandiAnNVIDIA, could you please review and test this upstream follow-up? This draft also tracks the linux-cxl/LKML submission plan. |
Collaborator
Author
BaseOS Kernel ReviewSummaryCXL Type-2 memdev backport has Critical issues: a double put_device causing refcount underflow/UAF and an ABBA deadlock between kill_regions and endpoint_unregister_region over regions_lock. cxl_reset_done() also risks an oops dereferencing an ERR_PTR endpoint. Findings: Critical: 2, High: 2, Medium: 1, Low: 4 Latest watcher review: open review Generated test plan: open test plan Kernel deb build: successful (download debs, 4 files) Head: This comment is maintained by nv-pr-bot. It is updated when the GitHub watcher publishes a newer review. |
Collaborator
|
@nirmoy: The change looks okay, but shouldn't this be a "SAUCE" patch? |
Collaborator
Author
Ack. I missed that. I should also add few other upstream fixes which codex seems to missed while creating the PR |
Collaborator
|
For which kernel is this? 7.0-bos? |
Collaborator
Author
|
nirmoy
force-pushed
the
codex/cxl-attach-action-deadlock-fix
branch
from
July 6, 2026 18:00
89e2944 to
1fd99fe
Compare
nirmoy
changed the title
Collaborator
Author
|
Scope corrected: PR #483 now targets 7.0 BOS and carries the complete dependent lifetime stack. The BOS-specific combined migration and the not-yet-upstream deadlock fix are marked SAUCE. The clean upstream patch is preserved separately for linux-cxl/LKML submission. @JiandiAnNVIDIA, please review and test this BOS backport. |
Contributor
PR Validation ReportPatchscan ✅ No Missing FixesAll cherry-picked commits checked — no missing upstream fixes found. PR Lint ✅ All checks passedDetailsChecking 7 commits... Cherry-pick digest: ┌──────────────┬──────────────────────────────────────────────────────────────────┬────────────┬─────────┬───────────────────────────┐ │ Local │ Referenced upstream / Patch subject │ Patch-ID │ Subject │ SoB chain │ ├──────────────┼──────────────────────────────────────────────────────────────────┼────────────┼─────────┼───────────────────────────┤ │ b25396b9f8fe │ [SAUCE] cxl/region: avoid attach action self-deadlock │ N/A │ N/A │ nirmoyd │ ├──────────────┼──────────────────────────────────────────────────────────────────┼────────────┼─────────┼───────────────────────────┤ │ fc371e67ebf8 │ [SAUCE] cxl: backport the mainline type-2 memdev lifetime model │ N/A │ N/A │ nirmoyd │ ├──────────────┼──────────────────────────────────────────────────────────────────┼────────────┼─────────┼───────────────────────────┤ │ fdcd17096bd3 │ cxl/region: block region delete during region creation │ noted │ found │ ok, backporter: nirmoyd │ ├──────────────┼──────────────────────────────────────────────────────────────────┼────────────┼─────────┼───────────────────────────┤ │ ddc5989a3af4 │ 574eda81d0a7 cxl/memdev: Hold memdev lock during memdev poison i │ match │ match │ preserved + nirmoyd added │ ├──────────────┼──────────────────────────────────────────────────────────────────┼────────────┼─────────┼───────────────────────────┤ │ f2a0894d5f1e │ cxl/pci: check memdev driver binding status in cxl_reset_done() │ noted │ found │ ok, backporter: nirmoyd │ ├──────────────┼──────────────────────────────────────────────────────────────────┼────────────┼─────────┼───────────────────────────┤ │ b4bc42f3a440 │ dc372e5f429c cxl/pci: Hold memdev lock in cxl_event_trace_record │ match │ match │ preserved + nirmoyd added │ ├──────────────┼──────────────────────────────────────────────────────────────────┼────────────┼─────────┼───────────────────────────┤ │ 994b97bce014 │ bb729bf1d6fd driver core: Add conditional guard support for devi │ match │ match │ preserved + nirmoyd added │ └──────────────┴──────────────────────────────────────────────────────────────────┴────────────┴─────────┴───────────────────────────┘ Lint: all checks passed. |
Introduce conditional guard version of device_lock() for scenarios that require conditional device lock holding. Suggested-by: Dan Williams <dan.j.williams@intel.com> Reviewed-by: Dan Williams <dan.j.williams@intel.com> Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> Signed-off-by: Li Ming <ming.li@zohomail.com> Link: https://patch.msgid.link/20260310-fix_access_endpoint_without_drv_check-v1-1-94fe919a0b87@zohomail.com Signed-off-by: Danilo Krummrich <dakr@kernel.org> (cherry picked from commit bb729bf) Signed-off-by: Nirmoy Das <nirmoyd@nvidia.com>
cxl_event_config() invokes cxl_mem_get_event_record() to get remain event logs from CXL device during cxl_pci_probe(). If CXL memdev probing failed before that, it is possible to access an invalid endpoint. So adding a cxlmd->driver binding status checking inside cxl_dpa_to_region() to ensure the corresponding endpoint is valid. Besides, cxl_event_trace_record() needs to hold memdev lock to invoke cxl_dpa_to_region() to ensure the memdev probing completed. It is possible that cxl_event_trace_record() is invoked during the CXL memdev probing, especially user or cxl_acpi triggers CXL memdev re-probing. Suggested-by: Dan Williams <dan.j.williams@intel.com> Reviewed-by: Dan Williams <dan.j.williams@intel.com> Reviewed-by: Dave Jiang <dave.jiang@intel.com> Signed-off-by: Li Ming <ming.li@zohomail.com> Link: https://patch.msgid.link/20260314-fix_access_endpoint_without_drv_check-v2-3-4c09edf2e1db@zohomail.com Signed-off-by: Dave Jiang <dave.jiang@intel.com> (cherry picked from commit dc372e5) Signed-off-by: Nirmoy Das <nirmoyd@nvidia.com>
cxl_reset_done() accesses the endpoint of the corresponding CXL memdev without endpoint validity checking. By default, cxlmd->endpoint is initialized to -ENXIO, if cxl_reset_done() is triggered after the corresponding CXL memdev probing failed, this results in access to an invalid endpoint. CXL subsystem can always check CXL memdev driver binding status to confirm its endpoint validity. So adding the CXL memdev driver checking inside cxl_reset_done() to avoid accessing an invalid endpoint. Fixes: 934edcd ("cxl: Add post-reset warning if reset results in loss of previously committed HDM decoders") Reviewed-by: Dan Williams <dan.j.williams@intel.com> Reviewed-by: Dave Jiang <dave.jiang@intel.com> Signed-off-by: Li Ming <ming.li@zohomail.com> Link: https://patch.msgid.link/20260314-fix_access_endpoint_without_drv_check-v2-4-4c09edf2e1db@zohomail.com Signed-off-by: Dave Jiang <dave.jiang@intel.com> (backported from https://lore.kernel.org/r/20260314-fix_access_endpoint_without_drv_check-v2-4-4c09edf2e1db@zohomail.com) [nirmoyd: Adapt to the existing BOS NULL/ERR_PTR endpoint guard.] Signed-off-by: Nirmoy Das <nirmoyd@nvidia.com>
cxl_dpa_to_region() assumes that it is running a context where it is not racing changes to "cxlmd->dev.driver". Acquire the memdev device lock in the debugfs entry points to preclude debugfs usage racing cxl_mem driver detach. Suggested-by: Dan Williams <djbw@kernel.org> Reviewed-by: Dave Jiang <dave.jiang@intel.com> Reviewed-by: Alison Schofield <alison.schofield@intel.com> Reviewed-by: Dan Williams <djbw@kernel.org> Signed-off-by: Li Ming <ming.li@zohomail.com> Link: https://patch.msgid.link/20260423111949.177399-1-ming.li@zohomail.com Signed-off-by: Dave Jiang <dave.jiang@intel.com> (cherry picked from commit 574eda8) Signed-off-by: Nirmoy Das <nirmoyd@nvidia.com>
Expand the range lock, rename it "regions_lock", to disable region deletion in the critical period between construct_region() and attach_target(), as well as the period between device_add() and registering the remove actions. Otherwise, userspace can confuse the kernel. It can violate the assumption the region stays registered through the completion of cxl_add_to_region(). It can violate the assumption that devm_add_action_or_reset() is working with a live 'struct cxl_region'. It is ok for the region to disappear outside of those windows as that mirrors device hotplug flows where the proper locks are held. Fixes: a32320b ("cxl/region: Add region autodiscovery") Signed-off-by: Dan Williams <djbw@kernel.org> Reviewed-by: Alejandro Lucero <alucerop@amd.com> Tested-by: ALejandro Lucero <alucerop@amd.com> Reviewed-by: Dave Jiang <dave.jiang@intel.com> Link: https://patch.msgid.link/20260519210158.1499795-2-djbw@kernel.org Signed-off-by: Dave Jiang <dave.jiang@intel.com> (backported from https://lore.kernel.org/r/20260519210158.1499795-2-djbw@kernel.org) [nirmoyd: Extend regions_lock to the downstream cxl_create_region().] Signed-off-by: Nirmoy Das <nirmoyd@nvidia.com>
…model BOS carries a callback-based Type-2 attach API and exported region helpers that are incompatible with the mainline region lifetime model. Applying the mainline deletion-race fixes separately would leave an unsafe intermediate callback lifecycle. Land the dependent transition as one atomic backport. Combine the upstream changes that resolve region deletion races, pin memdev parents for their full lifetime, distinguish class memory devices, introduce devm_cxl_probe_mem(), and support builds without CONFIG_CXL_REGION. Adapt the transition to BOS by migrating both in-tree Type-2 clients, SFC and VFIO, to devm_cxl_probe_mem(). Remove the obsolete public decoder, DPA, HPA, and region helper surface and the callback-based region teardown. The VFIO path now enforces the single firmware-committed decoder contract that its probe path already requires and propagates confirmed-CXL setup failures instead of exposing partially initialized plain VFIO state. Release the SFC CXL mapping while its manually managed probe state is still live, and reject failed VFIO PCI configuration reads before publishing the CXL memdev. Fold in the upstream !CONFIG_CXL_REGION stub so every published prefix remains buildable. Link: https://patch.msgid.link/20260519210158.1499795-3-djbw@kernel.org Link: https://patch.msgid.link/20260519210158.1499795-4-djbw@kernel.org Link: https://patch.msgid.link/20260519210158.1499795-5-djbw@kernel.org Link: https://patch.msgid.link/20260519210158.1499795-6-djbw@kernel.org Link: https://patch.msgid.link/20260610001324.260268-1-dave.jiang@intel.com Signed-off-by: Nirmoy Das <nirmoyd@nvidia.com>
devm_add_action_or_reset() synchronously invokes endpoint_unregister_region() on allocation failure while cxl_memdev_attach_region() still holds cxl_rwsem.region for read. Region teardown then tries to acquire that lock for write and self-deadlocks. Use devm_add_action() and, on failure, leave the region and DPA read-lock scope before invoking endpoint_unregister_region() directly. Keep the endpoint device locked so the topology remains stable while the failed action registration is unwound. Fixes: fc371e6 ("NVIDIA: VR: SAUCE: cxl: Backport the mainline Type-2 memdev lifetime model") Signed-off-by: Nirmoy Das <nirmoyd@nvidia.com>
nirmoy
force-pushed
the
codex/cxl-attach-action-deadlock-fix
branch
from
July 6, 2026 18:15
1fd99fe to
b25396b
Compare
Collaborator
|
This commit fc371e6 is like pulling all those links listed at the commit? we can not do a commit for each link maybe for more clarity where the patch came from? |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Backport the complete mainline CXL Type-2 memdev lifetime model to the 7.0 BOS kernel. This replaces the narrow endpoint-pointer guard proposed in #482 with the locking and ownership model used by mainline.
The BOS adaptation migrates both in-tree Type-2 users, SFC and VFIO, to
devm_cxl_probe_mem(), removes the callback-based region helper surface, and ties endpoint, memdev, and region teardown to the consumer lifetime.Patch stack
The first five commits bring in the prerequisite locking and region protection:
bb729bf1d6fd— conditionaldevice_lock()guard supportdc372e5f429c— hold the memdev lock in event tracinge8069c66d093— validate the memdev driver incxl_reset_done()(adapted to the existing BOS guard)574eda81d0a7— hold the memdev lock during poison operationsd91feb88692e— block region deletion during region creation (adapted for downstreamcxl_create_region())The first SAUCE commit combines and adapts the remaining accepted mainline lifetime series:
4dd86ca99ffc— resolve region deletion racesbd3a6ff4b84e— pin memdev parents for their full lifetime2ed519c21bb4— distinguish class memory devicesd8dcb0b74b04— introducedevm_cxl_probe_mem()383f69656359— support!CONFIG_CXL_REGIONThe final SAUCE commit fixes a self-deadlock if devres action allocation fails while the region read lock is held.
Behavior change
After a device is confirmed as CXL.mem, the VFIO path now fails binding when the required topology or firmware-committed region is unavailable instead of leaving a published memdev or silently falling back to partially initialized plain VFIO state. Jiandi should validate this policy on the target hardware.
This stack supersedes #482 after functional validation; the guard PR and this lifetime backport should not both be merged.
Upstream plan
The dependency stack is already accepted in mainline 7.2. The clean upstream version of the new deadlock fix is preserved on codex/cxl-attach-action-deadlock-fix-upstream.
After fault-injection and lockdep validation, post that clean patch to
linux-cxl@vger.kernel.organdlinux-kernel@vger.kernel.org, then track review on lore. The combined SFC/VFIO migration remains BOS SAUCE because it adapts downstream Type-2 users.Current validation
git diff --check: passCONFIG_CXL_REGION=y,CONFIG_SFC_CXL=y, andCONFIG_VFIO_CXL_CORE=yChange-Id,BugLink, or internal bug URL trailersThe target branch's older checkpatch reports inherited
ACQUIRE_ERR()assignment-in-condition findings in two accepted upstream backports; no local rewrite was made to those upstream changes.Pending validation
Tracking