[PATCH net-next v4 00/17] Some pktgen fixes/improvments

[PATCH net-next v4 00/17] Some pktgen fixes/improvments

[Peter Seiderer]

hile taking a look at '[PATCH net] pktgen: Avoid out-of-range in
get_imix_entries' ([1]) and '[PATCH net v2] pktgen: Avoid out-of-bounds access
in get_imix_entries' ([2], [3]) and doing some tests and code review I
detected that the /proc/net/pktgen/... parsing logic does not honour the
user given buffer bounds (resulting in out-of-bounds access).

This can be observed e.g. by the following simple test (sometimes the
old/'longer' previous value is re-read from the buffer):

        $ echo add_device lo@0 > /proc/net/pktgen/kpktgend_0

        $ echo "min_pkt_size 12345" > /proc/net/pktgen/lo\@0 && grep min_pkt_size /proc/net/pktgen/lo\@0
Params: count 1000  min_pkt_size: 12345  max_pkt_size: 0
Result: OK: min_pkt_size=12345

        $ echo -n "min_pkt_size 123" > /proc/net/pktgen/lo\@0 && grep min_pkt_size /proc/net/pktgen/lo\@0
Params: count 1000  min_pkt_size: 12345  max_pkt_size: 0
Result: OK: min_pkt_size=12345

        $ echo "min_pkt_size 123" > /proc/net/pktgen/lo\@0 && grep min_pkt_size /proc/net/pktgen/lo\@0
Params: count 1000  min_pkt_size: 123  max_pkt_size: 0
Result: OK: min_pkt_size=123

So fix the out-of-bounds access (and some minor findings) and add a simple
proc_net_pktgen selftest...

Regards,
Peter

Changes v3 -> v4:
 - add rev-by Simon Horman
 - new patch 'net: pktgen: use defines for the various dec/hex number parsing
   digits lengths' (suggested by Simon Horman)
 - replace C99 comment (suggested by Paolo Abeni)
 - drop available characters check in strn_len() (suggested by Paolo Abeni)
 - factored out patch 'net: pktgen: align some variable declarations to the
   most common pattern' (suggested by Paolo Abeni)
 - factored out patch 'net: pktgen: remove extra tmp variable (re-use len
   instead)' (suggested by Paolo Abeni)
 - factored out patch 'net: pktgen: remove some superfluous variable
   initializing' (suggested by Paolo Abeni)
 - factored out patch 'net: pktgen: fix mpls maximum labels list parsing'
   (suggested by Paolo Abeni)
 - factored out 'net: pktgen: hex32_arg/num_arg error out in case no
   characters are available' (suggested by Paolo Abeni)
 - factored out 'net: pktgen: num_arg error out in case no valid character
   is parsed' (suggested by Paolo Abeni)

Changes v2 -> v3:
 - new patch: 'net: pktgen: fix ctrl interface command parsing'
 - new patch: 'net: pktgen: fix mpls reset parsing'
 - tools/testing/selftests/net/proc_net_pktgen.c:
   - fix typo in change description ('v1 -> v1' and tyop)
   - rename some vars to better match usage
     add_loopback_0 -> thr_cmd_add_loopback_0
     rm_loopback_0 -> thr_cmd_rm_loopback_0
     wrong_ctrl_cmd -> wrong_thr_cmd
     legacy_ctrl_cmd -> legacy_thr_cmd
     ctrl_fd -> thr_fd
   - add ctrl interface tests

Changes v1 -> v2:
 - new patch: 'net: pktgen: fix hex32_arg parsing for short reads'
 - new patch: 'net: pktgen: fix 'rate 0' error handling (return -EINVAL)'
 - new patch: 'net: pktgen: fix 'ratep 0' error handling (return -EINVAL)'
 - net/core/pktgen.c: additional fix get_imix_entries() and get_labels()
 - tools/testing/selftests/net/proc_net_pktgen.c:
   - fix tyop not vs. nod (suggested by Jakub Kicinski)
   - fix misaligned line (suggested by Jakub Kicinski)
   - enable fomerly commented out CONFIG_XFRM dependent test (command spi),
     as CONFIG_XFRM is enabled via tools/testing/selftests/net/config
     CONFIG_XFRM_INTERFACE/CONFIG_XFRM_USER (suggestex by Jakub Kicinski)
   - add CONFIG_NET_PKTGEN=m to tools/testing/selftests/net/config
     (suggested by Jakub Kicinski)
   - add modprobe pktgen to FIXTURE_SETUP() (suggested by Jakub Kicinski)
   - fix some checkpatch warnings (Missing a blank line after declarations)
   - shrink line length by re-naming some variables (command -> cmd,
     device -> dev)
   - add 'rate 0' testcase
   - add 'ratep 0' testcase

[1] https://lore.kernel.org/netdev/20241006221221.3744995-1-artem.chernyshev@red-soft.ru/
[2] https://lore.kernel.org/netdev/20250109083039.14004-1-pchelkin@ispras.ru/
[3] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=76201b5979768500bca362871db66d77cb4c225e


Peter Seiderer (17):
  net: pktgen: replace ENOTSUPP with EOPNOTSUPP
  net: pktgen: enable 'param=value' parsing
  net: pktgen: fix hex32_arg parsing for short reads
  net: pktgen: fix 'rate 0' error handling (return -EINVAL)
  net: pktgen: fix 'ratep 0' error handling (return -EINVAL)
  net: pktgen: fix ctrl interface command parsing
  net: pktgen: fix access outside of user given buffer in
    pktgen_thread_write()
  net: pktgen: use defines for the various dec/hex number parsing digits
    lengths
  net: pktgen: align some variable declarations to the most common
    pattern
  net: pktgen: remove extra tmp variable (re-use len instead)
  net: pktgen: remove some superfluous variable initializing
  net: pktgen: fix mpls maximum labels list parsing
  net: pktgen: fix access outside of user given buffer in
    pktgen_if_write()
  net: pktgen: hex32_arg/num_arg error out in case no characters are
    available
  net: pktgen: num_arg error out in case no valid character is parsed
  net: pktgen: fix mpls reset parsing
  selftest: net: add proc_net_pktgen

 net/core/pktgen.c                             | 268 +++++---
 tools/testing/selftests/net/Makefile          |   1 +
 tools/testing/selftests/net/config            |   1 +
 tools/testing/selftests/net/proc_net_pktgen.c | 650 ++++++++++++++++++
 4 files changed, 828 insertions(+), 92 deletions(-)
 create mode 100644 tools/testing/selftests/net/proc_net_pktgen.c

-- 
2.48.1

[PATCH net-next v4 01/17] net: pktgen: replace ENOTSUPP with EOPNOTSUPP

[Peter Seiderer]

Replace ENOTSUPP with EOPNOTSUPP, fixes checkpatch hint

  WARNING: ENOTSUPP is not a SUSV4 error code, prefer EOPNOTSUPP

and e.g.

  $ echo "clone_skb 1" > /proc/net/pktgen/lo\@0
  -bash: echo: write error: Unknown error 524

Signed-off-by: Peter Seiderer <ps.report@gmx.net>
Reviewed-by: Simon Horman <horms@kernel.org>
---
Changes v3 -> v4
  - add rev-by Simon Horman

Changes v2 -> v3
  - no changes

Changes v1 -> v2
  - no changes
---
 net/core/pktgen.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/net/core/pktgen.c b/net/core/pktgen.c
