bdev/nvme: prevent UAF by unregistering connect poller and null-guarding qpair#68
Conversation
There was a problem hiding this comment.
Pull request overview
This PR addresses a Use-After-Free (UAF) vulnerability in the NVMe bdev module that can occur during connect/disconnect/reset sequences. The fix adds defensive guards in the connect poller callback to detect when resources have been freed, and ensures the poller is unregistered during channel destruction before clearing the qpair pointer.
Key changes:
- Added NULL checks in
bdev_nvme_reset_check_qpair_connectedto prevent accessing freed nvme_qpair or controller resources - Ensured
connect_polleris unregistered inbdev_nvme_destroy_ctrlr_channel_cbbefore clearing pointers to prevent callbacks from accessing freed memory - Added bidirectional NULL-ing of qpair references (both
ctrlr_ch->qpairandnvme_qpair->ctrlr_ch) to prevent dangling pointers
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review for a chance to win a $100 gift card. Take the survey.
…ing qpair longhorn/longhorn-11698 Signed-off-by: Chin-Ya Huang <chin-ya.huang@suse.com>
647965a to
fcef5df
Compare
| "Unregistering connect_poller %p during channel destroy " | ||
| "(ctrlr_ch=%p, nvme_qpair=%p).\n", | ||
| ctrlr_ch->connect_poller, ctrlr_ch, nvme_qpair); | ||
| spdk_poller_unregister(&ctrlr_ch->connect_poller); |
There was a problem hiding this comment.
I haven't checked the codes for registering and unregistering connect poller. Some questions
- When will the connect register be registered?
- It seems the UAF doesn't always happen. Do you know why?
There was a problem hiding this comment.
When will the connect register be registered?
It's registered during reset/failover: https://github.com/longhorn/spdk/blob/longhorn-v25.05/module/bdev/nvme/bdev_nvme.c#L2460. A detach during this window can trigger a race.
It seems the UAF doesn't always happen. Do you know why?
It’s a race between three async pieces that can arrive in different orders:
- Channel destroy callback (
bdev_nvme_destroy_ctrlr_channel_cb) - Disconnect callback (
bdev_nvme_disconnected_qpair_cb) - The connect poller firing (
bdev_nvme_reset_check_qpair_connected)
Which issue(s) this PR fixes:
Issue longhorn/longhorn#11698
What this PR does / why we need it:
longhorn/longhorn#11698 (comment)
Special notes for your reviewer:
Additional documentation or context