This is an archive of the discontinued LLVM Phabricator instance.

[MSAN] extend ioctl interceptor to support BLKSSZGET
ClosedPublic

Authored by kda on Feb 28 2022, 8:18 PM.

Diff Detail

Event Timeline

kda created this revision.Feb 28 2022, 8:18 PM
kda requested review of this revision.Feb 28 2022, 8:18 PM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 28 2022, 8:18 PM
Herald added a subscriber: Restricted Project. · View Herald Transcript
kda added inline comments.Feb 28 2022, 8:19 PM
compiler-rt/lib/msan/tests/msan_test.cpp
714 ↗(On Diff #411951)

Is there a device I can easily use to make this function work correctly?

eugenis added inline comments.Mar 1 2022, 11:34 AM
compiler-rt/lib/sanitizer_common/sanitizer_common_interceptors_ioctl.inc
605

Please add it to the list instead (the same as done for the other BLK* ioctls).
Also, could you check for more missing BLK ioctls? I see at least a couple in the system header.

eugenis added inline comments.Mar 1 2022, 11:37 AM
compiler-rt/lib/msan/tests/msan_test.cpp
714 ↗(On Diff #411951)

I'd say it's ok not to test it if it is hard to set up. Especially if you don't add any extra code, only a list entry.

kda updated this revision to Diff 412881.Mar 3 2022, 5:47 PM
kda marked 2 inline comments as done.

added to list and dropped test
add a few other BLK ioctls

Herald added a project: Restricted Project. · View Herald TranscriptMar 3 2022, 5:47 PM
kda added inline comments.Mar 3 2022, 5:47 PM
compiler-rt/lib/msan/tests/msan_test.cpp
714 ↗(On Diff #411951)

Dropping test.

eugenis added inline comments.Mar 3 2022, 6:22 PM
compiler-rt/lib/sanitizer_common/sanitizer_common_interceptors_ioctl.inc
123

some of these are wrong: *SET read from the argument, *GET write to the argument.

Also, some types are wrong, including in the existing code!

BLKRAGET / BLKFRAGET uses uptr,
BLKSECTGET uses short
BLKBSZGET uses int
BLKGETSIZE64 uses u64

Please double check against the kernel source.

128

uptr instead of SIZE_TYPE

kda updated this revision to Diff 413028.Mar 4 2022, 8:43 AM
kda marked an inline comment as done.

adjusted types and READ/WRITE

compiler-rt/lib/sanitizer_common/sanitizer_common_interceptors_ioctl.inc
123

Oh. That is embarrassing. Thank you.

I used this for the Linux code: https://elixir.bootlin.com/linux/latest/source/block/ioctl.c#L500
Is that the right place?

eugenis added inline comments.Mar 4 2022, 10:22 AM
compiler-rt/lib/sanitizer_common/sanitizer_common_interceptors_ioctl.inc
123

No problem! Some of these are still wrong though.
BLKFRASET should be NONE
BLKRAGET / BLKFRAGET should use uptr

kda updated this revision to Diff 413138.Mar 4 2022, 2:08 PM
kda marked an inline comment as done.

a few more adjustments

eugenis accepted this revision.Mar 4 2022, 2:11 PM

LGTM

This revision is now accepted and ready to land.Mar 4 2022, 2:11 PM
This revision was automatically updated to reflect the committed changes.