index 82b6a2c3c141..496aa16773e7 100644
--- a/net/core/pktgen.c
+++ b/net/core/pktgen.c
@@ -1198,7 +1198,7 @@ static ssize_t pktgen_if_write(struct file *file,
 		if ((value > 0) &&
 		    ((pkt_dev->xmit_mode == M_NETIF_RECEIVE) ||
 		     !(pkt_dev->odev->priv_flags & IFF_TX_SKB_SHARING)))
-			return -ENOTSUPP;
+			return -EOPNOTSUPP;
 		if (value > 0 && (pkt_dev->n_imix_entries > 0 ||
 				  !(pkt_dev->flags & F_SHARED)))
 			return -EINVAL;
@@ -1258,7 +1258,7 @@ static ssize_t pktgen_if_write(struct file *file,
 		    ((pkt_dev->xmit_mode == M_QUEUE_XMIT) ||
 		     ((pkt_dev->xmit_mode == M_START_XMIT) &&
 		     (!(pkt_dev->odev->priv_flags & IFF_TX_SKB_SHARING)))))
-			return -ENOTSUPP;
+			return -EOPNOTSUPP;
 
 		if (value > 1 && !(pkt_dev->flags & F_SHARED))
 			return -EINVAL;
@@ -1303,7 +1303,7 @@ static ssize_t pktgen_if_write(struct file *file,
 		} else if (strcmp(f, "netif_receive") == 0) {
 			/* clone_skb set earlier, not supported in this mode */
 			if (pkt_dev->clone_skb > 0)
-				return -ENOTSUPP;
+				return -EOPNOTSUPP;
 
 			pkt_dev->xmit_mode = M_NETIF_RECEIVE;
 
-- 
2.48.1

[PATCH net-next v4 02/17] net: pktgen: enable 'param=value' parsing

[Peter Seiderer]

Enable more flexible parameters syntax, allowing 'param=value' in
addition to the already supported 'param value' pattern (additional
this gives the skipping '=' in count_trail_chars() a purpose).

Tested with:

	$ echo "min_pkt_size 999" > /proc/net/pktgen/lo\@0
	$ echo "min_pkt_size=999" > /proc/net/pktgen/lo\@0
	$ echo "min_pkt_size =999" > /proc/net/pktgen/lo\@0
	$ echo "min_pkt_size= 999" > /proc/net/pktgen/lo\@0
	$ echo "min_pkt_size = 999" > /proc/net/pktgen/lo\@0

Signed-off-by: Peter Seiderer <ps.report@gmx.net>
---
Changes v3 -> v4:
  - rephrase commit message (suggested by Paolo Abeni)

Changes v2 -> v3:
  - no changes

Changes v1 -> v2:
  - no changes
---
 net/core/pktgen.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/net/core/pktgen.c b/net/core/pktgen.c
index 496aa16773e7..4f8ec6c9bed4 100644
--- a/net/core/pktgen.c
+++ b/net/core/pktgen.c
@@ -823,6 +823,7 @@ static int strn_len(const char __user * user_buffer, unsigned int maxlen)
 		case '\r':
 		case '\t':
 		case ' ':
+		case '=':
 			goto done_str;
 		default:
 			break;
-- 
2.48.1

[PATCH net-next v4 03/17] net: pktgen: fix hex32_arg parsing for short reads

[Peter Seiderer]

Fix hex32_arg parsing for short reads (here 7 hex digits instead of the
expected 8), shift result only on successful input parsing.

- before the patch

	$ echo "mpls 0000123" > /proc/net/pktgen/lo\@0
	$ grep mpls /proc/net/pktgen/lo\@0
	     mpls: 00001230
	Result: OK: mpls=00001230

- with patch applied

	$ echo "mpls 0000123" > /proc/net/pktgen/lo\@0
	$ grep mpls /proc/net/pktgen/lo\@0
	     mpls: 00000123
	Result: OK: mpls=00000123

Signed-off-by: Peter Seiderer <ps.report@gmx.net>
Reviewed-by: Simon Horman <horms@kernel.org>
---
Changes v3 -> v4
  - add rev-by Simon Horman

Changes v2 -> v3:
  - no changes

Changes v1 -> v2:
  - new patch
---
 net/core/pktgen.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/net/core/pktgen.c b/net/core/pktgen.c
index 4f8ec6c9bed4..28dbbf70e142 100644
--- a/net/core/pktgen.c
+++ b/net/core/pktgen.c
@@ -753,14 +753,15 @@ static int hex32_arg(const char __user *user_buffer, unsigned long maxlen,
 	for (; i < maxlen; i++) {
 		int value;
 		char c;
-		*num <<= 4;
 		if (get_user(c, &user_buffer[i]))
 			return -EFAULT;
 		value = hex_to_bin(c);
-		if (value >= 0)
+		if (value >= 0) {
+			*num <<= 4;
 			*num |= value;
-		else
+		} else {
 			break;
+		}
 	}
 	return i;
 }
-- 
2.48.1

[PATCH net-next v4 04/17] net: pktgen: fix 'rate 0' error handling (return -EINVAL)

[Peter Seiderer]

Given an invalid 'rate' command e.g. 'rate 0' the return value is '1',
leading to the following misleading output:

- the good case

	$ echo "rate 100" > /proc/net/pktgen/lo\@0
	$ grep "Result:" /proc/net/pktgen/lo\@0
	Result: OK: rate=100

- the bad case (before the patch)

	$ echo "rate 0" > /proc/net/pktgen/lo\@0"
	-bash: echo: write error: Invalid argument
	$ grep "Result:" /proc/net/pktgen/lo\@0
	Result: No such parameter "ate"

- with patch applied

	$ echo "rate 0" > /proc/net/pktgen/lo\@0
	-bash: echo: write error: Invalid argument
	$ grep "Result:" /proc/net/pktgen/lo\@0
	Result: Idle

Signed-off-by: Peter Seiderer <ps.report@gmx.net>
Reviewed-by: Simon Horman <horms@kernel.org>
---
Changes v3 -> v4
      - add rev-by Simon Horman

Changes v2 -> v3:
  - no changes

Changes v1 -> v2:
  - new patch
---
 net/core/pktgen.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/core/pktgen.c b/net/core/pktgen.c
index 28dbbf70e142..75c7511bf492 100644
--- a/net/core/pktgen.c
+++ b/net/core/pktgen.c
@@ -1115,7 +1115,7 @@ static ssize_t pktgen_if_write(struct file *file,
 
 		i += len;
 		if (!value)
-			return len;
+			return -EINVAL;
 		pkt_dev->delay = pkt_dev->min_pkt_size*8*NSEC_PER_USEC/value;
 		if (debug)
 			pr_info("Delay set at: %llu ns\n", pkt_dev->delay);
-- 
2.48.1

[PATCH net-next v4 05/17] net: pktgen: fix 'ratep 0' error handling (return -EINVAL)

[Peter Seiderer]

Given an invalid 'ratep' command e.g. 'ratep 0' the return value is '1',
leading to the following misleading output:

- the good case

	$ echo "ratep 100" > /proc/net/pktgen/lo\@0
	$ grep "Result:" /proc/net/pktgen/lo\@0
	Result: OK: ratep=100

- the bad case (before the patch)

	$ echo "ratep 0" > /proc/net/pktgen/lo\@0"
	-bash: echo: write error: Invalid argument
	$ grep "Result:" /proc/net/pktgen/lo\@0
	Result: No such parameter "atep"

- with patch applied

	$ echo "ratep 0" > /proc/net/pktgen/lo\@0
	-bash: echo: write error: Invalid argument
	$ grep "Result:" /proc/net/pktgen/lo\@0
	Result: Idle

Signed-off-by: Peter Seiderer <ps.report@gmx.net>
Reviewed-by: Simon Horman <horms@kernel.org>
---
Changes v3 -> v4
  - add rev-by Simon Horman

Changes v2 -> v3:
  - no changes

Changes v1 -> v2:
  - new patch
---
 net/core/pktgen.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/core/pktgen.c b/net/core/pktgen.c
index 75c7511bf492..c8a5b4d17407 100644
--- a/net/core/pktgen.c
+++ b/net/core/pktgen.c
@@ -1130,7 +1130,7 @@ static ssize_t pktgen_if_write(struct file *file,
 
 		i += len;
 		if (!value)
-			return len;
+			return -EINVAL;
 		pkt_dev->delay = NSEC_PER_SEC/value;
 		if (debug)
 			pr_info("Delay set at: %llu ns\n", pkt_dev->delay);
-- 
2.48.1

[PATCH net-next v4 06/17] net: pktgen: fix ctrl interface command parsing

[Peter Seiderer]

Enable command writing without trailing '\n':

- the good case

	$ echo "reset" > /proc/net/pktgen/pgctrl

- the bad case (before the patch)

	$ echo -n "reset" > /proc/net/pktgen/pgctrl
	-bash: echo: write error: Invalid argument

- with patch applied

	$ echo -n "reset" > /proc/net/pktgen/pgctrl

Signed-off-by: Peter Seiderer <ps.report@gmx.net>
Reviewed-by: Simon Horman <horms@kernel.org>
---
Changes v3 -> v4
  - add rev-by Simon Horman

Changes v2 -> v3:
      - new patch
---
 net/core/pktgen.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/net/core/pktgen.c b/net/core/pktgen.c
index c8a5b4d17407..f6e35ba035c7 100644
--- a/net/core/pktgen.c
+++ b/net/core/pktgen.c
@@ -517,21 +517,23 @@ static ssize_t pgctrl_write(struct file *file, const char __user *buf,
 			    size_t count, loff_t *ppos)
 {
 	char data[128];
+	size_t max;
 	struct pktgen_net *pn = net_generic(current->nsproxy->net_ns, pg_net_id);
 
 	if (!capable(CAP_NET_ADMIN))
 		return -EPERM;
 
-	if (count == 0)
+	if (count < 1)
 		return -EINVAL;
 
-	if (count > sizeof(data))
-		count = sizeof(data);
-
-	if (copy_from_user(data, buf, count))
+	max = min(count, sizeof(data) - 1);
+	if (copy_from_user(data, buf, max))
 		return -EFAULT;
 
-	data[count - 1] = 0;	/* Strip trailing '\n' and terminate string */
+	if (data[max - 1] == '\n')
+		data[max - 1] = 0; /* strip trailing '\n', terminate string */
+	else
+		data[max] = 0; /* terminate string */
 
 	if (!strcmp(data, "stop"))
 		pktgen_stop_all_threads(pn);
-- 
2.48.1

[PATCH net-next v4 07/17] net: pktgen: fix access outside of user given buffer in pktgen_thread_write()

[Peter Seiderer]

Honour the user given buffer size for the strn_len() calls (otherwise
strn_len() will access memory outside of the user given buffer).

Signed-off-by: Peter Seiderer <ps.report@gmx.net>
Reviewed-by: Simon Horman <horms@kernel.org>
---
Changes v3 -> v4
  - add rev-by Simon Horman

Changes v2 -> v3:
  - no changes

Changes v1 -> v2:
  - no changes
---
 net/core/pktgen.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/net/core/pktgen.c b/net/core/pktgen.c
index f6e35ba035c7..55064713223e 100644
--- a/net/core/pktgen.c
+++ b/net/core/pktgen.c
@@ -1900,8 +1900,8 @@ static ssize_t pktgen_thread_write(struct file *file,
 	i = len;
 
 	/* Read variable name */
-
-	len = strn_len(&user_buffer[i], sizeof(name) - 1);
+	max = min(sizeof(name) - 1, count - i);
+	len = strn_len(&user_buffer[i], max);
 	if (len < 0)
 		return len;
 
@@ -1931,7 +1931,8 @@ static ssize_t pktgen_thread_write(struct file *file,
 	if (!strcmp(name, "add_device")) {
 		char f[32];
 		memset(f, 0, 32);
-		len = strn_len(&user_buffer[i], sizeof(f) - 1);
+		max = min(sizeof(f) - 1, count - i);
+		len = strn_len(&user_buffer[i], max);
 		if (len < 0) {
 			ret = len;
 			goto out;
-- 
2.48.1

[PATCH net-next v4 08/17] net: pktgen: use defines for the various dec/hex number parsing digits lengths

[Peter Seiderer]

Use defines for the various dec/hex number parsing digits lengths
(hex32_arg/num_arg calls).

Signed-off-by: Peter Seiderer <ps.report@gmx.net>
---
Changes v3 -> v4
  - new patch (suggested by Simon Horman)
---
 net/core/pktgen.c | 80 ++++++++++++++++++++++++++---------------------
 1 file changed, 44 insertions(+), 36 deletions(-)

diff --git a/net/core/pktgen.c b/net/core/pktgen.c
index 55064713223e..4f201a2db2dc 100644
--- a/net/core/pktgen.c
+++ b/net/core/pktgen.c
@@ -179,6 +179,15 @@
 #define MAX_IMIX_ENTRIES 20
 #define IMIX_PRECISION 100 /* Precision of IMIX distribution */
 
+#define DEC_1_DIGITS 1
+#define DEC_4_DIGITS 4
+#define DEC_5_DIGITS 5
+#define DEC_9_DIGITS 9
+#define DEC_10_DIGITS 10
+
+#define HEX_2_DIGITS 2
+#define HEX_8_DIGITS 8
+
 #define func_enter() pr_debug("entering %s\n", __func__);
 
 #define PKT_FLAGS							\
@@ -844,7 +853,6 @@ static int strn_len(const char __user * user_buffer, unsigned int maxlen)
 static ssize_t get_imix_entries(const char __user *buffer,
 				struct pktgen_dev *pkt_dev)
 {
-	const int max_digits = 10;
 	int i = 0;
 	long len;
 	char c;
@@ -858,7 +866,7 @@ static ssize_t get_imix_entries(const char __user *buffer,
 		if (pkt_dev->n_imix_entries >= MAX_IMIX_ENTRIES)
 			return -E2BIG;
 
-		len = num_arg(&buffer[i], max_digits, &size);
+		len = num_arg(&buffer[i], DEC_10_DIGITS, &size);
 		if (len < 0)
 			return len;
 		i += len;
@@ -872,7 +880,7 @@ static ssize_t get_imix_entries(const char __user *buffer,
 		if (size < 14 + 20 + 8)
 			size = 14 + 20 + 8;
 
-		len = num_arg(&buffer[i], max_digits, &weight);
+		len = num_arg(&buffer[i], DEC_10_DIGITS, &weight);
 		if (len < 0)
 			return len;
 		if (weight <= 0)
@@ -902,7 +910,7 @@ static ssize_t get_labels(const char __user *buffer, struct pktgen_dev *pkt_dev)
 	pkt_dev->nr_labels = 0;
 	do {
 		__u32 tmp;
-		len = hex32_arg(&buffer[i], 8, &tmp);
+		len = hex32_arg(&buffer[i], HEX_8_DIGITS, &tmp);
 		if (len <= 0)
 			return len;
 		pkt_dev->labels[n] = htonl(tmp);
@@ -1008,7 +1016,7 @@ static ssize_t pktgen_if_write(struct file *file,
 	}
 
 	if (!strcmp(name, "min_pkt_size")) {
-		len = num_arg(&user_buffer[i], 10, &value);
+		len = num_arg(&user_buffer[i], DEC_10_DIGITS, &value);
 		if (len < 0)
 			return len;
 
@@ -1025,7 +1033,7 @@ static ssize_t pktgen_if_write(struct file *file,
 	}
 
 	if (!strcmp(name, "max_pkt_size")) {
-		len = num_arg(&user_buffer[i], 10, &value);
+		len = num_arg(&user_buffer[i], DEC_10_DIGITS, &value);
 		if (len < 0)
 			return len;
 
@@ -1044,7 +1052,7 @@ static ssize_t pktgen_if_write(struct file *file,
 	/* Shortcut for min = max */
 
 	if (!strcmp(name, "pkt_size")) {
-		len = num_arg(&user_buffer[i], 10, &value);
+		len = num_arg(&user_buffer[i], DEC_10_DIGITS, &value);
 		if (len < 0)
 			return len;
 
@@ -1075,7 +1083,7 @@ static ssize_t pktgen_if_write(struct file *file,
 	}
 
 	if (!strcmp(name, "debug")) {
-		len = num_arg(&user_buffer[i], 10, &value);
+		len = num_arg(&user_buffer[i], DEC_10_DIGITS, &value);
 		if (len < 0)
 			return len;
 
@@ -1086,7 +1094,7 @@ static ssize_t pktgen_if_write(struct file *file,
 	}
 
 	if (!strcmp(name, "frags")) {
-		len = num_arg(&user_buffer[i], 10, &value);
+		len = num_arg(&user_buffer[i], DEC_10_DIGITS, &value);
 		if (len < 0)
 			return len;
 
@@ -1096,7 +1104,7 @@ static ssize_t pktgen_if_write(struct file *file,
 		return count;
 	}
 	if (!strcmp(name, "delay")) {
-		len = num_arg(&user_buffer[i], 10, &value);
+		len = num_arg(&user_buffer[i], DEC_10_DIGITS, &value);
 		if (len < 0)
 			return len;
 
@@ -1111,7 +1119,7 @@ static ssize_t pktgen_if_write(struct file *file,
 		return count;
 	}
 	if (!strcmp(name, "rate")) {
-		len = num_arg(&user_buffer[i], 10, &value);
+		len = num_arg(&user_buffer[i], DEC_10_DIGITS, &value);
 		if (len < 0)
 			return len;
 
@@ -1126,7 +1134,7 @@ static ssize_t pktgen_if_write(struct file *file,
 		return count;
 	}
 	if (!strcmp(name, "ratep")) {
-		len = num_arg(&user_buffer[i], 10, &value);
+		len = num_arg(&user_buffer[i], DEC_10_DIGITS, &value);
 		if (len < 0)
 			return len;
 
@@ -1141,7 +1149,7 @@ static ssize_t pktgen_if_write(struct file *file,
 		return count;
 	}
 	if (!strcmp(name, "udp_src_min")) {
-		len = num_arg(&user_buffer[i], 10, &value);
+		len = num_arg(&user_buffer[i], DEC_10_DIGITS, &value);
 		if (len < 0)
 			return len;
 
@@ -1154,7 +1162,7 @@ static ssize_t pktgen_if_write(struct file *file,
 		return count;
 	}
 	if (!strcmp(name, "udp_dst_min")) {
-		len = num_arg(&user_buffer[i], 10, &value);
+		len = num_arg(&user_buffer[i], DEC_10_DIGITS, &value);
 		if (len < 0)
 			return len;
 
@@ -1167,7 +1175,7 @@ static ssize_t pktgen_if_write(struct file *file,
 		return count;
 	}
 	if (!strcmp(name, "udp_src_max")) {
-		len = num_arg(&user_buffer[i], 10, &value);
+		len = num_arg(&user_buffer[i], DEC_10_DIGITS, &value);
 		if (len < 0)
 			return len;
 
@@ -1180,7 +1188,7 @@ static ssize_t pktgen_if_write(struct file *file,
 		return count;
 	}
 	if (!strcmp(name, "udp_dst_max")) {
-		len = num_arg(&user_buffer[i], 10, &value);
+		len = num_arg(&user_buffer[i], DEC_10_DIGITS, &value);
 		if (len < 0)
 			return len;
 
@@ -1193,7 +1201,7 @@ static ssize_t pktgen_if_write(struct file *file,
 		return count;
 	}
 	if (!strcmp(name, "clone_skb")) {
-		len = num_arg(&user_buffer[i], 10, &value);
+		len = num_arg(&user_buffer[i], DEC_10_DIGITS, &value);
 		if (len < 0)
 			return len;
 		/* clone_skb is not supported for netif_receive xmit_mode and
@@ -1214,7 +1222,7 @@ static ssize_t pktgen_if_write(struct file *file,
 		return count;
 	}
 	if (!strcmp(name, "count")) {
-		len = num_arg(&user_buffer[i], 10, &value);
+		len = num_arg(&user_buffer[i], DEC_10_DIGITS, &value);
 		if (len < 0)
 			return len;
 
@@ -1225,7 +1233,7 @@ static ssize_t pktgen_if_write(struct file *file,
 		return count;
 	}
 	if (!strcmp(name, "src_mac_count")) {
-		len = num_arg(&user_buffer[i], 10, &value);
+		len = num_arg(&user_buffer[i], DEC_10_DIGITS, &value);
 		if (len < 0)
 			return len;
 
@@ -1239,7 +1247,7 @@ static ssize_t pktgen_if_write(struct file *file,
 		return count;
 	}
 	if (!strcmp(name, "dst_mac_count")) {
-		len = num_arg(&user_buffer[i], 10, &value);
+		len = num_arg(&user_buffer[i], DEC_10_DIGITS, &value);
 		if (len < 0)
 			return len;
 
@@ -1253,7 +1261,7 @@ static ssize_t pktgen_if_write(struct file *file,
 		return count;
 	}
 	if (!strcmp(name, "burst")) {
-		len = num_arg(&user_buffer[i], 10, &value);
+		len = num_arg(&user_buffer[i], DEC_10_DIGITS, &value);
 		if (len < 0)
 			return len;
 
@@ -1272,7 +1280,7 @@ static ssize_t pktgen_if_write(struct file *file,
 		return count;
 	}
 	if (!strcmp(name, "node")) {
-		len = num_arg(&user_buffer[i], 10, &value);
+		len = num_arg(&user_buffer[i], DEC_10_DIGITS, &value);
 		if (len < 0)
 			return len;
 
@@ -1592,7 +1600,7 @@ static ssize_t pktgen_if_write(struct file *file,
 	}
 
 	if (!strcmp(name, "flows")) {
-		len = num_arg(&user_buffer[i], 10, &value);
+		len = num_arg(&user_buffer[i], DEC_10_DIGITS, &value);
 		if (len < 0)
 			return len;
 
@@ -1606,7 +1614,7 @@ static ssize_t pktgen_if_write(struct file *file,
 	}
 #ifdef CONFIG_XFRM
 	if (!strcmp(name, "spi")) {
-		len = num_arg(&user_buffer[i], 10, &value);
+		len = num_arg(&user_buffer[i], DEC_10_DIGITS, &value);
 		if (len < 0)
 			return len;
 
@@ -1617,7 +1625,7 @@ static ssize_t pktgen_if_write(struct file *file,
 	}
 #endif
 	if (!strcmp(name, "flowlen")) {
-		len = num_arg(&user_buffer[i], 10, &value);
+		len = num_arg(&user_buffer[i], DEC_10_DIGITS, &value);
 		if (len < 0)
 			return len;
 
@@ -1628,7 +1636,7 @@ static ssize_t pktgen_if_write(struct file *file,
 	}
 
 	if (!strcmp(name, "queue_map_min")) {
-		len = num_arg(&user_buffer[i], 5, &value);
+		len = num_arg(&user_buffer[i], DEC_5_DIGITS, &value);
 		if (len < 0)
 			return len;
 
@@ -1639,7 +1647,7 @@ static ssize_t pktgen_if_write(struct file *file,
 	}
 
 	if (!strcmp(name, "queue_map_max")) {
-		len = num_arg(&user_buffer[i], 5, &value);
+		len = num_arg(&user_buffer[i], DEC_5_DIGITS, &value);
 		if (len < 0)
 			return len;
 
@@ -1673,7 +1681,7 @@ static ssize_t pktgen_if_write(struct file *file,
 	}
 
 	if (!strcmp(name, "vlan_id")) {
-		len = num_arg(&user_buffer[i], 4, &value);
+		len = num_arg(&user_buffer[i], DEC_4_DIGITS, &value);
 		if (len < 0)
 			return len;
 
@@ -1700,7 +1708,7 @@ static ssize_t pktgen_if_write(struct file *file,
 	}
 
 	if (!strcmp(name, "vlan_p")) {
-		len = num_arg(&user_buffer[i], 1, &value);
+		len = num_arg(&user_buffer[i], DEC_1_DIGITS, &value);
 		if (len < 0)
 			return len;
 
@@ -1715,7 +1723,7 @@ static ssize_t pktgen_if_write(struct file *file,
 	}
 
 	if (!strcmp(name, "vlan_cfi")) {
-		len = num_arg(&user_buffer[i], 1, &value);
+		len = num_arg(&user_buffer[i], DEC_1_DIGITS, &value);
 		if (len < 0)
 			return len;
 
@@ -1730,7 +1738,7 @@ static ssize_t pktgen_if_write(struct file *file,
 	}
 
 	if (!strcmp(name, "svlan_id")) {
-		len = num_arg(&user_buffer[i], 4, &value);
+		len = num_arg(&user_buffer[i], DEC_4_DIGITS, &value);
 		if (len < 0)
 			return len;
 
@@ -1757,7 +1765,7 @@ static ssize_t pktgen_if_write(struct file *file,
 	}
 
 	if (!strcmp(name, "svlan_p")) {
-		len = num_arg(&user_buffer[i], 1, &value);
+		len = num_arg(&user_buffer[i], DEC_1_DIGITS, &value);
 		if (len < 0)
 			return len;
 
@@ -1772,7 +1780,7 @@ static ssize_t pktgen_if_write(struct file *file,
 	}
 
 	if (!strcmp(name, "svlan_cfi")) {
-		len = num_arg(&user_buffer[i], 1, &value);
+		len = num_arg(&user_buffer[i], DEC_1_DIGITS, &value);
 		if (len < 0)
 			return len;
 
@@ -1788,7 +1796,7 @@ static ssize_t pktgen_if_write(struct file *file,
 
 	if (!strcmp(name, "tos")) {
 		__u32 tmp_value = 0;
-		len = hex32_arg(&user_buffer[i], 2, &tmp_value);
+		len = hex32_arg(&user_buffer[i], HEX_2_DIGITS, &tmp_value);
 		if (len < 0)
 			return len;
 
@@ -1804,7 +1812,7 @@ static ssize_t pktgen_if_write(struct file *file,
 
 	if (!strcmp(name, "traffic_class")) {
 		__u32 tmp_value = 0;
-		len = hex32_arg(&user_buffer[i], 2, &tmp_value);
+		len = hex32_arg(&user_buffer[i], HEX_2_DIGITS, &tmp_value);
 		if (len < 0)
 			return len;
 
@@ -1819,7 +1827,7 @@ static ssize_t pktgen_if_write(struct file *file,
 	}
 
 	if (!strcmp(name, "skb_priority")) {
-		len = num_arg(&user_buffer[i], 9, &value);
+		len = num_arg(&user_buffer[i], DEC_9_DIGITS, &value);
 		if (len < 0)
 			return len;
 
-- 
2.48.1

[PATCH net-next v4 09/17] net: pktgen: align some variable declarations to the most common pattern

[Peter Seiderer]

Align some variable declarations (in get_imix_entries and get_labels) to
the most common pattern (int instead of ssize_t/long) and adjust function
return value accordingly.

Signed-off-by: Peter Seiderer <ps.report@gmx.net>
---
Changes v3 -> v4
  - new patch (factored out of patch 'net: pktgen: fix access outside of user
    given buffer in pktgen_if_write()')
---
 net/core/pktgen.c | 14 ++++++--------
 1 file changed, 6 insertions(+), 8 deletions(-)

diff --git a/net/core/pktgen.c b/net/core/pktgen.c
index 4f201a2db2dc..279910367ad4 100644
--- a/net/core/pktgen.c
+++ b/net/core/pktgen.c
@@ -850,12 +850,11 @@ static int strn_len(const char __user * user_buffer, unsigned int maxlen)
  * where each entry consists of size and weight delimited by commas.
  * "size1,weight_1 size2,weight_2 ... size_n,weight_n" for example.
  */
-static ssize_t get_imix_entries(const char __user *buffer,
-				struct pktgen_dev *pkt_dev)
+static int get_imix_entries(const char __user *buffer,
+			    struct pktgen_dev *pkt_dev)
 {
-	int i = 0;
-	long len;
 	char c;
+	int i = 0, len;
 
 	pkt_dev->n_imix_entries = 0;
 
@@ -900,12 +899,11 @@ static ssize_t get_imix_entries(const char __user *buffer,
 	return i;
 }
 
-static ssize_t get_labels(const char __user *buffer, struct pktgen_dev *pkt_dev)
+static int get_labels(const char __user *buffer, struct pktgen_dev *pkt_dev)
 {
-	unsigned int n = 0;
 	char c;
-	ssize_t i = 0;
-	int len;
+	int i = 0, len;
+	unsigned int n = 0;
 
 	pkt_dev->nr_labels = 0;
 	do {
-- 
2.48.1

[PATCH net-next v4 10/17] net: pktgen: remove extra tmp variable (re-use len instead)

[Peter Seiderer]

Remove extra tmp variable in pktgen_if_write (re-use len instead).

Signed-off-by: Peter Seiderer <ps.report@gmx.net>
---
Changes v3 -> v4
  - new patch (factored out of patch 'net: pktgen: fix access outside of user
    given buffer in pktgen_if_write()')
---
 net/core/pktgen.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/net/core/pktgen.c b/net/core/pktgen.c
index 279910367ad4..91b06473c925 100644
--- a/net/core/pktgen.c
+++ b/net/core/pktgen.c
@@ -966,7 +966,6 @@ static ssize_t pktgen_if_write(struct file *file,
 	char name[16], valstr[32];
 	unsigned long value = 0;
 	char *pg_result = NULL;
-	int tmp = 0;
 	char buf[128];
 
 	pg_result = &(pkt_dev->result[0]);
@@ -977,12 +976,12 @@ static ssize_t pktgen_if_write(struct file *file,
 	}
 
 	max = count;
-	tmp = count_trail_chars(user_buffer, max);
-	if (tmp < 0) {
+	len = count_trail_chars(user_buffer, max);
+	if (len < 0) {
 		pr_warn("illegal format\n");
-		return tmp;
+		return len;
 	}
-	i = tmp;
+	i = len;
 
 	/* Read variable name */
 
-- 
2.48.1

[PATCH net-next v4 11/17] net: pktgen: remove some superfluous variable initializing

[Peter Seiderer]

Remove some superfluous variable initializing before hex32_arg call (as the
same init is done here already).

Signed-off-by: Peter Seiderer <ps.report@gmx.net>
---
Changes v3 -> v4
  - new patch (factored out of patch 'net: pktgen: fix access outside of user
    given buffer in pktgen_if_write()')
---
 net/core/pktgen.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/core/pktgen.c b/net/core/pktgen.c
index 91b06473c925..84fd88e48275 100644
--- a/net/core/pktgen.c
+++ b/net/core/pktgen.c
@@ -1792,7 +1792,7 @@ static ssize_t pktgen_if_write(struct file *file,
 	}
 
 	if (!strcmp(name, "tos")) {
-		__u32 tmp_value = 0;
+		__u32 tmp_value;
 		len = hex32_arg(&user_buffer[i], HEX_2_DIGITS, &tmp_value);
 		if (len < 0)
 			return len;
@@ -1808,7 +1808,7 @@ static ssize_t pktgen_if_write(struct file *file,
 	}
 
 	if (!strcmp(name, "traffic_class")) {
-		__u32 tmp_value = 0;
+		__u32 tmp_value;
 		len = hex32_arg(&user_buffer[i], HEX_2_DIGITS, &tmp_value);
 		if (len < 0)
 			return len;
-- 
2.48.1

[PATCH net-next v4 12/17] net: pktgen: fix mpls maximum labels list parsing

[Peter Seiderer]

Fix mpls maximum labels list parsing up to MAX_MPLS_LABELS/16 entries
(instead of up to MAX_MPLS_LABELS - 1).

Fixes:

	$ echo "mpls 00000f00,00000f01,00000f02,00000f03,00000f04,00000f05,00000f06,00000f07,00000f08,00000f09,00000f0a,00000f0b,00000f0c,00000f0d,00000f0e,00000f0f" > /proc/net/pktgen/lo\@0
	-bash: echo: write error: Argument list too long

Signed-off-by: Peter Seiderer <ps.report@gmx.net>
---
Changes v3 -> v4
  - new patch (factored out of patch 'net: pktgen: fix access outside of user
    given buffer in pktgen_if_write()')
---
 net/core/pktgen.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/net/core/pktgen.c b/net/core/pktgen.c
index 84fd88e48275..0fd15f21119b 100644
--- a/net/core/pktgen.c
+++ b/net/core/pktgen.c
@@ -908,6 +908,10 @@ static int get_labels(const char __user *buffer, struct pktgen_dev *pkt_dev)
 	pkt_dev->nr_labels = 0;
 	do {
 		__u32 tmp;
+
+		if (n >= MAX_MPLS_LABELS)
+			return -E2BIG;
+
 		len = hex32_arg(&buffer[i], HEX_8_DIGITS, &tmp);
 		if (len <= 0)
 			return len;
@@ -919,8 +923,6 @@ static int get_labels(const char __user *buffer, struct pktgen_dev *pkt_dev)
 			return -EFAULT;
 		i++;
 		n++;
-		if (n >= MAX_MPLS_LABELS)
-			return -E2BIG;
 	} while (c == ',');
 
 	pkt_dev->nr_labels = n;
-- 
2.48.1

[PATCH net-next v4 13/17] net: pktgen: fix access outside of user given buffer in pktgen_if_write()

[Peter Seiderer]

Honour the user given buffer size for the hex32_arg(), num_arg(),
strn_len(), get_imix_entries() and get_labels() calls (otherwise they will
access memory outside of the user given buffer).

Signed-off-by: Peter Seiderer <ps.report@gmx.net>
---
Changes v3 -> v4:
  - replace C99 comment (suggested by Paolo Abeni)
  - drop available characters check in strn_len() (suggested by Paolo Abeni)
  - factored out patch 'net: pktgen: align some variable declarations to the
    most common pattern' (suggested by Paolo Abeni)
  - factored out patch 'net: pktgen: remove extra tmp variable (re-use len
    instead)' (suggested by Paolo Abeni)
  - factored out patch 'net: pktgen: remove some superfluous variable
    initializing' (suggested by Paolo Abeni)
  - factored out patch 'net: pktgen: fix mpls maximum labels list parsing'
    (suggested by Paolo Abeni)
  - factored out 'net: pktgen: hex32_arg/num_arg error out in case no
    characters are available' (suggested by Paolo Abeni)
  - factored out 'net: pktgen: num_arg error out in case no valid character
    is parsed' (suggested by Paolo Abeni)

Changes v2 -> v3:
  - no changes

Changes v1 -> v2:
  - additional fix get_imix_entries() and get_labels()
---
 net/core/pktgen.c | 176 ++++++++++++++++++++++++++++++----------------
 1 file changed, 117 insertions(+), 59 deletions(-)

diff --git a/net/core/pktgen.c b/net/core/pktgen.c
index 0fd15f21119b..3c42ecf17ba2 100644
--- a/net/core/pktgen.c
+++ b/net/core/pktgen.c
@@ -851,10 +851,11 @@ static int strn_len(const char __user * user_buffer, unsigned int maxlen)
  * "size1,weight_1 size2,weight_2 ... size_n,weight_n" for example.
  */
 static int get_imix_entries(const char __user *buffer,
+			    unsigned int maxlen,
 			    struct pktgen_dev *pkt_dev)
 {
 	char c;
-	int i = 0, len;
+	int i = 0, max, len;
 
 	pkt_dev->n_imix_entries = 0;
 
@@ -865,10 +866,13 @@ static int get_imix_entries(const char __user *buffer,
 		if (pkt_dev->n_imix_entries >= MAX_IMIX_ENTRIES)
 			return -E2BIG;
 
-		len = num_arg(&buffer[i], DEC_10_DIGITS, &size);
+		max = min(DEC_10_DIGITS, maxlen - i);
+		len = num_arg(&buffer[i], max, &size);
 		if (len < 0)
 			return len;
 		i += len;
+		if (i >= maxlen)
+			return -EINVAL;
 		if (get_user(c, &buffer[i]))
 			return -EFAULT;
 		/* Check for comma between size_i and weight_i */
@@ -879,7 +883,8 @@ static int get_imix_entries(const char __user *buffer,
 		if (size < 14 + 20 + 8)
 			size = 14 + 20 + 8;
 
-		len = num_arg(&buffer[i], DEC_10_DIGITS, &weight);
+		max = min(DEC_10_DIGITS, maxlen - i);
+		len = num_arg(&buffer[i], max, &weight);
 		if (len < 0)
 			return len;
 		if (weight <= 0)
@@ -889,21 +894,23 @@ static int get_imix_entries(const char __user *buffer,
 		pkt_dev->imix_entries[pkt_dev->n_imix_entries].weight = weight;
 
 		i += len;
+		pkt_dev->n_imix_entries++;
+
+		if (i >= maxlen)
+			break;
 		if (get_user(c, &buffer[i]))
 			return -EFAULT;
-
 		i++;
-		pkt_dev->n_imix_entries++;
 	} while (c == ' ');
 
 	return i;
 }
 
-static int get_labels(const char __user *buffer, struct pktgen_dev *pkt_dev)
+static int get_labels(const char __user *buffer, int maxlen, struct pktgen_dev *pkt_dev)
 {
 	char c;
-	int i = 0, len;
 	unsigned int n = 0;
+	int i = 0, max, len;
 
 	pkt_dev->nr_labels = 0;
 	do {
@@ -912,17 +919,20 @@ static int get_labels(const char __user *buffer, struct pktgen_dev *pkt_dev)
 		if (n >= MAX_MPLS_LABELS)
 			return -E2BIG;
 
-		len = hex32_arg(&buffer[i], HEX_8_DIGITS, &tmp);
+		max = min(HEX_8_DIGITS, maxlen - i);
+		len = hex32_arg(&buffer[i], max, &tmp);
 		if (len <= 0)
 			return len;
 		pkt_dev->labels[n] = htonl(tmp);
 		if (pkt_dev->labels[n] & MPLS_STACK_BOTTOM)
 			pkt_dev->flags |= F_MPLS_RND;
 		i += len;
+		n++;
+		if (i >= maxlen)
+			break;
 		if (get_user(c, &buffer[i]))
 			return -EFAULT;
 		i++;
-		n++;
 	} while (c == ',');
 
 	pkt_dev->nr_labels = n;
@@ -986,8 +996,8 @@ static ssize_t pktgen_if_write(struct file *file,
 	i = len;
 
 	/* Read variable name */
-
-	len = strn_len(&user_buffer[i], sizeof(name) - 1);
+	max = min(sizeof(name) - 1, count - i);
+	len = strn_len(&user_buffer[i], max);
 	if (len < 0)
 		return len;
 
@@ -1015,7 +1025,8 @@ static ssize_t pktgen_if_write(struct file *file,
 	}
 
 	if (!strcmp(name, "min_pkt_size")) {
-		len = num_arg(&user_buffer[i], DEC_10_DIGITS, &value);
+		max = min(DEC_10_DIGITS, count - i);
+		len = num_arg(&user_buffer[i], max, &value);
 		if (len < 0)
 			return len;
 
@@ -1032,7 +1043,8 @@ static ssize_t pktgen_if_write(struct file *file,
 	}
 
 	if (!strcmp(name, "max_pkt_size")) {
-		len = num_arg(&user_buffer[i], DEC_10_DIGITS, &value);
+		max = min(DEC_10_DIGITS, count - i);
+		len = num_arg(&user_buffer[i], max, &value);
 		if (len < 0)
 			return len;
 
@@ -1051,7 +1063,8 @@ static ssize_t pktgen_if_write(struct file *file,
 	/* Shortcut for min = max */
 
 	if (!strcmp(name, "pkt_size")) {
-		len = num_arg(&user_buffer[i], DEC_10_DIGITS, &value);
+		max = min(DEC_10_DIGITS, count - i);
+		len = num_arg(&user_buffer[i], max, &value);
 		if (len < 0)
 			return len;
 
@@ -1071,7 +1084,8 @@ static ssize_t pktgen_if_write(struct file *file,
 		if (pkt_dev->clone_skb > 0)
 			return -EINVAL;
 
-		len = get_imix_entries(&user_buffer[i], pkt_dev);
+		max = count - i;
+		len = get_imix_entries(&user_buffer[i], max, pkt_dev);
 		if (len < 0)
 			return len;
 
@@ -1082,7 +1096,8 @@ static ssize_t pktgen_if_write(struct file *file,
 	}
 
 	if (!strcmp(name, "debug")) {
-		len = num_arg(&user_buffer[i], DEC_10_DIGITS, &value);
+		max = min(DEC_10_DIGITS, count - i);
+		len = num_arg(&user_buffer[i], max, &value);
 		if (len < 0)
 			return len;
 
@@ -1093,7 +1108,8 @@ static ssize_t pktgen_if_write(struct file *file,
 	}
 
 	if (!strcmp(name, "frags")) {
-		len = num_arg(&user_buffer[i], DEC_10_DIGITS, &value);
+		max = min(DEC_10_DIGITS, count - i);
+		len = num_arg(&user_buffer[i], max, &value);
 		if (len < 0)
 			return len;
 
@@ -1103,7 +1119,8 @@ static ssize_t pktgen_if_write(struct file *file,
 		return count;
 	}
 	if (!strcmp(name, "delay")) {
-		len = num_arg(&user_buffer[i], DEC_10_DIGITS, &value);
+		max = min(DEC_10_DIGITS, count - i);
+		len = num_arg(&user_buffer[i], max, &value);
 		if (len < 0)
 			return len;
 
@@ -1118,7 +1135,8 @@ static ssize_t pktgen_if_write(struct file *file,
 		return count;
 	}
 	if (!strcmp(name, "rate")) {
-		len = num_arg(&user_buffer[i], DEC_10_DIGITS, &value);
+		max = min(DEC_10_DIGITS, count - i);
+		len = num_arg(&user_buffer[i], max, &value);
 		if (len < 0)
 			return len;
 
@@ -1133,7 +1151,8 @@ static ssize_t pktgen_if_write(struct file *file,
 		return count;
 	}
 	if (!strcmp(name, "ratep")) {
-		len = num_arg(&user_buffer[i], DEC_10_DIGITS, &value);
+		max = min(DEC_10_DIGITS, count - i);
+		len = num_arg(&user_buffer[i], max, &value);
 		if (len < 0)
 			return len;
 
@@ -1148,7 +1167,8 @@ static ssize_t pktgen_if_write(struct file *file,
 		return count;
 	}
 	if (!strcmp(name, "udp_src_min")) {
-		len = num_arg(&user_buffer[i], DEC_10_DIGITS, &value);
+		max = min(DEC_10_DIGITS, count - i);
+		len = num_arg(&user_buffer[i], max, &value);
 		if (len < 0)
 			return len;
 
@@ -1161,7 +1181,8 @@ static ssize_t pktgen_if_write(struct file *file,
 		return count;
 	}
 	if (!strcmp(name, "udp_dst_min")) {
-		len = num_arg(&user_buffer[i], DEC_10_DIGITS, &value);
+		max = min(DEC_10_DIGITS, count - i);
+		len = num_arg(&user_buffer[i], max, &value);
 		if (len < 0)
 			return len;
 
@@ -1174,7 +1195,8 @@ static ssize_t pktgen_if_write(struct file *file,
 		return count;
 	}
 	if (!strcmp(name, "udp_src_max")) {
-		len = num_arg(&user_buffer[i], DEC_10_DIGITS, &value);
+		max = min(DEC_10_DIGITS, count - i);
+		len = num_arg(&user_buffer[i], max, &value);
 		if (len < 0)
 			return len;
 
@@ -1187,7 +1209,8 @@ static ssize_t pktgen_if_write(struct file *file,
 		return count;
 	}
 	if (!strcmp(name, "udp_dst_max")) {
-		len = num_arg(&user_buffer[i], DEC_10_DIGITS, &value);
+		max = min(DEC_10_DIGITS, count - i);
+		len = num_arg(&user_buffer[i], max, &value);
 		if (len < 0)
 			return len;
 
@@ -1200,7 +1223,8 @@ static ssize_t pktgen_if_write(struct file *file,
 		return count;
 	}
 	if (!strcmp(name, "clone_skb")) {
-		len = num_arg(&user_buffer[i], DEC_10_DIGITS, &value);
+		max = min(DEC_10_DIGITS, count - i);
+		len = num_arg(&user_buffer[i], max, &value);
 		if (len < 0)
 			return len;
 		/* clone_skb is not supported for netif_receive xmit_mode and
@@ -1221,7 +1245,8 @@ static ssize_t pktgen_if_write(struct file *file,
 		return count;
 	}
 	if (!strcmp(name, "count")) {
-		len = num_arg(&user_buffer[i], DEC_10_DIGITS, &value);
+		max = min(DEC_10_DIGITS, count - i);
+		len = num_arg(&user_buffer[i], max, &value);
 		if (len < 0)
 			return len;
 
@@ -1232,7 +1257,8 @@ static ssize_t pktgen_if_write(struct file *file,
 		return count;
 	}
 	if (!strcmp(name, "src_mac_count")) {
-		len = num_arg(&user_buffer[i], DEC_10_DIGITS, &value);
+		max = min(DEC_10_DIGITS, count - i);
+		len = num_arg(&user_buffer[i], max, &value);
 		if (len < 0)
 			return len;
 
@@ -1246,7 +1272,8 @@ static ssize_t pktgen_if_write(struct file *file,
 		return count;
 	}
 	if (!strcmp(name, "dst_mac_count")) {
-		len = num_arg(&user_buffer[i], DEC_10_DIGITS, &value);
+		max = min(DEC_10_DIGITS, count - i);
+		len = num_arg(&user_buffer[i], max, &value);
 		if (len < 0)
 			return len;
 
@@ -1260,7 +1287,8 @@ static ssize_t pktgen_if_write(struct file *file,
 		return count;
 	}
 	if (!strcmp(name, "burst")) {
-		len = num_arg(&user_buffer[i], DEC_10_DIGITS, &value);
+		max = min(DEC_10_DIGITS, count - i);
+		len = num_arg(&user_buffer[i], max, &value);
 		if (len < 0)
 			return len;
 
@@ -1279,7 +1307,8 @@ static ssize_t pktgen_if_write(struct file *file,
 		return count;
 	}
 	if (!strcmp(name, "node")) {
-		len = num_arg(&user_buffer[i], DEC_10_DIGITS, &value);
+		max = min(DEC_10_DIGITS, count - i);
+		len = num_arg(&user_buffer[i], max, &value);
 		if (len < 0)
 			return len;
 
@@ -1300,11 +1329,12 @@ static ssize_t pktgen_if_write(struct file *file,
 	if (!strcmp(name, "xmit_mode")) {
 		char f[32];
 
-		memset(f, 0, 32);
-		len = strn_len(&user_buffer[i], sizeof(f) - 1);
+		max = min(sizeof(f) - 1, count - i);
+		len = strn_len(&user_buffer[i], max);
 		if (len < 0)
 			return len;
 
+		memset(f, 0, sizeof(f));
 		if (copy_from_user(f, &user_buffer[i], len))
 			return -EFAULT;
 		i += len;
@@ -1340,11 +1370,12 @@ static ssize_t pktgen_if_write(struct file *file,
 		char f[32];
 		char *end;
 
-		memset(f, 0, 32);
-		len = strn_len(&user_buffer[i], sizeof(f) - 1);
+		max = min(sizeof(f) - 1, count - i);
+		len = strn_len(&user_buffer[i], max);
 		if (len < 0)
 			return len;
 
+		memset(f, 0, 32);
 		if (copy_from_user(f, &user_buffer[i], len))
 			return -EFAULT;
 		i += len;
@@ -1389,7 +1420,8 @@ static ssize_t pktgen_if_write(struct file *file,
 		return count;
 	}
 	if (!strcmp(name, "dst_min") || !strcmp(name, "dst")) {
-		len = strn_len(&user_buffer[i], sizeof(pkt_dev->dst_min) - 1);
+		max = min(sizeof(pkt_dev->dst_min) - 1, count - i);
+		len = strn_len(&user_buffer[i], max);
 		if (len < 0)
 			return len;
 
@@ -1409,7 +1441,8 @@ static ssize_t pktgen_if_write(struct file *file,
 		return count;
 	}
 	if (!strcmp(name, "dst_max")) {
-		len = strn_len(&user_buffer[i], sizeof(pkt_dev->dst_max) - 1);
+		max = min(sizeof(pkt_dev->dst_max) - 1, count - i);
+		len = strn_len(&user_buffer[i], max);
 		if (len < 0)
 			return len;
 
@@ -1429,7 +1462,8 @@ static ssize_t pktgen_if_write(struct file *file,
 		return count;
 	}
 	if (!strcmp(name, "dst6")) {
-		len = strn_len(&user_buffer[i], sizeof(buf) - 1);
+		max = min(sizeof(buf) - 1, count - i);
+		len = strn_len(&user_buffer[i], max);
 		if (len < 0)
 			return len;
 
@@ -1452,7 +1486,8 @@ static ssize_t pktgen_if_write(struct file *file,
 		return count;
 	}
 	if (!strcmp(name, "dst6_min")) {
-		len = strn_len(&user_buffer[i], sizeof(buf) - 1);
+		max = min(sizeof(buf) - 1, count - i);
+		len = strn_len(&user_buffer[i], max);
 		if (len < 0)
 			return len;
 
@@ -1474,7 +1509,8 @@ static ssize_t pktgen_if_write(struct file *file,
 		return count;
 	}
 	if (!strcmp(name, "dst6_max")) {
-		len = strn_len(&user_buffer[i], sizeof(buf) - 1);
+		max = min(sizeof(buf) - 1, count - i);
+		len = strn_len(&user_buffer[i], max);
 		if (len < 0)
 			return len;
 
@@ -1495,7 +1531,8 @@ static ssize_t pktgen_if_write(struct file *file,
 		return count;
 	}
 	if (!strcmp(name, "src6")) {
-		len = strn_len(&user_buffer[i], sizeof(buf) - 1);
+		max = min(sizeof(buf) - 1, count - i);
+		len = strn_len(&user_buffer[i], max);
 		if (len < 0)
 			return len;
 
@@ -1518,7 +1555,8 @@ static ssize_t pktgen_if_write(struct file *file,
 		return count;
 	}
 	if (!strcmp(name, "src_min")) {
-		len = strn_len(&user_buffer[i], sizeof(pkt_dev->src_min) - 1);
+		max = min(sizeof(pkt_dev->src_min) - 1, count - i);
+		len = strn_len(&user_buffer[i], max);
 		if (len < 0)
 			return len;
 
@@ -1538,7 +1576,8 @@ static ssize_t pktgen_if_write(struct file *file,
 		return count;
 	}
 	if (!strcmp(name, "src_max")) {
-		len = strn_len(&user_buffer[i], sizeof(pkt_dev->src_max) - 1);
+		max = min(sizeof(pkt_dev->src_max) - 1, count - i);
+		len = strn_len(&user_buffer[i], max);
 		if (len < 0)
 			return len;
 
@@ -1558,7 +1597,8 @@ static ssize_t pktgen_if_write(struct file *file,
 		return count;
 	}
 	if (!strcmp(name, "dst_mac")) {
-		len = strn_len(&user_buffer[i], sizeof(valstr) - 1);
+		max = min(sizeof(valstr) - 1, count - i);
+		len = strn_len(&user_buffer[i], max);
 		if (len < 0)
 			return len;
 
@@ -1575,7 +1615,8 @@ static ssize_t pktgen_if_write(struct file *file,
 		return count;
 	}
 	if (!strcmp(name, "src_mac")) {
-		len = strn_len(&user_buffer[i], sizeof(valstr) - 1);
+		max = min(sizeof(valstr) - 1, count - i);
+		len = strn_len(&user_buffer[i], max);
 		if (len < 0)
 			return len;
 
@@ -1599,7 +1640,8 @@ static ssize_t pktgen_if_write(struct file *file,
 	}
 
 	if (!strcmp(name, "flows")) {
-		len = num_arg(&user_buffer[i], DEC_10_DIGITS, &value);
+		max = min(DEC_10_DIGITS, count - i);
+		len = num_arg(&user_buffer[i], max, &value);
 		if (len < 0)
 			return len;
 
@@ -1613,7 +1655,8 @@ static ssize_t pktgen_if_write(struct file *file,
 	}
 #ifdef CONFIG_XFRM
 	if (!strcmp(name, "spi")) {
-		len = num_arg(&user_buffer[i], DEC_10_DIGITS, &value);
+		max = min(DEC_10_DIGITS, count - i);
+		len = num_arg(&user_buffer[i], max, &value);
 		if (len < 0)
 			return len;
 
@@ -1624,7 +1667,8 @@ static ssize_t pktgen_if_write(struct file *file,
 	}
 #endif
 	if (!strcmp(name, "flowlen")) {
-		len = num_arg(&user_buffer[i], DEC_10_DIGITS, &value);
+		max = min(DEC_10_DIGITS, count - i);
+		len = num_arg(&user_buffer[i], max, &value);
 		if (len < 0)
 			return len;
 
@@ -1635,7 +1679,8 @@ static ssize_t pktgen_if_write(struct file *file,
 	}
 
 	if (!strcmp(name, "queue_map_min")) {
-		len = num_arg(&user_buffer[i], DEC_5_DIGITS, &value);
+		max = min(DEC_5_DIGITS, count - i);
+		len = num_arg(&user_buffer[i], max, &value);
 		if (len < 0)
 			return len;
 
@@ -1646,7 +1691,8 @@ static ssize_t pktgen_if_write(struct file *file,
 	}
 
 	if (!strcmp(name, "queue_map_max")) {
-		len = num_arg(&user_buffer[i], DEC_5_DIGITS, &value);
+		max = min(DEC_5_DIGITS, count - i);
+		len = num_arg(&user_buffer[i], max, &value);
 		if (len < 0)
 			return len;
 
@@ -1659,7 +1705,8 @@ static ssize_t pktgen_if_write(struct file *file,
 	if (!strcmp(name, "mpls")) {
 		unsigned int n, cnt;
 
-		len = get_labels(&user_buffer[i], pkt_dev);
+		max = count - i;
+		len = get_labels(&user_buffer[i], max, pkt_dev);
 		if (len < 0)
 			return len;
 		i += len;
@@ -1680,7 +1727,8 @@ static ssize_t pktgen_if_write(struct file *file,
 	}
 
 	if (!strcmp(name, "vlan_id")) {
-		len = num_arg(&user_buffer[i], DEC_4_DIGITS, &value);
+		max = min(DEC_4_DIGITS, count - i);
+		len = num_arg(&user_buffer[i], max, &value);
 		if (len < 0)
 			return len;
 
@@ -1707,7 +1755,8 @@ static ssize_t pktgen_if_write(struct file *file,
 	}
 
 	if (!strcmp(name, "vlan_p")) {
-		len = num_arg(&user_buffer[i], DEC_1_DIGITS, &value);
+		max = min(DEC_1_DIGITS, count - i);
+		len = num_arg(&user_buffer[i], max, &value);
 		if (len < 0)
 			return len;
 
@@ -1722,7 +1771,8 @@ static ssize_t pktgen_if_write(struct file *file,
 	}
 
 	if (!strcmp(name, "vlan_cfi")) {
-		len = num_arg(&user_buffer[i], DEC_1_DIGITS, &value);
+		max = min(DEC_1_DIGITS, count - i);
+		len = num_arg(&user_buffer[i], max, &value);
 		if (len < 0)
 			return len;
 
@@ -1737,7 +1787,8 @@ static ssize_t pktgen_if_write(struct file *file,
 	}
 
 	if (!strcmp(name, "svlan_id")) {
-		len = num_arg(&user_buffer[i], DEC_4_DIGITS, &value);
+		max = min(DEC_4_DIGITS, count - i);
+		len = num_arg(&user_buffer[i], max, &value);
 		if (len < 0)
 			return len;
 
@@ -1764,7 +1815,8 @@ static ssize_t pktgen_if_write(struct file *file,
 	}
 
 	if (!strcmp(name, "svlan_p")) {
-		len = num_arg(&user_buffer[i], DEC_1_DIGITS, &value);
+		max = min(DEC_1_DIGITS, count - i);
+		len = num_arg(&user_buffer[i], max, &value);
 		if (len < 0)
 			return len;
 
@@ -1779,7 +1831,8 @@ static ssize_t pktgen_if_write(struct file *file,
 	}
 
 	if (!strcmp(name, "svlan_cfi")) {
-		len = num_arg(&user_buffer[i], DEC_1_DIGITS, &value);
+		max = min(DEC_1_DIGITS, count - i);
+		len = num_arg(&user_buffer[i], max, &value);
 		if (len < 0)
 			return len;
 
@@ -1795,7 +1848,9 @@ static ssize_t pktgen_if_write(struct file *file,
 
 	if (!strcmp(name, "tos")) {
 		__u32 tmp_value;
-		len = hex32_arg(&user_buffer[i], HEX_2_DIGITS, &tmp_value);
+
+		max = min(HEX_2_DIGITS, count - i);
+		len = hex32_arg(&user_buffer[i], max, &tmp_value);
 		if (len < 0)
 			return len;
 
@@ -1811,7 +1866,9 @@ static ssize_t pktgen_if_write(struct file *file,
 
 	if (!strcmp(name, "traffic_class")) {
 		__u32 tmp_value;
-		len = hex32_arg(&user_buffer[i], HEX_2_DIGITS, &tmp_value);
+
+		max = min(HEX_2_DIGITS, count - i);
+		len = hex32_arg(&user_buffer[i], max, &tmp_value);
 		if (len < 0)
 			return len;
 
@@ -1826,7 +1883,8 @@ static ssize_t pktgen_if_write(struct file *file,
 	}
 
 	if (!strcmp(name, "skb_priority")) {
-		len = num_arg(&user_buffer[i], DEC_9_DIGITS, &value);
+		max = min(DEC_9_DIGITS, count - i);
+		len = num_arg(&user_buffer[i], max, &value);
 		if (len < 0)
 			return len;
 
-- 
2.48.1

[PATCH net-next v4 14/17] net: pktgen: hex32_arg/num_arg error out in case no characters are available

[Peter Seiderer]

In hex32_arg() and num_arg() error out in case no characters are available
(maxlen = 0).

Signed-off-by: Peter Seiderer <ps.report@gmx.net>
---
Changes v3 -> v4
  - new patch (factored out of patch 'net: pktgen: fix access outside of user
    given buffer in pktgen_if_write()')
---
 net/core/pktgen.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/net/core/pktgen.c b/net/core/pktgen.c
index 3c42ecf17ba2..cb3b732fd0a3 100644
--- a/net/core/pktgen.c
+++ b/net/core/pktgen.c
@@ -761,6 +761,9 @@ static int hex32_arg(const char __user *user_buffer, unsigned long maxlen,
 	int i = 0;
 	*num = 0;
 
+	if (!maxlen)
+		return -EINVAL;
+
 	for (; i < maxlen; i++) {
 		int value;
 		char c;
@@ -808,6 +811,9 @@ static long num_arg(const char __user *user_buffer, unsigned long maxlen,
 	int i;
 	*num = 0;
 
+	if (!maxlen)
+		return -EINVAL;
+
 	for (i = 0; i < maxlen; i++) {
 		char c;
 		if (get_user(c, &user_buffer[i]))
-- 
2.48.1

[PATCH net-next v4 15/17] net: pktgen: num_arg error out in case no valid character is parsed

[Peter Seiderer]

In num_arg() error out in case no valid character is parsed.

Signed-off-by: Peter Seiderer <ps.report@gmx.net>
---
Changes v3 -> v4
  - new patch (factored out of patch 'net: pktgen: fix access outside of user
    given buffer in pktgen_if_write()')
---
 net/core/pktgen.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/net/core/pktgen.c b/net/core/pktgen.c
index cb3b732fd0a3..a46eb20edf6c 100644
--- a/net/core/pktgen.c
+++ b/net/core/pktgen.c
@@ -821,6 +821,9 @@ static long num_arg(const char __user *user_buffer, unsigned long maxlen,
 		if ((c >= '0') && (c <= '9')) {
 			*num *= 10;
 			*num += c - '0';
+		} else if (i == 0) {
+			/* no valid character parsed, error out */
+			return -EINVAL;
 		} else
 			break;
 	}
-- 
2.48.1

[PATCH net-next v4 16/17] net: pktgen: fix mpls reset parsing

[Peter Seiderer]

Fix mpls list reset parsing to work as describe in
Documentation/networking/pktgen.rst:

  pgset "mpls 0"    turn off mpls (or any invalid argument works too!)

- before the patch

	$ echo "mpls 00000001,00000002" > /proc/net/pktgen/lo\@0
	$ grep mpls /proc/net/pktgen/lo\@0
	     mpls: 00000001, 00000002
	Result: OK: mpls=00000001,00000002

	$ echo "mpls 00000001,00000002" > /proc/net/pktgen/lo\@0
	$ echo "mpls 0" > /proc/net/pktgen/lo\@0
	$ grep mpls /proc/net/pktgen/lo\@0
	     mpls: 00000000
	Result: OK: mpls=00000000

	$ echo "mpls 00000001,00000002" > /proc/net/pktgen/lo\@0
	$ echo "mpls invalid" > /proc/net/pktgen/lo\@0
	$ grep mpls /proc/net/pktgen/lo\@0
	Result: OK: mpls=

- after the patch

	$ echo "mpls 00000001,00000002" > /proc/net/pktgen/lo\@0
	$ grep mpls /proc/net/pktgen/lo\@0
	     mpls: 00000001, 00000002
	Result: OK: mpls=00000001,00000002

	$ echo "mpls 00000001,00000002" > /proc/net/pktgen/lo\@0
	$ echo "mpls 0" > /proc/net/pktgen/lo\@0
	$ grep mpls /proc/net/pktgen/lo\@0
	Result: OK: mpls=

	$ echo "mpls 00000001,00000002" > /proc/net/pktgen/lo\@0
	$ echo "mpls invalid" > /proc/net/pktgen/lo\@0
	$ grep mpls /proc/net/pktgen/lo\@0
	Result: OK: mpls=

Signed-off-by: Peter Seiderer <ps.report@gmx.net>
Reviewed-by: Simon Horman <horms@kernel.org>
---
Changes v3 -> v4
          - add rev-by Simon Horman

Changes v2 -> v3:
  - new patch
---
 net/core/pktgen.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/net/core/pktgen.c b/net/core/pktgen.c
index a46eb20edf6c..434531a3e3c0 100644
--- a/net/core/pktgen.c
+++ b/net/core/pktgen.c
@@ -930,8 +930,13 @@ static int get_labels(const char __user *buffer, int maxlen, struct pktgen_dev *
 
 		max = min(HEX_8_DIGITS, maxlen - i);
 		len = hex32_arg(&buffer[i], max, &tmp);
-		if (len <= 0)
+		if (len < 0)
 			return len;
+
+		// return empty list in case of invalid input and/or zero value
+		if (len == 0 || tmp == 0)
+			return maxlen;
+
 		pkt_dev->labels[n] = htonl(tmp);
 		if (pkt_dev->labels[n] & MPLS_STACK_BOTTOM)
 			pkt_dev->flags |= F_MPLS_RND;
-- 
2.48.1

[PATCH net-next v4 17/17] selftest: net: add proc_net_pktgen

[Peter Seiderer]

Add some test for /proc/net/pktgen/... interface.

- enable 'CONFIG_NET_PKTGEN=m' in tools/testing/selftests/net/config

Signed-off-by: Peter Seiderer <ps.report@gmx.net>
---
Changes v3 -> v4:
  - no changes

Changes v2 -> v3:
  - fix typo in change description ('v1 -> v1' and tyop)
  - rename some vars to better match usage
    add_loopback_0 -> thr_cmd_add_loopback_0
    rm_loopback_0 -> thr_cmd_rm_loopback_0
    wrong_ctrl_cmd -> wrong_thr_cmd
    legacy_ctrl_cmd -> legacy_thr_cmd
    ctrl_fd -> thr_fd
  - add ctrl interface tests

Changes v1 -> v2:
  - fix tyop not vs. nod (suggested by Jakub Kicinski)
  - fix misaligned line (suggested by Jakub Kicinski)
  - enable fomerly commented out CONFIG_XFRM dependent test (command spi),
    as CONFIG_XFRM is enabled via tools/testing/selftests/net/config
    CONFIG_XFRM_INTERFACE/CONFIG_XFRM_USER (suggestex by Jakub Kicinski)
  - add CONFIG_NET_PKTGEN=m to tools/testing/selftests/net/config
    (suggested by Jakub Kicinski)
  - add modprobe pktgen to FIXTURE_SETUP() (suggested by Jakub Kicinski)
  - fix some checkpatch warnings (Missing a blank line after declarations)
  - shrink line length by re-naming some variables (command -> cmd,
    device -> dev)
  - add 'rate 0' testcase
  - add 'ratep 0' testcase
---
 tools/testing/selftests/net/Makefile          |   1 +
 tools/testing/selftests/net/config            |   1 +
 tools/testing/selftests/net/proc_net_pktgen.c | 650 ++++++++++++++++++
 3 files changed, 652 insertions(+)
 create mode 100644 tools/testing/selftests/net/proc_net_pktgen.c

diff --git a/tools/testing/selftests/net/Makefile b/tools/testing/selftests/net/Makefile
index 73ee88d6b043..095708cd8345 100644
--- a/tools/testing/selftests/net/Makefile
+++ b/tools/testing/selftests/net/Makefile
@@ -100,6 +100,7 @@ TEST_PROGS += vlan_bridge_binding.sh
 TEST_PROGS += bpf_offload.py
 TEST_PROGS += ipv6_route_update_soft_lockup.sh
 TEST_PROGS += busy_poll_test.sh
+TEST_GEN_PROGS += proc_net_pktgen
 
 # YNL files, must be before "include ..lib.mk"
 YNL_GEN_FILES := busy_poller netlink-dumps
diff --git a/tools/testing/selftests/net/config b/tools/testing/selftests/net/config
index 5b9baf708950..9fe1b3464fbc 100644
--- a/tools/testing/selftests/net/config
+++ b/tools/testing/selftests/net/config
@@ -107,3 +107,4 @@ CONFIG_XFRM_INTERFACE=m
 CONFIG_XFRM_USER=m
 CONFIG_IP_NF_MATCH_RPFILTER=m
 CONFIG_IP6_NF_MATCH_RPFILTER=m
+CONFIG_NET_PKTGEN=m
diff --git a/tools/testing/selftests/net/proc_net_pktgen.c b/tools/testing/selftests/net/proc_net_pktgen.c
new file mode 100644
index 000000000000..81bc9e9e412f
--- /dev/null
+++ b/tools/testing/selftests/net/proc_net_pktgen.c
@@ -0,0 +1,650 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * proc_net_pktgen: kselftest for /proc/net/pktgen interface
+ *
+ * Copyright (c) 2025 Peter Seiderer <ps.report@gmx.net>
+ *
+ */
+#include <errno.h>
+#include <fcntl.h>
+#include <stdlib.h>
+#include <unistd.h>
+
+#include "../kselftest_harness.h"
+
+static const char ctrl_cmd_stop[] = "stop";
+static const char ctrl_cmd_start[] = "start";
+static const char ctrl_cmd_reset[] = "reset";
+
+static const char wrong_ctrl_cmd[] = "0123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789";
+
+static const char thr_cmd_add_loopback_0[] = "add_device lo@0";
+static const char thr_cmd_rm_loopback_0[] = "rem_device_all";
+
+static const char wrong_thr_cmd[] = "forsureawrongcommand";
+static const char legacy_thr_cmd[] = "max_before_softirq";
+
+static const char wrong_dev_cmd[] = "forsurewrongcommand";
+static const char dev_cmd_min_pkt_size_0[] = "min_pkt_size";
+static const char dev_cmd_min_pkt_size_1[] = "min_pkt_size ";
+static const char dev_cmd_min_pkt_size_2[] = "min_pkt_size 0";
+static const char dev_cmd_min_pkt_size_3[] = "min_pkt_size 1";
+static const char dev_cmd_min_pkt_size_4[] = "min_pkt_size 100";
+static const char dev_cmd_min_pkt_size_5[] = "min_pkt_size=1001";
+static const char dev_cmd_min_pkt_size_6[] = "min_pkt_size =2002";
+static const char dev_cmd_min_pkt_size_7[] = "min_pkt_size= 3003";
+static const char dev_cmd_min_pkt_size_8[] = "min_pkt_size = 4004";
+static const char dev_cmd_max_pkt_size_0[] = "max_pkt_size 200";
+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_debug_0[] = "debug 1";
+static const char dev_cmd_debug_1[] = "debug 0";
+static const char dev_cmd_frags_0[] = "frags 100";
+static const char dev_cmd_delay_0[] = "delay 100";
+static const char dev_cmd_delay_1[] = "delay 2147483647";
+static const char dev_cmd_rate_0[] = "rate 0";
+static const char dev_cmd_rate_1[] = "rate 100";
+static const char dev_cmd_ratep_0[] = "ratep 0";
+static const char dev_cmd_ratep_1[] = "ratep 200";
+static const char dev_cmd_udp_src_min_0[] = "udp_src_min 1";
+static const char dev_cmd_udp_dst_min_0[] = "udp_dst_min 2";
+static const char dev_cmd_udp_src_max_0[] = "udp_src_max 3";
+static const char dev_cmd_udp_dst_max_0[] = "udp_dst_max 4";
+static const char dev_cmd_clone_skb_0[] = "clone_skb 1";
+static const char dev_cmd_clone_skb_1[] = "clone_skb 0";
+static const char dev_cmd_count_0[] = "count 100";
+static const char dev_cmd_src_mac_count_0[] = "src_mac_count 100";
+static const char dev_cmd_dst_mac_count_0[] = "dst_mac_count 100";
+static const char dev_cmd_burst_0[] = "burst 0";
+static const char dev_cmd_node_0[] = "node 100";
+static const char dev_cmd_xmit_mode_0[] = "xmit_mode start_xmit";
+static const char dev_cmd_xmit_mode_1[] = "xmit_mode netif_receive";
+static const char dev_cmd_xmit_mode_2[] = "xmit_mode queue_xmit";
+static const char dev_cmd_xmit_mode_3[] = "xmit_mode nonsense";
+static const char dev_cmd_flag_0[] = "flag UDPCSUM";
+static const char dev_cmd_flag_1[] = "flag !UDPCSUM";
+static const char dev_cmd_flag_2[] = "flag nonsense";
+static const char dev_cmd_dst_min_0[] = "dst_min 101.102.103.104";
+static const char dev_cmd_dst_0[] = "dst 101.102.103.104";
+static const char dev_cmd_dst_max_0[] = "dst_max 201.202.203.204";
+static const char dev_cmd_dst6_0[] = "dst6 2001:db38:1234:0000:0000:0000:0000:0000";
+static const char dev_cmd_dst6_min_0[] = "dst6_min 2001:db8:1234:0000:0000:0000:0000:0000";
+static const char dev_cmd_dst6_max_0[] = "dst6_max 2001:db8:1234:0000:0000:0000:0000:0000";
+static const char dev_cmd_src6_0[] = "src6 2001:db38:1234:0000:0000:0000:0000:0000";
+static const char dev_cmd_src_min_0[] = "src_min 101.102.103.104";
+static const char dev_cmd_src_max_0[] = "src_max 201.202.203.204";
+static const char dev_cmd_dst_mac_0[] = "dst_mac 01:02:03:04:05:06";
+static const char dev_cmd_src_mac_0[] = "src_mac 11:12:13:14:15:16";
+static const char dev_cmd_clear_counters_0[] = "clear_counters";
+static const char dev_cmd_flows_0[] = "flows 100";
+static const char dev_cmd_spi_0[] = "spi 100";
+static const char dev_cmd_flowlen_0[] = "flowlen 100";
+static const char dev_cmd_queue_map_min_0[] = "queue_map_min 1";
+static const char dev_cmd_queue_map_max_0[] = "queue_map_max 2";
+static const char dev_cmd_mpls_0[] = "mpls 00000001";
+static const char dev_cmd_mpls_1[] = "mpls 00000001,000000f2";
+static const char dev_cmd_mpls_2[] = "mpls 00000f00,00000f01,00000f02,00000f03,00000f04,00000f05,00000f06,00000f07,00000f08,00000f09,00000f0a,00000f0b,00000f0c,00000f0d,00000f0e,00000f0f";
+static const char dev_cmd_mpls_3[] = "mpls 00000f00,00000f01,00000f02,00000f03,00000f04,00000f05,00000f06,00000f07,00000f08,00000f09,00000f0a,00000f0b,00000f0c,00000f0d,00000f0e,00000f0f,00000f10";
+static const char dev_cmd_vlan_id_0[] = "vlan_id 1";
+static const char dev_cmd_vlan_p_0[] = "vlan_p 1";
+static const char dev_cmd_vlan_cfi_0[] = "vlan_cfi 1";
+static const char dev_cmd_vlan_id_1[] = "vlan_id 4096";
+static const char dev_cmd_svlan_id_0[] = "svlan_id 1";
+static const char dev_cmd_svlan_p_0[] = "svlan_p 1";
+static const char dev_cmd_svlan_cfi_0[] = "svlan_cfi 1";
+static const char dev_cmd_svlan_id_1[] = "svlan_id 4096";
+static const char dev_cmd_tos_0[] = "tos 0";
+static const char dev_cmd_tos_1[] = "tos 0f";
+static const char dev_cmd_tos_2[] = "tos 0ff";
+static const char dev_cmd_traffic_class_0[] = "traffic_class f0";
+static const char dev_cmd_skb_priority_0[] = "skb_priority 999";
+
+FIXTURE(proc_net_pktgen) {
+	int ctrl_fd;
+	int thr_fd;
+	int dev_fd;
+};
+
+FIXTURE_SETUP(proc_net_pktgen) {
+	int r;
+	ssize_t len;
+
+	r = system("modprobe pktgen");
+	ASSERT_EQ(r, 0) TH_LOG("CONFIG_NET_PKTGEN not enabled, module pktgen not loaded?");
+
+	self->ctrl_fd = open("/proc/net/pktgen/pgctrl", O_RDWR);
+	ASSERT_GE(self->ctrl_fd, 0) TH_LOG("CONFIG_NET_PKTGEN not enabled, module pktgen not loaded?");
+
+	self->thr_fd = open("/proc/net/pktgen/kpktgend_0", O_RDWR);
+	ASSERT_GE(self->thr_fd, 0) TH_LOG("CONFIG_NET_PKTGEN not enabled, module pktgen not loaded?");
+
+	len = write(self->thr_fd, thr_cmd_add_loopback_0, sizeof(thr_cmd_add_loopback_0));
+	ASSERT_EQ(len, sizeof(thr_cmd_add_loopback_0)) TH_LOG("device lo@0 already registered?");
+
+	self->dev_fd = open("/proc/net/pktgen/lo@0", O_RDWR);
+	ASSERT_GE(self->dev_fd, 0) TH_LOG("device entry for lo@0 missing?");
+}
+
+FIXTURE_TEARDOWN(proc_net_pktgen) {
+	int ret;
+	ssize_t len;
+
+	ret = close(self->dev_fd);
+	EXPECT_EQ(ret, 0);
+
+	len = write(self->thr_fd, thr_cmd_rm_loopback_0, sizeof(thr_cmd_rm_loopback_0));
+	EXPECT_EQ(len, sizeof(thr_cmd_rm_loopback_0));
+
+	ret = close(self->thr_fd);
+	EXPECT_EQ(ret, 0);
+
+	ret = close(self->ctrl_fd);
+	EXPECT_EQ(ret, 0);
+}
+
+TEST_F(proc_net_pktgen, wrong_ctrl_cmd) {
+	for (int i = 0; i <= sizeof(wrong_ctrl_cmd); i++) {
+		ssize_t len;
+
+		len = write(self->ctrl_fd, wrong_ctrl_cmd, i);
+		EXPECT_EQ(len, -1);
+		EXPECT_EQ(errno, EINVAL);
+	}
+}
+
+TEST_F(proc_net_pktgen, ctrl_cmd) {
+	ssize_t len;
+
+	len = write(self->ctrl_fd, ctrl_cmd_stop, sizeof(ctrl_cmd_stop));
+	EXPECT_EQ(len,	sizeof(ctrl_cmd_stop));
+
+	len = write(self->ctrl_fd, ctrl_cmd_stop, sizeof(ctrl_cmd_stop) - 1);
+	EXPECT_EQ(len,	sizeof(ctrl_cmd_stop) - 1);
+
+	len = write(self->ctrl_fd, ctrl_cmd_start, sizeof(ctrl_cmd_start));
+	EXPECT_EQ(len,	sizeof(ctrl_cmd_start));
+
+	len = write(self->ctrl_fd, ctrl_cmd_start, sizeof(ctrl_cmd_start) - 1);
+	EXPECT_EQ(len,	sizeof(ctrl_cmd_start) - 1);
+
+	len = write(self->ctrl_fd, ctrl_cmd_reset, sizeof(ctrl_cmd_reset));
+	EXPECT_EQ(len,	sizeof(ctrl_cmd_reset));
+
+	len = write(self->ctrl_fd, ctrl_cmd_reset, sizeof(ctrl_cmd_reset) - 1);
+	EXPECT_EQ(len,	sizeof(ctrl_cmd_reset) - 1);
+}
+
+TEST_F(proc_net_pktgen, wrong_thr_cmd) {
+	for (int i = 0; i <= sizeof(wrong_thr_cmd); i++) {
+		ssize_t len;
+
+		len = write(self->thr_fd, wrong_thr_cmd, i);
+		EXPECT_EQ(len, -1);
+		EXPECT_EQ(errno, EINVAL);
+	}
+}
+
+TEST_F(proc_net_pktgen, legacy_thr_cmd) {
+	for (int i = 0; i <= sizeof(legacy_thr_cmd); i++) {
+		ssize_t len;
+
+		len = write(self->thr_fd, legacy_thr_cmd, i);
+		if (i < (sizeof(legacy_thr_cmd) - 1)) {
+			// incomplete command string
+			EXPECT_EQ(len, -1);
+			EXPECT_EQ(errno, EINVAL);
+		} else {
+			// complete command string without/with trailing '\0'
+			EXPECT_EQ(len, i);
+		}
+	}
+}
+
+TEST_F(proc_net_pktgen, wrong_dev_cmd) {
+	for (int i = 0; i <= sizeof(wrong_dev_cmd); i++) {
+		ssize_t len;
+
+		len = write(self->dev_fd, wrong_dev_cmd, i);
+		EXPECT_EQ(len, -1);
+		EXPECT_EQ(errno, EINVAL);
+	}
+}
+
+TEST_F(proc_net_pktgen, dev_cmd_min_pkt_size) {
+	ssize_t len;
+
+	// with trailing '\0'
+	len = write(self->dev_fd, dev_cmd_min_pkt_size_0, sizeof(dev_cmd_min_pkt_size_0));
+	EXPECT_EQ(len, -1);
+	EXPECT_EQ(errno, EINVAL);
+
+	// without trailing '\0'
+	len = write(self->dev_fd, dev_cmd_min_pkt_size_0, sizeof(dev_cmd_min_pkt_size_0) - 1);
+	EXPECT_EQ(len, -1);
+	EXPECT_EQ(errno, EINVAL);
+
+	// with trailing '\0'
+	len = write(self->dev_fd, dev_cmd_min_pkt_size_1, sizeof(dev_cmd_min_pkt_size_1));
+	EXPECT_EQ(len, -1);
+	EXPECT_EQ(errno, EINVAL);
+
+	// without trailing '\0'
+	len = write(self->dev_fd, dev_cmd_min_pkt_size_1, sizeof(dev_cmd_min_pkt_size_1) - 1);
+	EXPECT_EQ(len, -1);
+	EXPECT_EQ(errno, EINVAL);
+
+	// with trailing '\0'
+	len = write(self->dev_fd, dev_cmd_min_pkt_size_2, sizeof(dev_cmd_min_pkt_size_2));
+	EXPECT_EQ(len, sizeof(dev_cmd_min_pkt_size_2));
+
+	// without trailing '\0'
+	len = write(self->dev_fd, dev_cmd_min_pkt_size_2, sizeof(dev_cmd_min_pkt_size_2) - 1);
+	EXPECT_EQ(len, sizeof(dev_cmd_min_pkt_size_2) - 1);
+
+	len = write(self->dev_fd, dev_cmd_min_pkt_size_3, sizeof(dev_cmd_min_pkt_size_3));
+	EXPECT_EQ(len, sizeof(dev_cmd_min_pkt_size_3));
+
+	len = write(self->dev_fd, dev_cmd_min_pkt_size_4, sizeof(dev_cmd_min_pkt_size_4));
+	EXPECT_EQ(len, sizeof(dev_cmd_min_pkt_size_4));
+
+	len = write(self->dev_fd, dev_cmd_min_pkt_size_5, sizeof(dev_cmd_min_pkt_size_5));
+	EXPECT_EQ(len, sizeof(dev_cmd_min_pkt_size_5));
+
+	len = write(self->dev_fd, dev_cmd_min_pkt_size_6, sizeof(dev_cmd_min_pkt_size_6));
+	EXPECT_EQ(len, sizeof(dev_cmd_min_pkt_size_6));
+
+	len = write(self->dev_fd, dev_cmd_min_pkt_size_7, sizeof(dev_cmd_min_pkt_size_7));
+	EXPECT_EQ(len, sizeof(dev_cmd_min_pkt_size_7));
+
+	len = write(self->dev_fd, dev_cmd_min_pkt_size_8, sizeof(dev_cmd_min_pkt_size_8));
+	EXPECT_EQ(len, sizeof(dev_cmd_min_pkt_size_8));
+}
+
+TEST_F(proc_net_pktgen, dev_cmd_max_pkt_size) {
+	ssize_t len;
+
+	len = write(self->dev_fd, dev_cmd_max_pkt_size_0, sizeof(dev_cmd_max_pkt_size_0));
+	EXPECT_EQ(len, sizeof(dev_cmd_max_pkt_size_0));
+}
+
+TEST_F(proc_net_pktgen, dev_cmd_pkt_size) {
+	ssize_t len;
+
+	len = write(self->dev_fd, dev_cmd_pkt_size_0, sizeof(dev_cmd_pkt_size_0));
+	EXPECT_EQ(len, sizeof(dev_cmd_pkt_size_0));
+}
+
+TEST_F(proc_net_pktgen, dev_cmd_imix_weights) {
+	ssize_t len;
+
+	len = write(self->dev_fd, dev_cmd_imix_weights_0, sizeof(dev_cmd_imix_weights_0));
+	EXPECT_EQ(len, sizeof(dev_cmd_imix_weights_0));
+
+	len = write(self->dev_fd, dev_cmd_imix_weights_1, sizeof(dev_cmd_imix_weights_1));
+	EXPECT_EQ(len, sizeof(dev_cmd_imix_weights_1));
+
+	len = write(self->dev_fd, dev_cmd_imix_weights_2, sizeof(dev_cmd_imix_weights_2));
+	EXPECT_EQ(len, -1);
+	EXPECT_EQ(errno, E2BIG);
+}
+
+TEST_F(proc_net_pktgen, dev_cmd_debug) {
+	ssize_t len;
+
+	// debug on
+	len = write(self->dev_fd, dev_cmd_debug_0, sizeof(dev_cmd_debug_0));
+	EXPECT_EQ(len, sizeof(dev_cmd_debug_0));
+
+	// debug off
+	len = write(self->dev_fd, dev_cmd_debug_1, sizeof(dev_cmd_debug_1));
+	EXPECT_EQ(len, sizeof(dev_cmd_debug_1));
+}
+
+TEST_F(proc_net_pktgen, dev_cmd_frags) {
+	ssize_t len;
+
+	len = write(self->dev_fd, dev_cmd_frags_0, sizeof(dev_cmd_frags_0));
+	EXPECT_EQ(len, sizeof(dev_cmd_frags_0));
+}
+
+TEST_F(proc_net_pktgen, dev_cmd_delay) {
+	ssize_t len;
+
+	len = write(self->dev_fd, dev_cmd_delay_0, sizeof(dev_cmd_delay_0));
+	EXPECT_EQ(len, sizeof(dev_cmd_delay_0));
+
+	len = write(self->dev_fd, dev_cmd_delay_1, sizeof(dev_cmd_delay_1));
+	EXPECT_EQ(len, sizeof(dev_cmd_delay_1));
+}
+
+TEST_F(proc_net_pktgen, dev_cmd_rate) {
+	ssize_t len;
+
+	len = write(self->dev_fd, dev_cmd_rate_0, sizeof(dev_cmd_rate_0));
+	EXPECT_EQ(len, -1);
+	EXPECT_EQ(errno, EINVAL);
+
+	len = write(self->dev_fd, dev_cmd_rate_1, sizeof(dev_cmd_rate_1));
+	EXPECT_EQ(len, sizeof(dev_cmd_rate_1));
+}
+
+TEST_F(proc_net_pktgen, dev_cmd_ratep) {
+	ssize_t len;
+
+	len = write(self->dev_fd, dev_cmd_ratep_0, sizeof(dev_cmd_ratep_0));
+	EXPECT_EQ(len, -1);
+	EXPECT_EQ(errno, EINVAL);
+
+	len = write(self->dev_fd, dev_cmd_ratep_1, sizeof(dev_cmd_ratep_1));
+	EXPECT_EQ(len, sizeof(dev_cmd_ratep_1));
+}
+
+TEST_F(proc_net_pktgen, dev_cmd_udp_src_min) {
+	ssize_t len;
+
+	len = write(self->dev_fd, dev_cmd_udp_src_min_0, sizeof(dev_cmd_udp_src_min_0));
+	EXPECT_EQ(len, sizeof(dev_cmd_udp_src_min_0));
+}
+
+TEST_F(proc_net_pktgen, dev_cmd_udp_dst_min) {
+	ssize_t len;
+
+	len = write(self->dev_fd, dev_cmd_udp_dst_min_0, sizeof(dev_cmd_udp_dst_min_0));
+	EXPECT_EQ(len, sizeof(dev_cmd_udp_dst_min_0));
+}
+
+TEST_F(proc_net_pktgen, dev_cmd_udp_src_max) {
+	ssize_t len;
+
+	len = write(self->dev_fd, dev_cmd_udp_src_max_0, sizeof(dev_cmd_udp_src_max_0));
+	EXPECT_EQ(len, sizeof(dev_cmd_udp_src_max_0));
+}
+
+TEST_F(proc_net_pktgen, dev_cmd_udp_dst_max) {
+	ssize_t len;
+
+	len = write(self->dev_fd, dev_cmd_udp_dst_max_0, sizeof(dev_cmd_udp_dst_max_0));
+	EXPECT_EQ(len, sizeof(dev_cmd_udp_dst_max_0));
+}
+
+TEST_F(proc_net_pktgen, dev_cmd_clone_skb) {
+	ssize_t len;
+
+	// clone_skb on (gives EOPNOTSUPP on lo device)
+	len = write(self->dev_fd, dev_cmd_clone_skb_0, sizeof(dev_cmd_clone_skb_0));
+	EXPECT_EQ(len, -1);
+	EXPECT_EQ(errno, EOPNOTSUPP);
+
+	// clone_skb off
+	len = write(self->dev_fd, dev_cmd_clone_skb_1, sizeof(dev_cmd_clone_skb_1));
+	EXPECT_EQ(len, sizeof(dev_cmd_clone_skb_1));
+}
+
+TEST_F(proc_net_pktgen, dev_cmd_count) {
+	ssize_t len;
+
+	len = write(self->dev_fd, dev_cmd_count_0, sizeof(dev_cmd_count_0));
+	EXPECT_EQ(len, sizeof(dev_cmd_count_0));
+}
+
+TEST_F(proc_net_pktgen, dev_cmd_src_mac_count) {
+	ssize_t len;
+
+	len = write(self->dev_fd, dev_cmd_src_mac_count_0, sizeof(dev_cmd_src_mac_count_0));
+	EXPECT_EQ(len, sizeof(dev_cmd_src_mac_count_0));
+}
+
+TEST_F(proc_net_pktgen, dev_cmd_dst_mac_count) {
+	ssize_t len;
+
+	len = write(self->dev_fd, dev_cmd_dst_mac_count_0, sizeof(dev_cmd_dst_mac_count_0));
+	EXPECT_EQ(len, sizeof(dev_cmd_dst_mac_count_0));
+}
+
+TEST_F(proc_net_pktgen, dev_cmd_burst) {
+	ssize_t len;
+
+	// burst off
+	len = write(self->dev_fd, dev_cmd_burst_0, sizeof(dev_cmd_burst_0));
+	EXPECT_EQ(len, sizeof(dev_cmd_burst_0));
+}
+
+TEST_F(proc_net_pktgen, dev_cmd_node) {
+	ssize_t len;
+
+	len = write(self->dev_fd, dev_cmd_node_0, sizeof(dev_cmd_node_0));
+	EXPECT_EQ(len, sizeof(dev_cmd_node_0));
+}
+
+TEST_F(proc_net_pktgen, dev_cmd_xmit_mode) {
+	ssize_t len;
+
+	len = write(self->dev_fd, dev_cmd_xmit_mode_0, sizeof(dev_cmd_xmit_mode_0));
+	EXPECT_EQ(len, sizeof(dev_cmd_xmit_mode_0));
+
+	len = write(self->dev_fd, dev_cmd_xmit_mode_1, sizeof(dev_cmd_xmit_mode_1));
+	EXPECT_EQ(len, sizeof(dev_cmd_xmit_mode_1));
+
+	len = write(self->dev_fd, dev_cmd_xmit_mode_2, sizeof(dev_cmd_xmit_mode_2));
+	EXPECT_EQ(len, sizeof(dev_cmd_xmit_mode_2));
+
+	len = write(self->dev_fd, dev_cmd_xmit_mode_3, sizeof(dev_cmd_xmit_mode_3));
+	EXPECT_EQ(len, sizeof(dev_cmd_xmit_mode_3));
+}
+
+TEST_F(proc_net_pktgen, dev_cmd_flag) {
+	ssize_t len;
+
+	// flag UDPCSUM on
+	len = write(self->dev_fd, dev_cmd_flag_0, sizeof(dev_cmd_flag_0));
+	EXPECT_EQ(len, sizeof(dev_cmd_flag_0));
+
+	// flag UDPCSUM off
+	len = write(self->dev_fd, dev_cmd_flag_1, sizeof(dev_cmd_flag_1));
+	EXPECT_EQ(len, sizeof(dev_cmd_flag_1));
+
+	// flag invalid
+	len = write(self->dev_fd, dev_cmd_flag_2, sizeof(dev_cmd_flag_2));
+	EXPECT_EQ(len, sizeof(dev_cmd_flag_2));
+}
+
+TEST_F(proc_net_pktgen, dev_cmd_dst_min) {
+	ssize_t len;
+
+	len = write(self->dev_fd, dev_cmd_dst_min_0, sizeof(dev_cmd_dst_min_0));
+	EXPECT_EQ(len, sizeof(dev_cmd_dst_min_0));
+}
+
+TEST_F(proc_net_pktgen, dev_cmd_dst) {
+	ssize_t len;
+
+	len = write(self->dev_fd, dev_cmd_dst_0, sizeof(dev_cmd_dst_0));
+	EXPECT_EQ(len, sizeof(dev_cmd_dst_0));
+}
+
+TEST_F(proc_net_pktgen, dev_cmd_dst_max) {
+	ssize_t len;
+
+	len = write(self->dev_fd, dev_cmd_dst_max_0, sizeof(dev_cmd_dst_max_0));
+	EXPECT_EQ(len, sizeof(dev_cmd_dst_max_0));
+}
+
+TEST_F(proc_net_pktgen, dev_cmd_dst6) {
+	ssize_t len;
+
+	len = write(self->dev_fd, dev_cmd_dst6_0, sizeof(dev_cmd_dst6_0));
+	EXPECT_EQ(len, sizeof(dev_cmd_dst6_0));
+}
+
+TEST_F(proc_net_pktgen, dev_cmd_dst6_min) {
+	ssize_t len;
+
+	len = write(self->dev_fd, dev_cmd_dst6_min_0, sizeof(dev_cmd_dst6_min_0));
+	EXPECT_EQ(len, sizeof(dev_cmd_dst6_min_0));
+}
+
+TEST_F(proc_net_pktgen, dev_cmd_dst6_max) {
+	ssize_t len;
+
+	len = write(self->dev_fd, dev_cmd_dst6_max_0, sizeof(dev_cmd_dst6_max_0));
+	EXPECT_EQ(len, sizeof(dev_cmd_dst6_max_0));
+}
+
+TEST_F(proc_net_pktgen, dev_cmd_src6) {
+	ssize_t len;
+
+	len = write(self->dev_fd, dev_cmd_src6_0, sizeof(dev_cmd_src6_0));
+	EXPECT_EQ(len, sizeof(dev_cmd_src6_0));
+}
+
+TEST_F(proc_net_pktgen, dev_cmd_src_min) {
+	ssize_t len;
+
+	len = write(self->dev_fd, dev_cmd_src_min_0, sizeof(dev_cmd_src_min_0));
+	EXPECT_EQ(len, sizeof(dev_cmd_src_min_0));
+}
+
+TEST_F(proc_net_pktgen, dev_cmd_src_max) {
+	ssize_t len;
+
+	len = write(self->dev_fd, dev_cmd_src_max_0, sizeof(dev_cmd_src_max_0));
+	EXPECT_EQ(len, sizeof(dev_cmd_src_max_0));
+}
+
+TEST_F(proc_net_pktgen, dev_cmd_dst_mac) {
+	ssize_t len;
+
+	len = write(self->dev_fd, dev_cmd_dst_mac_0, sizeof(dev_cmd_dst_mac_0));
+	EXPECT_EQ(len, sizeof(dev_cmd_dst_mac_0));
+}
+
+TEST_F(proc_net_pktgen, dev_cmd_src_mac) {
+	ssize_t len;
+
+	len = write(self->dev_fd, dev_cmd_src_mac_0, sizeof(dev_cmd_src_mac_0));
+	EXPECT_EQ(len, sizeof(dev_cmd_src_mac_0));
+}
+
+TEST_F(proc_net_pktgen, dev_cmd_clear_counters) {
+	ssize_t len;
+
+	len = write(self->dev_fd, dev_cmd_clear_counters_0, sizeof(dev_cmd_clear_counters_0));
+	EXPECT_EQ(len, sizeof(dev_cmd_clear_counters_0));
+}
+
+TEST_F(proc_net_pktgen, dev_cmd_flows) {
+	ssize_t len;
+
+	len = write(self->dev_fd, dev_cmd_flows_0, sizeof(dev_cmd_flows_0));
+	EXPECT_EQ(len, sizeof(dev_cmd_flows_0));
+}
+
+TEST_F(proc_net_pktgen, dev_cmd_spi) {
+	ssize_t len;
+
+	len = write(self->dev_fd, dev_cmd_spi_0, sizeof(dev_cmd_spi_0));
+	EXPECT_EQ(len, sizeof(dev_cmd_spi_0)) TH_LOG("CONFIG_XFRM not enabled?");
+}
+
+TEST_F(proc_net_pktgen, dev_cmd_flowlen) {
+	ssize_t len;
+
+	len = write(self->dev_fd, dev_cmd_flowlen_0, sizeof(dev_cmd_flowlen_0));
+	EXPECT_EQ(len, sizeof(dev_cmd_flowlen_0));
+}
+
+TEST_F(proc_net_pktgen, dev_cmd_queue_map_min) {
+	ssize_t len;
+
+	len = write(self->dev_fd, dev_cmd_queue_map_min_0, sizeof(dev_cmd_queue_map_min_0));
+	EXPECT_EQ(len, sizeof(dev_cmd_queue_map_min_0));
+}
+
+TEST_F(proc_net_pktgen, dev_cmd_queue_map_max) {
+	ssize_t len;
+
+	len = write(self->dev_fd, dev_cmd_queue_map_max_0, sizeof(dev_cmd_queue_map_max_0));
+	EXPECT_EQ(len, sizeof(dev_cmd_queue_map_max_0));
+}
+
+TEST_F(proc_net_pktgen, dev_cmd_mpls) {
+	ssize_t len;
+
+	len = write(self->dev_fd, dev_cmd_mpls_0, sizeof(dev_cmd_mpls_0));
+	EXPECT_EQ(len, sizeof(dev_cmd_mpls_0));
+
+	len = write(self->dev_fd, dev_cmd_mpls_1, sizeof(dev_cmd_mpls_1));
+	EXPECT_EQ(len, sizeof(dev_cmd_mpls_1));
+
+	len = write(self->dev_fd, dev_cmd_mpls_2, sizeof(dev_cmd_mpls_2));
+	EXPECT_EQ(len, sizeof(dev_cmd_mpls_2));
+
+	len = write(self->dev_fd, dev_cmd_mpls_3, sizeof(dev_cmd_mpls_3));
+	EXPECT_EQ(len, -1);
+	EXPECT_EQ(errno, E2BIG);
+}
+
+TEST_F(proc_net_pktgen, dev_cmd_vlan_id) {
+	ssize_t len;
+
+	len = write(self->dev_fd, dev_cmd_vlan_id_0, sizeof(dev_cmd_vlan_id_0));
+	EXPECT_EQ(len, sizeof(dev_cmd_vlan_id_0));
+
+	len = write(self->dev_fd, dev_cmd_vlan_p_0, sizeof(dev_cmd_vlan_p_0));
+	EXPECT_EQ(len, sizeof(dev_cmd_vlan_p_0));
+
+	len = write(self->dev_fd, dev_cmd_vlan_cfi_0, sizeof(dev_cmd_vlan_cfi_0));
+	EXPECT_EQ(len, sizeof(dev_cmd_vlan_cfi_0));
+
+	len = write(self->dev_fd, dev_cmd_vlan_id_1, sizeof(dev_cmd_vlan_id_1));
+	EXPECT_EQ(len, sizeof(dev_cmd_vlan_id_1));
+}
+
+TEST_F(proc_net_pktgen, dev_cmd_svlan_id) {
+	ssize_t len;
+
+	len = write(self->dev_fd, dev_cmd_svlan_id_0, sizeof(dev_cmd_svlan_id_0));
+	EXPECT_EQ(len, sizeof(dev_cmd_svlan_id_0));
+
+	len = write(self->dev_fd, dev_cmd_svlan_p_0, sizeof(dev_cmd_svlan_p_0));
+	EXPECT_EQ(len, sizeof(dev_cmd_svlan_p_0));
+
+	len = write(self->dev_fd, dev_cmd_svlan_cfi_0, sizeof(dev_cmd_svlan_cfi_0));
+	EXPECT_EQ(len, sizeof(dev_cmd_svlan_cfi_0));
+
+	len = write(self->dev_fd, dev_cmd_svlan_id_1, sizeof(dev_cmd_svlan_id_1));
+	EXPECT_EQ(len, sizeof(dev_cmd_svlan_id_1));
+}
+
+
+TEST_F(proc_net_pktgen, dev_cmd_tos) {
+	ssize_t len;
+
+	len = write(self->dev_fd, dev_cmd_tos_0, sizeof(dev_cmd_tos_0));
+	EXPECT_EQ(len, sizeof(dev_cmd_tos_0));
+
+	len = write(self->dev_fd, dev_cmd_tos_1, sizeof(dev_cmd_tos_1));
+	EXPECT_EQ(len, sizeof(dev_cmd_tos_1));
+
+	len = write(self->dev_fd, dev_cmd_tos_2, sizeof(dev_cmd_tos_2));
+	EXPECT_EQ(len, sizeof(dev_cmd_tos_2));
+}
+
+
+TEST_F(proc_net_pktgen, dev_cmd_traffic_class) {
+	ssize_t len;
+
+	len = write(self->dev_fd, dev_cmd_traffic_class_0, sizeof(dev_cmd_traffic_class_0));
+	EXPECT_EQ(len, sizeof(dev_cmd_traffic_class_0));
+}
+
+TEST_F(proc_net_pktgen, dev_cmd_skb_priority) {
+	ssize_t len;
+
+	len = write(self->dev_fd, dev_cmd_skb_priority_0, sizeof(dev_cmd_skb_priority_0));
+	EXPECT_EQ(len, sizeof(dev_cmd_skb_priority_0));
+}
+
+TEST_HARNESS_MAIN
-- 
2.48.1

Re: [PATCH net-next v4 12/17] net: pktgen: fix mpls maximum labels list parsing

[Simon Horman]

On Wed, Feb 05, 2025 at 02:11:48PM +0100, Peter Seiderer wrote:
> Fix mpls maximum labels list parsing up to MAX_MPLS_LABELS/16 entries
> (instead of up to MAX_MPLS_LABELS - 1).
> 
> Fixes:

"Fixes: " has a special meaning, it  is recognised as a tag by tooling, and
implies a bug fix. Please consider some other way of describing this, e.g.

Addresses the following:

> 
> 	$ echo "mpls 00000f00,00000f01,00000f02,00000f03,00000f04,00000f05,00000f06,00000f07,00000f08,00000f09,00000f0a,00000f0b,00000f0c,00000f0d,00000f0e,00000f0f" > /proc/net/pktgen/lo\@0
> 	-bash: echo: write error: Argument list too long
> 
> Signed-off-by: Peter Seiderer <ps.report@gmx.net>
> ---
> Changes v3 -> v4
>   - new patch (factored out of patch 'net: pktgen: fix access outside of user
>     given buffer in pktgen_if_write()')
> ---
>  net/core/pktgen.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/net/core/pktgen.c b/net/core/pktgen.c
> index 84fd88e48275..0fd15f21119b 100644
> --- a/net/core/pktgen.c
> +++ b/net/core/pktgen.c
> @@ -908,6 +908,10 @@ static int get_labels(const char __user *buffer, struct pktgen_dev *pkt_dev)
>  	pkt_dev->nr_labels = 0;
>  	do {
>  		__u32 tmp;
> +
> +		if (n >= MAX_MPLS_LABELS)
> +			return -E2BIG;
> +
>  		len = hex32_arg(&buffer[i], HEX_8_DIGITS, &tmp);
>  		if (len <= 0)
>  			return len;
> @@ -919,8 +923,6 @@ static int get_labels(const char __user *buffer, struct pktgen_dev *pkt_dev)
>  			return -EFAULT;
>  		i++;
>  		n++;
> -		if (n >= MAX_MPLS_LABELS)
> -			return -E2BIG;
>  	} while (c == ',');
>  
>  	pkt_dev->nr_labels = n;
> -- 
> 2.48.1
> 

Re: [PATCH net-next v4 11/17] net: pktgen: remove some superfluous variable initializing

[Simon Horman]

On Wed, Feb 05, 2025 at 02:11:47PM +0100, Peter Seiderer wrote:
> Remove some superfluous variable initializing before hex32_arg call (as the
> same init is done here already).
> 
> Signed-off-by: Peter Seiderer <ps.report@gmx.net>

Reviewed-by: Simon Horman <horms@kernel.org>

Re: [PATCH net-next v4 10/17] net: pktgen: remove extra tmp variable (re-use len instead)

[Simon Horman]

On Wed, Feb 05, 2025 at 02:11:46PM +0100, Peter Seiderer wrote:
> Remove extra tmp variable in pktgen_if_write (re-use len instead).
> 
> Signed-off-by: Peter Seiderer <ps.report@gmx.net>

Reviewed-by: Simon Horman <horms@kernel.org>

Re: [PATCH net-next v4 08/17] net: pktgen: use defines for the various dec/hex number parsing digits lengths

[Simon Horman]

On Wed, Feb 05, 2025 at 02:11:44PM +0100, Peter Seiderer wrote:
> Use defines for the various dec/hex number parsing digits lengths
> (hex32_arg/num_arg calls).
> 
> Signed-off-by: Peter Seiderer <ps.report@gmx.net>
> ---
> Changes v3 -> v4
>   - new patch (suggested by Simon Horman)

Reviewed-by: Simon Horman <horms@kernel.org>

Re: [PATCH net-next v4 09/17] net: pktgen: align some variable declarations to the most common pattern

[Simon Horman]

On Wed, Feb 05, 2025 at 02:11:45PM +0100, Peter Seiderer wrote:
> Align some variable declarations (in get_imix_entries and get_labels) to
> the most common pattern (int instead of ssize_t/long) and adjust function
> return value accordingly.
> 
> Signed-off-by: Peter Seiderer <ps.report@gmx.net>

Hi Peter,

These comments are is true in general of this patchset, but particularly so
in the case of this patch:

* I think a more succinct subject would be nice.
* I think the patch description should provide some reason
  _why_ the change is being made.

Also, specifically relating to this patch, I wonder if it's scope ought to
be extended. For example, the two callers of num_arg(), get_imix_entries() and
pktgen_if_write() assign the return value of num_arg() to len, which is now
an int in both functions. But num_args() returns a long.

> ---
> Changes v3 -> v4
>   - new patch (factored out of patch 'net: pktgen: fix access outside of user
>     given buffer in pktgen_if_write()')
> ---
>  net/core/pktgen.c | 14 ++++++--------
>  1 file changed, 6 insertions(+), 8 deletions(-)
> 
> diff --git a/net/core/pktgen.c b/net/core/pktgen.c
> index 4f201a2db2dc..279910367ad4 100644
> --- a/net/core/pktgen.c
> +++ b/net/core/pktgen.c
> @@ -850,12 +850,11 @@ static int strn_len(const char __user * user_buffer, unsigned int maxlen)
>   * where each entry consists of size and weight delimited by commas.
>   * "size1,weight_1 size2,weight_2 ... size_n,weight_n" for example.
>   */
> -static ssize_t get_imix_entries(const char __user *buffer,
> -				struct pktgen_dev *pkt_dev)
> +static int get_imix_entries(const char __user *buffer,
> +			    struct pktgen_dev *pkt_dev)
>  {
> -	int i = 0;
> -	long len;
>  	char c;
> +	int i = 0, len;

Given it can be achieved with exactly the same lines changed, just in a
different order, please arrange the local variable declarations in reverse
xmas tree order - longest line to shortest.

Likewise for the other hunk of this patch.  And I believe there are also
other cases in this patchset where this comment applied.

The following tool can be useful:
https://github.com/ecree-solarflare/xmastree

...

-- 
pw-bot: changes-requested

Re: [PATCH net-next v4 00/17] Some pktgen fixes/improvments

[Simon Horman]

On Wed, Feb 05, 2025 at 02:11:36PM +0100, Peter Seiderer wrote:
> hile taking a look at '[PATCH net] pktgen: Avoid out-of-range in
> get_imix_entries' ([1]) and '[PATCH net v2] pktgen: Avoid out-of-bounds access
> in get_imix_entries' ([2], [3]) and doing some tests and code review I
> detected that the /proc/net/pktgen/... parsing logic does not honour the
> user given buffer bounds (resulting in out-of-bounds access).
> 
> This can be observed e.g. by the following simple test (sometimes the
> old/'longer' previous value is re-read from the buffer):
> 
>         $ echo add_device lo@0 > /proc/net/pktgen/kpktgend_0
> 
>         $ echo "min_pkt_size 12345" > /proc/net/pktgen/lo\@0 && grep min_pkt_size /proc/net/pktgen/lo\@0
> Params: count 1000  min_pkt_size: 12345  max_pkt_size: 0
> Result: OK: min_pkt_size=12345
> 
>         $ echo -n "min_pkt_size 123" > /proc/net/pktgen/lo\@0 && grep min_pkt_size /proc/net/pktgen/lo\@0
> Params: count 1000  min_pkt_size: 12345  max_pkt_size: 0
> Result: OK: min_pkt_size=12345
> 
>         $ echo "min_pkt_size 123" > /proc/net/pktgen/lo\@0 && grep min_pkt_size /proc/net/pktgen/lo\@0
> Params: count 1000  min_pkt_size: 123  max_pkt_size: 0
> Result: OK: min_pkt_size=123
> 
> So fix the out-of-bounds access (and some minor findings) and add a simple
> proc_net_pktgen selftest...
> 
> Regards,
> Peter
> 
> Changes v3 -> v4:
>  - add rev-by Simon Horman
>  - new patch 'net: pktgen: use defines for the various dec/hex number parsing
>    digits lengths' (suggested by Simon Horman)
>  - replace C99 comment (suggested by Paolo Abeni)
>  - drop available characters check in strn_len() (suggested by Paolo Abeni)
>  - factored out patch 'net: pktgen: align some variable declarations to the
>    most common pattern' (suggested by Paolo Abeni)
>  - factored out patch 'net: pktgen: remove extra tmp variable (re-use len
>    instead)' (suggested by Paolo Abeni)
>  - factored out patch 'net: pktgen: remove some superfluous variable
>    initializing' (suggested by Paolo Abeni)
>  - factored out patch 'net: pktgen: fix mpls maximum labels list parsing'
>    (suggested by Paolo Abeni)
>  - factored out 'net: pktgen: hex32_arg/num_arg error out in case no
>    characters are available' (suggested by Paolo Abeni)
>  - factored out 'net: pktgen: num_arg error out in case no valid character
>    is parsed' (suggested by Paolo Abeni)

Hi Peter,

Thanks for splitting up the patchset some more, I for one find it much
easier to review them in this form.

That said, we are now over the preferred maximum of 15 patches in a series.
Perhaps the maintainers are ok with that, but I'd like to suggest breaking
the series in two: The first 7 patches seem to be somewhat stable, from a
review perspective, and could be posted as "part i"; And then the remaining
patches could be posted as "part ii" once "part i" has been accepted.

As for the selftests (the last patch of the series). A version,
trimmed down as appropriate, could be included in "part i", with a
follow-up in "part ii". Or the cover note for "part i" could state that the
selftests have been deferred to "part ii".

Perhaps the maintainers have other ideas, if so hopefully they will comment
here.

Re: [PATCH net-next v4 13/17] net: pktgen: fix access outside of user given buffer in pktgen_if_write()

[Simon Horman]

On Wed, Feb 05, 2025 at 02:11:49PM +0100, Peter Seiderer wrote:
> Honour the user given buffer size for the hex32_arg(), num_arg(),
> strn_len(), get_imix_entries() and get_labels() calls (otherwise they will
> access memory outside of the user given buffer).
> 
> Signed-off-by: Peter Seiderer <ps.report@gmx.net>
> ---
> Changes v3 -> v4:
>   - replace C99 comment (suggested by Paolo Abeni)
>   - drop available characters check in strn_len() (suggested by Paolo Abeni)
>   - factored out patch 'net: pktgen: align some variable declarations to the
>     most common pattern' (suggested by Paolo Abeni)
>   - factored out patch 'net: pktgen: remove extra tmp variable (re-use len
>     instead)' (suggested by Paolo Abeni)
>   - factored out patch 'net: pktgen: remove some superfluous variable
>     initializing' (suggested by Paolo Abeni)
>   - factored out patch 'net: pktgen: fix mpls maximum labels list parsing'
>     (suggested by Paolo Abeni)
>   - factored out 'net: pktgen: hex32_arg/num_arg error out in case no
>     characters are available' (suggested by Paolo Abeni)
>   - factored out 'net: pktgen: num_arg error out in case no valid character
>     is parsed' (suggested by Paolo Abeni)
> 
> Changes v2 -> v3:
>   - no changes
> 
> Changes v1 -> v2:
>   - additional fix get_imix_entries() and get_labels()

Reviewed-by: Simon Horman <horms@kernel.org>

> ---
>  net/core/pktgen.c | 176 ++++++++++++++++++++++++++++++----------------

...

> @@ -1015,7 +1025,8 @@ static ssize_t pktgen_if_write(struct file *file,
>  	}
>  
>  	if (!strcmp(name, "min_pkt_size")) {
> -		len = num_arg(&user_buffer[i], DEC_10_DIGITS, &value);
> +		max = min(DEC_10_DIGITS, count - i);
> +		len = num_arg(&user_buffer[i], max, &value);
>  		if (len < 0)
>  			return len;
>  

As an aside:

The code immediately following the hunk above is as follows.
And this block it is representative of many (all?) of the code
modified by the hunks that make up the remainder of this patch.

My observation is that i is incremented but never used again -
the function subsequently returns at the end of the if condition.

So perhaps it would be a nice, as a follow-up, to clean this up
be removing the increment of i from this and similar blocks.


		if (len < 0)
			return len;

		i += len;
		if (value < 14 + 20 + 8)
			value = 14 + 20 + 8;
		if (value != pkt_dev->min_pkt_size) {
			pkt_dev->min_pkt_size = value;
			pkt_dev->cur_pkt_size = value;
		}
		sprintf(pg_result, "OK: min_pkt_size=%d",
			pkt_dev->min_pkt_size);
		return count;
	}

...

Re: [PATCH net-next v4 14/17] net: pktgen: hex32_arg/num_arg error out in case no characters are available

[Simon Horman]

On Wed, Feb 05, 2025 at 02:11:50PM +0100, Peter Seiderer wrote:
> In hex32_arg() and num_arg() error out in case no characters are available
> (maxlen = 0).
> 
> Signed-off-by: Peter Seiderer <ps.report@gmx.net>

Hi Peter,

This patch and the following could benefit from:

* A more succinct subject
* An explanation of why the change is being made in the patch description

...

Re: [PATCH net-next v4 09/17] net: pktgen: align some variable declarations to the most common pattern

[Peter Seiderer]

Hello Simon,

On Thu, 6 Feb 2025 13:25:38 +0000, Simon Horman <horms@kernel.org> wrote:

> On Wed, Feb 05, 2025 at 02:11:45PM +0100, Peter Seiderer wrote:
> > Align some variable declarations (in get_imix_entries and get_labels) to
> > the most common pattern (int instead of ssize_t/long) and adjust function
> > return value accordingly.
> >
> > Signed-off-by: Peter Seiderer <ps.report@gmx.net>
>
> Hi Peter,
>
> These comments are is true in general of this patchset, but particularly so
> in the case of this patch:
>
> * I think a more succinct subject would be nice.
> * I think the patch description should provide some reason
>   _why_ the change is being made.

Yep, will improve...

>
> Also, specifically relating to this patch, I wonder if it's scope ought to
> be extended. For example, the two callers of num_arg(), get_imix_entries() and
> pktgen_if_write() assign the return value of num_arg() to len, which is now
> an int in both functions. But num_args() returns a long.

Aim was to get rid of the int/long mixture in the code (which works flawless
because no one writes to proc with more than a few bytes AND count is limited
to INT_MAX - PAGE_SIZE in vfs_write (see [1], [2])...

I believe the clean way is to use

  size_t i, max;
  ssize_t len;

consequently through out the code and adjust the function signatures
accordingly...., will re-spin...

>
> > ---
> > Changes v3 -> v4
> >   - new patch (factored out of patch 'net: pktgen: fix access outside of user
> >     given buffer in pktgen_if_write()')
> > ---
> >  net/core/pktgen.c | 14 ++++++--------
> >  1 file changed, 6 insertions(+), 8 deletions(-)
> >
> > diff --git a/net/core/pktgen.c b/net/core/pktgen.c
> > index 4f201a2db2dc..279910367ad4 100644
> > --- a/net/core/pktgen.c
> > +++ b/net/core/pktgen.c
> > @@ -850,12 +850,11 @@ static int strn_len(const char __user * user_buffer, unsigned int maxlen)
> >   * where each entry consists of size and weight delimited by commas.
> >   * "size1,weight_1 size2,weight_2 ... size_n,weight_n" for example.
> >   */
> > -static ssize_t get_imix_entries(const char __user *buffer,
> > -				struct pktgen_dev *pkt_dev)
> > +static int get_imix_entries(const char __user *buffer,
> > +			    struct pktgen_dev *pkt_dev)
> >  {
> > -	int i = 0;
> > -	long len;
> >  	char c;
> > +	int i = 0, len;
>
> Given it can be achieved with exactly the same lines changed, just in a
> different order, please arrange the local variable declarations in reverse
> xmas tree order - longest line to shortest.
>
> Likewise for the other hunk of this patch.  And I believe there are also
> other cases in this patchset where this comment applied.
>
> The following tool can be useful:
> https://github.com/ecree-solarflare/xmastree

O.k. will take a look at it...

Thanks for review!

Regards,
Peter


[1] https://elixir.bootlin.com/linux/v6.13.1/source/fs/read_write.c#L673
[2] https://elixir.bootlin.com/linux/v6.13.1/source/include/linux/fs.h#L2704

Re: [PATCH net-next v4 00/17] Some pktgen fixes/improvments

[Peter Seiderer]

Hello Simon,

On Thu, 6 Feb 2025 13:51:54 +0000, Simon Horman <horms@kernel.org> wrote:

> On Wed, Feb 05, 2025 at 02:11:36PM +0100, Peter Seiderer wrote:
> > hile taking a look at '[PATCH net] pktgen: Avoid out-of-range in
> > get_imix_entries' ([1]) and '[PATCH net v2] pktgen: Avoid out-of-bounds access
> > in get_imix_entries' ([2], [3]) and doing some tests and code review I
> > detected that the /proc/net/pktgen/... parsing logic does not honour the
> > user given buffer bounds (resulting in out-of-bounds access).
> >
> > This can be observed e.g. by the following simple test (sometimes the
> > old/'longer' previous value is re-read from the buffer):
> >
> >         $ echo add_device lo@0 > /proc/net/pktgen/kpktgend_0
> >
> >         $ echo "min_pkt_size 12345" > /proc/net/pktgen/lo\@0 && grep min_pkt_size /proc/net/pktgen/lo\@0
> > Params: count 1000  min_pkt_size: 12345  max_pkt_size: 0
> > Result: OK: min_pkt_size=12345
> >
> >         $ echo -n "min_pkt_size 123" > /proc/net/pktgen/lo\@0 && grep min_pkt_size /proc/net/pktgen/lo\@0
> > Params: count 1000  min_pkt_size: 12345  max_pkt_size: 0
> > Result: OK: min_pkt_size=12345
> >
> >         $ echo "min_pkt_size 123" > /proc/net/pktgen/lo\@0 && grep min_pkt_size /proc/net/pktgen/lo\@0
> > Params: count 1000  min_pkt_size: 123  max_pkt_size: 0
> > Result: OK: min_pkt_size=123
> >
> > So fix the out-of-bounds access (and some minor findings) and add a simple
> > proc_net_pktgen selftest...
> >
> > Regards,
> > Peter
> >
> > Changes v3 -> v4:
> >  - add rev-by Simon Horman
> >  - new patch 'net: pktgen: use defines for the various dec/hex number parsing
> >    digits lengths' (suggested by Simon Horman)
> >  - replace C99 comment (suggested by Paolo Abeni)
> >  - drop available characters check in strn_len() (suggested by Paolo Abeni)
> >  - factored out patch 'net: pktgen: align some variable declarations to the
> >    most common pattern' (suggested by Paolo Abeni)
> >  - factored out patch 'net: pktgen: remove extra tmp variable (re-use len
> >    instead)' (suggested by Paolo Abeni)
> >  - factored out patch 'net: pktgen: remove some superfluous variable
> >    initializing' (suggested by Paolo Abeni)
> >  - factored out patch 'net: pktgen: fix mpls maximum labels list parsing'
> >    (suggested by Paolo Abeni)
> >  - factored out 'net: pktgen: hex32_arg/num_arg error out in case no
> >    characters are available' (suggested by Paolo Abeni)
> >  - factored out 'net: pktgen: num_arg error out in case no valid character
> >    is parsed' (suggested by Paolo Abeni)
>
> Hi Peter,
>
> Thanks for splitting up the patchset some more, I for one find it much
> easier to review them in this form.

Definitely!

>
> That said, we are now over the preferred maximum of 15 patches in a series.
> Perhaps the maintainers are ok with that, but I'd like to suggest breaking
> the series in two: The first 7 patches seem to be somewhat stable, from a
> review perspective, and could be posted as "part i"; And then the remaining
> patches could be posted as "part ii" once "part i" has been accepted.

Yes, the patch set evolved a little bit over the 'just fix some little strange
behavior'..., splitting it up will work for me for sure..., will do on next
patch set iteration...

Regards,
Peter

>
> As for the selftests (the last patch of the series). A version,
> trimmed down as appropriate, could be included in "part i", with a
> follow-up in "part ii". Or the cover note for "part i" could state that the
> selftests have been deferred to "part ii".
>
> Perhaps the maintainers have other ideas, if so hopefully they will comment
> here.

Re: [PATCH net-next v4 09/17] net: pktgen: align some variable declarations to the most common pattern

[Simon Horman]

On Tue, Feb 11, 2025 at 10:29:59AM +0100, Peter Seiderer wrote:
> Hello Simon,
> 
> On Thu, 6 Feb 2025 13:25:38 +0000, Simon Horman <horms@kernel.org> wrote:
> 
> > On Wed, Feb 05, 2025 at 02:11:45PM +0100, Peter Seiderer wrote:
> > > Align some variable declarations (in get_imix_entries and get_labels) to
> > > the most common pattern (int instead of ssize_t/long) and adjust function
> > > return value accordingly.
> > >
> > > Signed-off-by: Peter Seiderer <ps.report@gmx.net>
> >
> > Hi Peter,
> >
> > These comments are is true in general of this patchset, but particularly so
> > in the case of this patch:
> >
> > * I think a more succinct subject would be nice.
> > * I think the patch description should provide some reason
> >   _why_ the change is being made.
> 
> Yep, will improve...
> 
> >
> > Also, specifically relating to this patch, I wonder if it's scope ought to
> > be extended. For example, the two callers of num_arg(), get_imix_entries() and
> > pktgen_if_write() assign the return value of num_arg() to len, which is now
> > an int in both functions. But num_args() returns a long.
> 
> Aim was to get rid of the int/long mixture in the code (which works flawless
> because no one writes to proc with more than a few bytes AND count is limited
> to INT_MAX - PAGE_SIZE in vfs_write (see [1], [2])...
> 
> I believe the clean way is to use
> 
>   size_t i, max;
>   ssize_t len;
> 
> consequently through out the code and adjust the function signatures
> accordingly...., will re-spin...

Thanks Peter,

I for one am all for things being consistent.

...