Re: [PATCH] smb: client: reject a tree connect response whose byte count is too small

[Bryam Vargas <hexlabsecurity@proton.me>]

Namjae,

> Can you check the build warning that was reported by the kernel test robot ?

It's mine. The three sites it points at are in the write path, but the
cause is in trace.h.

enum smb_eio_trace is __mode(byte), and the list has held exactly 128
entries since f80ac7eda1cf added it, which puts the last one,
smb_eio_trace_write_too_far, at index 127. My patch inserts
smb_eio_trace_tcon_bcc_too_small ahead of tdis_in_reconnect, everything
after it shifts up one, and write_too_far lands on 128.

It is worse than a warning where CONFIG_WERROR is set, and x86_64 defconfig
sets it: clang gives the enum a signed underlying type, converts the value
to -128 and the build stops. I reproduced that at v7.2 with clang 19.1.7 --
three errors and cifssmb.o fails, so v1 as it stands does not build there.
gcc gives the same enum an unsigned underlying type and says nothing, which
is why the tree had not run into it. Where WERROR is off, the recorded
value stops matching the __print_symbolic() table and those three events
print a raw number instead of their name.

So the enum is full for everyone, not only for me: the next
smb_eio_trace_* anyone adds lands on 128 too, and under gcc it does it
quietly. Of the 68 __mode(byte) enums in the tree this was the only one
with no room left, though rxrpc_abort_reason has 127 entries.

v2 is two patches: 1/2 drops __mode(byte) from enum smb_eio_trace, 2/2 is
the tree connect fix unchanged. I dropped the attribute rather than
widening it because on x86_64 the record does not grow either way -- the
field precedes an unsigned long at offset 8, so sizeof(struct
trace_event_raw_smb3_eio) is 32 whichever type the enum gets -- and a plain
enum avoids a spelling the tree does not use (all 68 __mode() uses are
__mode(byte)). If you or David would rather keep it packed and widen it,
say so and I'll respin.

Two corrections to v1 while I am here, both in the changelog rather than
the code. The initialised prefix is 256 bytes, not the 67 I wrote:
header_assemble() memsets 256 right after cifs_buf_get(), so the byte area
only starts past it from WordCount 111. And the impact is understated --
once bytes_left wraps, the bound handed to cifs_strndup_from_utf16() can
reach 65535 against a ~16 KB cifs_req_poolp object, so this is a slab
out-of-bounds read and not only an uninitialised one.

v2 is on the list:
https://lore.kernel.org/all/20260820-b4-disp-58f78a28-v2-0-1fb7a6cb1533@proton.me/

Thanks for the review.

Bryam