[net-next,v2,0/8] Some pktgen fixes/improvments

Message

Peter Seiderer Jan. 22, 2025, 2:41 p.m. UTC

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 two minor findings) and add a simple
proc_net_pktgen selftest...

Regards,
Peter

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/[e-mail]/
[2] https://lore.kernel.org/netdev/[e-mail]/
[3] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=76201b5979768500bca362871db66d77cb4c225e

Peter Seiderer (8):
  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 access outside of user given buffer in
    pktgen_thread_write()
  net: pktgen: fix access outside of user given buffer in
    pktgen_if_write()
  selftest: net: add proc_net_pktgen

 net/core/pktgen.c                             | 238 ++++---
 tools/testing/selftests/net/Makefile          |   1 +
 tools/testing/selftests/net/config            |   1 +
 tools/testing/selftests/net/proc_net_pktgen.c | 605 ++++++++++++++++++
 4 files changed, 761 insertions(+), 84 deletions(-)
 create mode 100644 tools/testing/selftests/net/proc_net_pktgen.c

Comments

Simon Horman Jan. 22, 2025, 6:16 p.m. UTC | #1

On Wed, Jan 22, 2025 at 03:41:02PM +0100, Peter Seiderer wrote:
> 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 two minor findings) and add a simple
> proc_net_pktgen selftest...

Hi Peter,

Unfortunately net-next is closed at this time.

## Form letter - net-next-closed

The merge window for v6.14 has begun. Therefore net-next is closed
for new drivers, features, code refactoring and optimizations.
We are currently accepting bug fixes only.

Please repost when net-next reopens after Feb 3rd.

RFC patches sent for review only are obviously welcome at any time.

See: https://www.kernel.org/doc/html/next/process/maintainer-netdev.html#development-cycle

--
pw-bot: deferred

Simon Horman Jan. 22, 2025, 6:18 p.m. UTC | #2

On Wed, Jan 22, 2025 at 06:16:35PM +0000, Simon Horman wrote:
> On Wed, Jan 22, 2025 at 03:41:02PM +0100, Peter Seiderer wrote:
> > 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 two minor findings) and add a simple
> > proc_net_pktgen selftest...
> 
> Hi Peter,
> 
> Unfortunately net-next is closed at this time.

Sorry, I was a little hasty.

It looks like a number of these patches are fixes.

If so, please consider separating them out into a separate series,
targeted at net (rather than net-next), each with an appropriate Fixes tag.