Page MenuHomeFreeBSD

Always null-terminate ccb_pathinq.(sim_vid|hba_vid|dev_name) (part 2)
ClosedPublic

Authored by asomers on Jan 4 2017, 12:17 AM.
Tags
None
Referenced Files
Unknown Object (File)
Sat, Mar 23, 1:27 AM
Unknown Object (File)
Feb 18 2024, 5:47 PM
Unknown Object (File)
Feb 18 2024, 3:08 PM
Unknown Object (File)
Feb 18 2024, 2:37 PM
Unknown Object (File)
Feb 13 2024, 8:40 PM
Unknown Object (File)
Jan 21 2024, 6:06 PM
Unknown Object (File)
Jan 16 2024, 2:11 PM
Unknown Object (File)
Dec 22 2023, 11:11 PM
Subscribers

Details

Summary

The sim_vid, hba_vid, and dev_name fields of struct ccb_pathinq are
fixed-length strings. AFAICT the only place they're read is in
sbin/camcontrol/camcontrol.c, which assumes they'll be null-terminated.
However, the kernel doesn't null-terminate them. A bunch of copy-pasted code
uses strncpy to write them, and doesn't guarantee null-termination. For at
least 4 drivers (mpr, mps, ciss, and hyperv), the hba_vid field actually
overflows. You can see the result by doing "camcontrol negotiate da0 -v".

This change null-terminates those fields everywhere they're set in the
kernel. It also shortens a few strings to ensure they'll fit within the
16-character field.

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 6555
Build 6777: arc lint + arc unit

Event Timeline

asomers retitled this revision from to Always null-terminate ccb_pathinq.(sim_vid|hba_vid|dev_name) (part 2).
asomers updated this object.
asomers edited the test plan for this revision. (Show Details)
asomers added reviewers: ken, slm.
imp added a reviewer: imp.
imp added inline comments.
sys/dev/mfi/mfi_cam.c
226

Isn't that Marvell these days? Assuming MFI is still for say...

sys/dev/mly/mly.c
2113

This seems wrong MLY isn't an artificial device...

sys/dev/mpr/mpr_sas.c
991

Isn't that Marvell now?

sys/dev/mps/mps_sas.c
947

marvell?

sys/dev/mpt/mpt_cam.c
3587

Marvell?

sys/dev/mrsas/mrsas_cam.c
360

Marvel?

This revision is now accepted and ready to land.Jan 4 2017, 2:23 AM
slm edited edge metadata.
slm added inline comments.
sys/dev/mfi/mfi_cam.c
226

LSI, then Avago, then Broadcom (not Marvell). But, 'Broadcom' should not be used yet. I would keep them all as they are. LSI is OK for MFI and MPT, and Avago Tech (or AVAGO) is OK for the others.

sys/dev/mly/mly.c
2113

What would you suggest? "Mylex"? Or "Avago", because they got bought by LSI years ago?

sys/dev/mly/mly.c
2113

I don't know anything about this driver. I would use Mylex.