Several lengths and offsets the MUX downlink decoders work with are
supplied by the modem: the size of the received block comes from a
device written transfer descriptor, and the ADGH datagram length, the
ACBH block length, the command chain offsets and the length a command
header declares for its parameter are all read from the block itself.
None of them is bounded against what actually arrived, so a truncated or
enlarged DL block makes the driver read past the received data, hand the
network stack a payload the device never declared, walk a command chain
that points back at itself, or decode the bytes of one command a second
time as the command following it. The aggregated datagram block path is
already covered by commit 526b8ef54668 ("net: wwan: iosm: bound device
offsets in the MUX downlink decoder") and is not touched here.
This series adds those bounds. The checks only relate the device
supplied lengths and offsets to the received data: no command type,
response or protocol semantics change, so a conforming modem behaves
exactly as before. Patch 4 is the prep change for patch 5, it stops the
driver from copying the received parameter union by value and from
building its answer inside the received block.
cmd_len is not used the same way in the two paths. In the aggregated
path the parameter boundary is next_cmd_index, or the end of the
received block for the last command: the flow control decoder already
keeps a backward compatible handling for a cmd_len that does not match
the header and its parameter size, so the field is not a hard bound
there. In MUX Lite it is an upper bound when it covers at least the
header; a smaller value is not taken as a usable declaration and the
received length stays the limit. That is a compatibility tolerance, not
a valid input: the protocol requires 0x10 or more.
A parameter too short for the member its command type reads is dropped
with -EMSGSIZE, which also ends an aggregated block at that command.
Patch 3 also delimits the command chain by the received length:
block_length is checked to cover the ACBH header and not exceed the
received buffer, but skb->len is what bounds the commands. The
array_index_nospec() call of the walk is gone with these explicit
checks: it bounds a byte offset by sizeof(struct mux_cmdh), which
turns every legal offset past that size into zero, while the checks
constrain the same value against the received length.
array_index_nospec() is kept only for if_id, an actual array index.
Patch 2 relies on the ADGH header check introduced by patch 1, and patch
5 relies on the ACBH block validation from patch 3 and on the parameter
pointer introduced by patch 4. Patches 1 through 3 apply cleanly and
build individually against the base, the series applies cleanly and
every commit builds, W=1 drivers/net/wwan/iosm/ is clean.
Found by reviewing the DL decode path against the MUX protocol
structures. Not reproduced on a live modem: reaching it needs a device
that truncates or enlarges its own downlink blocks.
The series is based on the net tree at 145480b28ca8 and applies to it
cleanly.
Fang Xieyan (5):
net: wwan: iosm: validate the MUX DL block length
net: wwan: iosm: validate the ADGH datagram length
net: wwan: iosm: bound the ACBH command chain walk
net: wwan: iosm: pass the DL command parameter by pointer
net: wwan: iosm: validate the command parameter length
drivers/net/wwan/iosm/iosm_ipc_mux_codec.c | 197 ++++++++++++++++++++++-------
1 file changed, 151 insertions(+), 46 deletions(-)
--
2.50.1