| Message ID | 20241007163654.499827-1-stephen@networkplumber.org (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | f4b86f752d3c38d682fc1a5c55c67c8006c23800 |
| Delegated to: | Stephen Hemminger |
| Headers | show |
| Series | [iproute] bridge: catch invalid stp state | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Not a local patch |
Hello: This patch was applied to iproute2/iproute2.git (main) by Stephen Hemminger <stephen@networkplumber.org>: On Mon, 7 Oct 2024 09:35:43 -0700 you wrote: > The stp state parsing was putting result in an __u8 which > would mean that check for invalid string was never happening. > > Caught by enabling -Wextra: > CC mst.o > mst.c: In function ‘mst_set’: > mst.c:217:27: warning: comparison is always false due to limited range of data type [-Wtype-limits] > 217 | if (state == -1) { > > [...] Here is the summary with links: - [iproute] bridge: catch invalid stp state https://git.kernel.org/pub/scm/network/iproute2/iproute2.git/commit/?id=f4b86f752d3c You are awesome, thank you!
diff --git a/bridge/mst.c b/bridge/mst.c index fccb7fd6..32f64aba 100644 --- a/bridge/mst.c +++ b/bridge/mst.c @@ -176,7 +176,7 @@ static int mst_set(int argc, char **argv) char *d = NULL, *m = NULL, *s = NULL, *endptr; struct rtattr *af_spec, *mst, *entry; __u16 msti; - __u8 state; + int state; while (argc > 0) { if (strcmp(*argv, "dev") == 0) { @@ -212,13 +212,12 @@ static int mst_set(int argc, char **argv) } state = strtol(s, &endptr, 10); - if (!(*s != '\0' && *endptr == '\0')) { + if (!(*s != '\0' && *endptr == '\0')) state = parse_stp_state(s); - if (state == -1) { - fprintf(stderr, - "Error: invalid STP port state\n"); - return -1; - } + + if (state < 0 || state > UINT8_MAX) { + fprintf(stderr, "Error: invalid STP port state\n"); + return -1; } af_spec = addattr_nest(&req.n, sizeof(req), IFLA_AF_SPEC);
The stp state parsing was putting result in an __u8 which would mean that check for invalid string was never happening. Caught by enabling -Wextra: CC mst.o mst.c: In function ‘mst_set’: mst.c:217:27: warning: comparison is always false due to limited range of data type [-Wtype-limits] 217 | if (state == -1) { Fixes: dae3e5de6eac ("bridge: mst: Add get/set support for MST states") Signed-off-by: Stephen Hemminger <stephen@networkplumber.org> --- bridge/mst.c | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-)