On 10.07.25 17:48, Bart Van Assche wrote:
> On 7/9/25 10:55 PM, Johannes Thumshirn wrote:
>> On 09.07.25 17:31, Bart Van Assche wrote:
>>> Has it been considered to add a warning statement in blk_fill_rwbs()
>>> that verifies that blk_fill_rwbs() does not write outside the bounds of
>>> the rwbs array? See also the RWBS_LEN definition.
>>
>> $ git grep -E "#define\sRWBS_LEN"
>> include/trace/events/block.h:#define RWBS_LEN 9
>>
>> So even if we would have
>>
>> opf = (REQ_PREFLUSH | REQ_OP_ZONE_APPEND | REQ_FUA | REQ_RAHEAD |
>> REQ_SYNC | REQ_META | REQ_ATOMIC);
>>
>> it'll be 8 including the trailing \0 it'll be 9.
>>
>> If you look closely, REQ_OP_SECURE_ERASE already is 'DE' so no changes.
>
> It seems like my comment was not clear enough. I am aware that the
> current code does not trigger a buffer overflow. Adding a length check
> would help in my opinion because:
> - It would catch potential future changes of blk_fill_rwbs() that
> introduce a buffer overflow.
> - It would document the length of the rwbs output buffer. Today there
> are no references to RWBS_LEN in the blk_fill_rwbs() function -
> neither in the code nor in any comments.
Do you mean something like this:
diff --git a/kernel/trace/blktrace.c b/kernel/trace/blktrace.c
index f1dc00c22e37..ac30cb83067f 100644
--- a/kernel/trace/blktrace.c
+++ b/kernel/trace/blktrace.c
@@ -1911,6 +1911,8 @@ void blk_fill_rwbs(char *rwbs, blk_opf_t opf)
if (opf & REQ_ATOMIC)
rwbs[i++] = 'U';
+ WARN_ON_ONCE(i >= RWBS_LEN);
+
rwbs[i] = '\0';
}
EXPORT_SYMBOL_GPL(blk_fill_rwbs);
