Skip to content

Fix device removal locking#46

Merged
jserv merged 1 commit into
sysprog21:masterfrom
Shaoen-Lin:fix-device-removal-loop
Jun 5, 2026
Merged

Fix device removal locking#46
jserv merged 1 commit into
sysprog21:masterfrom
Shaoen-Lin:fix-device-removal-loop

Conversation

@Shaoen-Lin

@Shaoen-Lin Shaoen-Lin commented Jun 1, 2026

Copy link
Copy Markdown
Contributor

control_iocontrol_destroy_device() reads a vcam_devices entry before
taking vcam_devices_lock, but later removes entries from the same
array while holding the lock. Concurrent destroy requests can therefore
use a stale index or device pointer after another request shifts the
array.

Take vcam_devices_lock before validating the index and reading the
device pointer. Remove the device from vcam_devices while still
holding the lock, then release the lock before destroying the device.

Also stop the removal loop before i + 1 reaches vcam_device_count, so it
does not read past the last valid vcam_devices entry.


Summary by cubic

Fix race and off-by-one in device removal to prevent stale pointers and out-of-bounds access. Take vcam_devices_lock before validating the index and reading the device pointer, shift vcam_devices with i + 1 < vcam_device_count, then release the lock before destroying the device.

Written for commit f0fc97c. Summary will update on new commits.

Review in cubic

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

1 issue found across 1 file

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

Comment thread control.c
@Shaoen-Lin Shaoen-Lin force-pushed the fix-device-removal-loop branch 2 times, most recently from ce31656 to 5a032f0 Compare June 5, 2026 07:39
@Shaoen-Lin Shaoen-Lin changed the title Fix device removal loop bounds Fix device removal locking Jun 5, 2026

@jserv jserv left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Indent with clang-format-18

control_iocontrol_destroy_device() reads a vcam_devices entry before
taking vcam_devices_lock, but later removes entries from the same array
while holding the lock. Concurrent destroy requests can therefore use a
stale index or device pointer after another request shifts the array.

Take vcam_devices_lock before validating the index and reading the
device pointer. Remove the device from vcam_devices while still holding
the lock, then release the lock before destroying the device.

The removal loop shifts vcam_devices entries left by reading
vcam_devices[i + 1]. The old loop allowed i to reach
vcam_device_count - 1, so the last iteration read one entry past the
valid array range. Stop the loop before i + 1 reaches
vcam_device_count.

Signed-off-by: Shaoen-Lin <shaoen.lin92@gmail.com>
@Shaoen-Lin Shaoen-Lin force-pushed the fix-device-removal-loop branch from 5a032f0 to f0fc97c Compare June 5, 2026 08:47
@Shaoen-Lin Shaoen-Lin closed this Jun 5, 2026
@Shaoen-Lin Shaoen-Lin reopened this Jun 5, 2026
@jserv jserv merged commit 1b02973 into sysprog21:master Jun 5, 2026
4 checks passed
@jserv

jserv commented Jun 5, 2026

Copy link
Copy Markdown
Collaborator

Thank @Shaoen-Lin for contributing!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants