log: fix overflow for very large telemetry logs - #4002
Conversation
| __func__, libnvme_strerror(errno)); | ||
| break; | ||
| } else if (data_written <= data_remaining) { | ||
| } else if ((size_t)data_written <= data_remaining) { |
There was a problem hiding this comment.
casting types is a code smell, is there no other way to get this sorted out properly?
There was a problem hiding this comment.
Ah ok, replaced the write loop with shr_write_alI(). No casts and removes duplicated functionality.
There was a problem hiding this comment.
🟢 Approval recommended
The core type changes correctly address the reported overflow, with only a minor maintainability suggestion around explicit casting remaining.
Pull request overview
This PR fixes telemetry log extraction failures for very large logs by correcting signed/unsigned type usage in get_telemetry_log() and updating related formatting to match the new types.
Changes:
- Change
data_remainingfrominttosize_tto prevent overflow/negative values for logs >2GiB. - Change
data_writtenfrominttossize_tto matchwrite()’s return type. - Update the overwrite diagnostic format specifiers to use
%zxand cast accordingly.
File summaries
| File | Description |
|---|---|
| plugins/log/log-plugin.c | Adjusts telemetry log write-loop variable types and error formatting to handle very large log sizes safely. |
Review details
- Files reviewed: 1/1 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| } else if ((size_t)data_written <= data_remaining) { | ||
| data_remaining -= data_written; | ||
| data_ptr += data_written; |
c20b7c9 to
4d4334e
Compare
The data_written and data_remaining variables in get_telemetry_log() are defined as ints. The data_remaining variable is assigned from a size_t representing the total log size. If this is greater than 2 GiB, the log cannot be extracted as data_remaining goes negative. Replace telemetry log write loop with shr_write_all() helper which already uses a size_t for log length. Signed-off-by: Keith McKay <kpmckay@gmail.com>
4d4334e to
8fdc4fc
Compare
The data_written and data_remaining variables in get_telemetry_log() are defined as ints. The data_remaining variable is assigned from a size_t representing the total log size. If this is greater than 2 GiB, the log cannot be extracted as data_remaining goes negative. The data_written variable takes an ssize_t from write().
Change data_remaining to size_t to prevent the overflow, and data_written to ssize_t to match write(). Modify the format specifiers in nvme_show_error() accordingly.