-
Notifications
You must be signed in to change notification settings - Fork 8
Tmp two patch #116
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
base: code-review
Are you sure you want to change the base?
Tmp two patch #116
Conversation
In debugging a recent problem with an xfstest, noticed that we weren't
tracing cases where the ioctl was not supported. Add dynamic tracepoint:
"trace-cmd record -e smb3_unsupported_ioctl"
and then after running an app which calls unsupported ioctl,
"trace-cmd show"would display e.g.
xfs_io-7289 [012] ..... 1205.137765: smb3_unsupported_ioctl: xid=19 fid=0x4535bb84 ioctl cmd=0x801c581f
Signed-off-by: Steve French <[email protected]>
struct copychunk_ioctl_req::ChunkCount is annotated with __counted_by_le() as the number of elements in Chunks[]. smb2_copychunk_range reuses ChunkCount to store the number of chunks sent in the current iteration. If a later iteration populates more chunks than a previous one, the stale smaller value trips UBSAN. Set ChunkCount to chunk_count (allocated capacity) before populating Chunks[]. Fixes: cc26f59 ("smb: move copychunk definitions to common/smb2pdu.h") Link: https://lore.kernel.org/linux-cifs/CAH2r5ms9AWLy8WZ04Cpq5XOeVK64tcrUQ6__iMW+yk1VPzo1BA@mail.gmail.com Tested-by: Youling Tang <[email protected]> Signed-off-by: Henrique Carvalho <[email protected]> Signed-off-by: Steve French <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR implements two security and observability improvements for the SMB client: adding tracing for unsupported ioctl operations and properly initializing the ChunkCount field to satisfy flexible array bounds checking annotations.
- Added trace event for unsupported ioctl commands to improve debugging and monitoring
- Implemented proper initialization of ChunkCount field before array access to satisfy __counted_by_le annotation requirements
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| fs/smb/client/trace.h | Defines new smb3_unsupported_ioctl trace event using the existing DEFINE_SMB3_IOCTL_EVENT macro |
| fs/smb/client/ioctl.c | Adds trace call in the default case to log unsupported ioctl commands with xid, file id, and command |
| fs/smb/client/smb2ops.c | Sets ChunkCount to allocated size before populating Chunks array to satisfy __counted_by_le bounds checking requirements |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Enabling runtime PM before attaching the QPHY instance as driver data can lead to a NULL pointer dereference in runtime PM callbacks that expect valid driver data. There is a small window where the suspend callback may run after PM runtime enabling and before runtime forbid. This causes a sporadic crash during boot: ``` Unable to handle kernel NULL pointer dereference at virtual address 00000000000000a1 [...] CPU: 0 UID: 0 PID: 11 Comm: kworker/0:1 Not tainted 6.16.7+ #116 PREEMPT Workqueue: pm pm_runtime_work pstate: 20000005 (nzCv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--) pc : qusb2_phy_runtime_suspend+0x14/0x1e0 [phy_qcom_qusb2] lr : pm_generic_runtime_suspend+0x2c/0x44 [...] ``` Attach the QPHY instance as driver data before enabling runtime PM to prevent NULL pointer dereference in runtime PM callbacks. Reorder pm_runtime_enable() and pm_runtime_forbid() to prevent a short window where an unnecessary runtime suspend can occur. Use the devres-managed version to ensure PM runtime is symmetrically disabled during driver removal for proper cleanup. Fixes: 891a96f ("phy: qcom-qusb2: Add support for runtime PM") Signed-off-by: Loic Poulain <[email protected]> Reviewed-by: Dmitry Baryshkov <[email protected]> Reviewed-by: Abel Vesa <[email protected]> Link: https://patch.msgid.link/[email protected] Signed-off-by: Vinod Koul <[email protected]>
No description provided.