Repository navigation
Fix bugs found by fuzzing the device data - #4223
Merged
Merged
Conversation
The --raw-binary flag only sets cfg.raw_binary when it is given, so without it query-cap reads an uninitialized bool. UBSan reports it while fuzzing the device data: sfx-nvme.c:731:12: runtime error: load of value 192, which is not a valid value for type 'bool' Initialize the options like the other commands do. Signed-off-by: Daniel Wagner <dwagner@suse.de>
The stdout and the JSON printer walk the descriptors with the sizes the
device reports, without checking them against the length of the log.
A descriptor at the end of the log is read beyond it, and a size larger
than the rest of the log moves the walk out of it:
ERROR: AddressSanitizer: heap-buffer-overflow
READ of size 8
#0 json_hwcomp_log plugins/ocp/ocp-print-json.c:131
Add hwcomp_desc_entry(), which returns a descriptor only if its header,
date/lot code and additional information fit into the rest of the log,
and walk the descriptors with it in both printers.
Signed-off-by: Daniel Wagner <dwagner@suse.de>
The Event Header Length counts the bytes of the event header after the
first 3, but the printers of the Persistent Event log read all header
fields of an event however short its header is. An event of a zeroed
log has only 3 bytes, so a log of 2 MiB is printed as some 700000
events, which takes minutes:
ALARM: working on the last Unit for 15 seconds
linux-nvme#4 shr_table_align_column shared/table-util.c:353
linux-nvme#5 stdout_kv_table_finish src/nvme-print-stdout.c:609
linux-nvme#6 nvme_show_pel_event_header src/nvme-print-stdout-log.c:244
The same walk is in five printers. Add nvme_pel_event(), which returns
an event only if its header holds all header fields, its vendor
specific information fits into the event and the event into the log,
and use it in all of them.
Signed-off-by: Daniel Wagner <dwagner@suse.de>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Three bugs found by fuzzing the device data with
fuzz-device(scripts/build.sh fuzz, 16 jobs):--raw-binary,query-capreads an uninitializedbool(UBSan:load of value 192, which is not a valid value for type 'bool'). It was the only uninitialized option struct in the tree.hwcomp_desc_entry()returns a descriptor only if its header, date/lot code and additional information fit into the rest of the log, and both printers use it.nvme_pel_event()returns an event only if its header holds all header fields (EHL + 3 bytes), its vendor specific information fits into the event and the event into the log.Each fixed input now runs cleanly in all output variants (the persistent event one in 0.08 s instead of over 2 minutes). The test suite passes, and a build without json-c compiles.
Not fixed here: a reachability groups log whose device claims a group of ~1.9 million namespace IDs is fetched and printed in full. That's slow but correct, and part of the open question of how far to trust device-reported sizes.