Re: [PATCH net 1/2] net: dsa: mxl862xx: bound FDB dump walk by table capacity

[memoriaru@foxmail.com]

From: Zhang Hewei <memoriaru@foxmail.com>

Thanks -- reviewed both Medium findings against the tree. Planning a
v2; replies to each below.

[Medium] off-by-one in the capacity check: the fix is warranted, with
one nuance on the premise. The kernel-doc for @last says "Indicates
that the read operation returned last entry", which reads as last
being set on the response that carries the final entry -- under that
reading, a walk of exactly MXL862XX_MAC_TABLE_FDB_SIZE entries
completes before the check trips. If instead the terminal response
carries no entry (and still counts as a read), the current order
rejects a correctly ended full table. The public headers do not
settle which it is, so v2 moves the param.last test above the count
check and drops the protocol dependency entirely; the macro name and
the error message get reworded to "conservative upper bound" to match
the comment.

[Medium] is rejecting only bridge_id 0 enough: fair. INVALID_HANDLE
is referenced by the API docs but never defined or checked by the
driver, so if it is 0xffff (the MaxLinear GSW convention), a failed
allocation would currently be consumed as a FID. v2 rejects 0xffff as
well, and range-checks the id once the driver-side bridge limit is
confirmed. Duplicate-id detection: the cross-domain fdb_del
consequence you describe is real; I'll see how far validation can go
without inventing API facts that are absent from the public headers.

The Lows are accepted too: the comment hunk moves into patch 1, the
"growing kernel memory without bound" wording is dropped (reported
entries consume the fixed netlink dump skb and -EMSGSIZE aborts the
walk; the unbounded case is a device answering forever with entries
that are filtered out before the callback), Fixes: tags will be
added, and the self Reported-by trailer goes away.

pw-bot: cr