Add strict buffer parsing index check to avoid the following Smatch warning:
net/core/pktgen.c:877 get_imix_entries() warn: check that incremented offset 'i' is capped
Checking the buffer index i after every get_user/i++ step and returning with error code immediately avoids the current indirect (but correct) error handling.
Reported-by: Dan Carpenter dan.carpenter@linaro.org Closes: https://lore.kernel.org/netdev/36cf3ee2-38b1-47e5-a42a-363efeb0ace3@stanley.... Signed-off-by: Peter Seiderer ps.report@gmx.net --- net/core/pktgen.c | 8 ++++++++ 1 file changed, 8 insertions(+)
diff --git a/net/core/pktgen.c b/net/core/pktgen.c index e850598db3e7..fe7fdefab994 100644 --- a/net/core/pktgen.c +++ b/net/core/pktgen.c @@ -856,6 +856,9 @@ static ssize_t get_imix_entries(const char __user *buffer, if (pkt_dev->n_imix_entries >= MAX_IMIX_ENTRIES) return -E2BIG;
+ if (i >= maxlen) + return -EINVAL; + max = min(10, maxlen - i); len = num_arg(&buffer[i], max, &size); if (len < 0) @@ -869,6 +872,8 @@ static ssize_t get_imix_entries(const char __user *buffer, if (c != ',') return -EINVAL; i++; + if (i >= maxlen) + return -EINVAL;
if (size < 14 + 20 + 8) size = 14 + 20 + 8; @@ -911,6 +916,9 @@ static ssize_t get_labels(const char __user *buffer, if (n >= MAX_MPLS_LABELS) return -E2BIG;
+ if (i >= maxlen) + return -EINVAL; + max = min(8, maxlen - i); len = hex32_arg(&buffer[i], max, &tmp); if (len < 0)
Add more imix_weights test cases (for incomplete input).
Signed-off-by: Peter Seiderer ps.report@gmx.net --- tools/testing/selftests/net/proc_net_pktgen.c | 44 +++++++++++++++++++ 1 file changed, 44 insertions(+)
diff --git a/tools/testing/selftests/net/proc_net_pktgen.c b/tools/testing/selftests/net/proc_net_pktgen.c index 462805ac1614..69444fb29577 100644 --- a/tools/testing/selftests/net/proc_net_pktgen.c +++ b/tools/testing/selftests/net/proc_net_pktgen.c @@ -39,6 +39,10 @@ static const char dev_cmd_pkt_size_0[] = "pkt_size 300"; static const char dev_cmd_imix_weights_0[] = "imix_weights 0,7 576,4 1500,1"; static const char dev_cmd_imix_weights_1[] = "imix_weights 101,1 102,2 103,3 104,4 105,5 106,6 107,7 108,8 109,9 110,10 111,11 112,12 113,13 114,14 115,15 116,16 117,17 118,18 119,19 120,20"; static const char dev_cmd_imix_weights_2[] = "imix_weights 100,1 102,2 103,3 104,4 105,5 106,6 107,7 108,8 109,9 110,10 111,11 112,12 113,13 114,14 115,15 116,16 117,17 118,18 119,19 120,20 121,21"; +static const char dev_cmd_imix_weights_3[] = "imix_weights"; +static const char dev_cmd_imix_weights_4[] = "imix_weights "; +static const char dev_cmd_imix_weights_5[] = "imix_weights 0"; +static const char dev_cmd_imix_weights_6[] = "imix_weights 0,"; static const char dev_cmd_debug_0[] = "debug 1"; static const char dev_cmd_debug_1[] = "debug 0"; static const char dev_cmd_frags_0[] = "frags 100"; @@ -284,6 +288,46 @@ TEST_F(proc_net_pktgen, dev_cmd_imix_weights) { len = write(self->dev_fd, dev_cmd_imix_weights_2, sizeof(dev_cmd_imix_weights_2)); EXPECT_EQ(len, -1); EXPECT_EQ(errno, E2BIG); + + /* with trailing '\0' */ + len = write(self->dev_fd, dev_cmd_imix_weights_3, sizeof(dev_cmd_imix_weights_3)); + EXPECT_EQ(len, -1); + EXPECT_EQ(errno, EINVAL); + + /* without trailing '\0' */ + len = write(self->dev_fd, dev_cmd_imix_weights_3, sizeof(dev_cmd_imix_weights_3) - 1); + EXPECT_EQ(len, -1); + EXPECT_EQ(errno, EINVAL); + + /* with trailing '\0' */ + len = write(self->dev_fd, dev_cmd_imix_weights_4, sizeof(dev_cmd_imix_weights_4)); + EXPECT_EQ(len, -1); + EXPECT_EQ(errno, EINVAL); + + /* without trailing '\0' */ + len = write(self->dev_fd, dev_cmd_imix_weights_4, sizeof(dev_cmd_imix_weights_4) - 1); + EXPECT_EQ(len, -1); + EXPECT_EQ(errno, EINVAL); + + /* with trailing '\0' */ + len = write(self->dev_fd, dev_cmd_imix_weights_5, sizeof(dev_cmd_imix_weights_5)); + EXPECT_EQ(len, -1); + EXPECT_EQ(errno, EINVAL); + + /* without trailing '\0' */ + len = write(self->dev_fd, dev_cmd_imix_weights_5, sizeof(dev_cmd_imix_weights_5) - 1); + EXPECT_EQ(len, -1); + EXPECT_EQ(errno, EINVAL); + + /* with trailing '\0' */ + len = write(self->dev_fd, dev_cmd_imix_weights_6, sizeof(dev_cmd_imix_weights_6)); + EXPECT_EQ(len, -1); + EXPECT_EQ(errno, EINVAL); + + /* without trailing '\0' */ + len = write(self->dev_fd, dev_cmd_imix_weights_6, sizeof(dev_cmd_imix_weights_6) - 1); + EXPECT_EQ(len, -1); + EXPECT_EQ(errno, EINVAL); }
TEST_F(proc_net_pktgen, dev_cmd_debug) {
On Mon, Mar 17, 2025 at 10:04:01AM +0100, Peter Seiderer wrote:
Add more imix_weights test cases (for incomplete input).
Signed-off-by: Peter Seiderer ps.report@gmx.net
Reviewed-by: Simon Horman horms@kernel.org
On Mon, Mar 17, 2025 at 10:04:00AM +0100, Peter Seiderer wrote:
Add strict buffer parsing index check to avoid the following Smatch warning:
net/core/pktgen.c:877 get_imix_entries() warn: check that incremented offset 'i' is capped
Checking the buffer index i after every get_user/i++ step and returning with error code immediately avoids the current indirect (but correct) error handling.
Reported-by: Dan Carpenter dan.carpenter@linaro.org Closes: https://lore.kernel.org/netdev/36cf3ee2-38b1-47e5-a42a-363efeb0ace3@stanley.... Signed-off-by: Peter Seiderer ps.report@gmx.net
Reviewed-by: Simon Horman horms@kernel.org
Hello:
This series was applied to netdev/net-next.git (main) by Jakub Kicinski kuba@kernel.org:
On Mon, 17 Mar 2025 10:04:00 +0100 you wrote:
Add strict buffer parsing index check to avoid the following Smatch warning:
net/core/pktgen.c:877 get_imix_entries() warn: check that incremented offset 'i' is capped
Checking the buffer index i after every get_user/i++ step and returning with error code immediately avoids the current indirect (but correct) error handling.
[...]
Here is the summary with links: - [net-next,v1,1/2] net: pktgen: add strict buffer parsing index check https://git.kernel.org/netdev/net-next/c/7151062c297c - [net-next,v1,2/2] selftest: net: update proc_net_pktgen (add more imix_weights test cases) https://git.kernel.org/netdev/net-next/c/3099f9e156b3
You are awesome, thank you!
linux-kselftest-mirror@lists.linaro.org
-
patchwork-bot+netdevbpf＠kernel.org -
Peter Seiderer -
Simon Horman