All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH net-next v5 0/8] Some pktgen fixes/improvments (part II)
@ 2025-02-13 11:19 Peter Seiderer
  2025-02-13 11:19 ` [PATCH net-next v5 1/8] net: pktgen: fix mix of int/long Peter Seiderer
                   ` (8 more replies)
  0 siblings, 9 replies; 13+ messages in thread
From: Peter Seiderer @ 2025-02-13 11:19 UTC (permalink / raw)
  To: netdev
  Cc: linux-kernel, linux-kselftest, David S . Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Simon Horman, Shuah Khan,
	Peter Seiderer, Thomas Gleixner, Artem Chernyshev, Nam Cao,
	Frederic Weisbecker

While 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...

Patch set splited into part I

- 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()

And part II (this one):

- net: pktgen: use defines for the various dec/hex number parsing digits lengths
- net: pktgen: fix mix of int/long
- 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: fix mpls reset parsing
- net: pktgen: remove all superfluous index assignements
- selftest: net: add proc_net_pktgen

Regards,
Peter

Changes v4 -> v5:
 - split up patchset into part i/ii (suggested by Simon Horman)
 - add rev-by Simon Horman
 - net: pktgen: align some variable  declarations to the most common pattern
   -> net: pktgen: fix mix of int/long
   - instead of align to most common pattern (int) adjust all usages to
      size_t for i and max and ssize_t for len and adjust function signatures
      of hex32_arg(), count_trail_chars(), num_arg() and strn_len() accordingly
   - respect reverse xmas tree order for local variable declarations (where
        possible without too much code churn)
   - update subject line and patch description
 - dropped net: pktgen: hex32_arg/num_arg error out in case no characters are
   available
   - keep empty hex/num arg is implicit assumed as zero value
 - dropped net: pktgen: num_arg error out in case no valid character is parsed
   - keep empty hex/num arg is implicit assumed as zero value
 - Change patch description ('Fixes:' -> 'Addresses the following:',
   suggested by Simon Horman)
 - net: pktgen: remove all superfluous index assignements
   - new patch (suggested by Simon Horman)
 - selftest: net: add proc_net_pktgen
   - addapt to dropped patch 'net: pktgen: hex32_arg/num_arg error out in case
     no characters are available', empty hex/num arg is now implicit assumed as
     zero value (instead of failure)

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 (8):
  net: pktgen: fix mix of int/long
  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: fix mpls reset parsing
  net: pktgen: remove all superfluous index assignements
  selftest: net: add proc_net_pktgen

 net/core/pktgen.c                             | 288 ++++----
 tools/testing/selftests/net/Makefile          |   1 +
 tools/testing/selftests/net/config            |   1 +
 tools/testing/selftests/net/proc_net_pktgen.c | 646 ++++++++++++++++++
 4 files changed, 806 insertions(+), 130 deletions(-)
 create mode 100644 tools/testing/selftests/net/proc_net_pktgen.c

-- 
2.48.1


^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2025-02-16 14:15 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-02-13 11:19 [PATCH net-next v5 0/8] Some pktgen fixes/improvments (part II) Peter Seiderer
2025-02-13 11:19 ` [PATCH net-next v5 1/8] net: pktgen: fix mix of int/long Peter Seiderer
2025-02-16 13:57   ` Simon Horman
2025-02-13 11:19 ` [PATCH net-next v5 2/8] net: pktgen: remove extra tmp variable (re-use len instead) Peter Seiderer
2025-02-13 11:19 ` [PATCH net-next v5 3/8] net: pktgen: remove some superfluous variable initializing Peter Seiderer
2025-02-13 11:19 ` [PATCH net-next v5 4/8] net: pktgen: fix mpls maximum labels list parsing Peter Seiderer
2025-02-16 13:58   ` Simon Horman
2025-02-13 11:19 ` [PATCH net-next v5 5/8] net: pktgen: fix access outside of user given buffer in pktgen_if_write() Peter Seiderer
2025-02-13 11:19 ` [PATCH net-next v5 6/8] net: pktgen: fix mpls reset parsing Peter Seiderer
2025-02-13 11:19 ` [PATCH net-next v5 7/8] net: pktgen: remove all superfluous index assignements Peter Seiderer
2025-02-16 14:15   ` Simon Horman
2025-02-13 11:19 ` [PATCH net-next v5 8/8] selftest: net: add proc_net_pktgen Peter Seiderer
2025-02-13 15:42 ` [PATCH net-next v5 0/8] Some pktgen fixes/improvments (part II) Jakub Kicinski

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.