The `ss` program no longer stops when providing a long list of filters
Previously, providing a long list of filters to the "ss" command caused an integer value overflow. As a consequence, the 'ss' tool could stop the program execution. With this update, faulty bits in the source code are corrected, and the described problem no longer occurs.
DescriptionMartin Schwenke
2017-07-31 06:43:27 UTC
Description of problem:
ss crashes when the filter is too long/complex
Version-Release number of selected component (if applicable):
iproute-2.6.32-54.el6.x86_64
How reproducible:
Very reliable
Steps to Reproduce:
1. valgrind -q ss -tn state established 'src 10.0.0.31:22 || src 10.0.0.32:22 || src 10.0.0.33:22 || src 10.0.0.34:22 || src 10.0.0.35:22 || src 10.0.0.36:22 || src 10.0.0.37:22 || src 10.0.0.38:22'
Actual results:
Recv-Q Send-Q Local Address:Port Peer Address:Port
==4372== Warning: silly arg (-100) to malloc()
Aborted (core dumped)
Expected results:
A possibly empty list of connections. valgrind still complains but it is at least non-fatal.
Additional info:
This can obviously be recreated without valgrind. However, valgrind points out the nature of the problem.
The problem is that the following upstream commit is missing:
commit 2a4fa1c305742e4bfbc2960c40e0d1ee55b30694
Author: Andreas Henriksson <andreas>
Date: Wed Nov 13 09:46:42 2013 +0100
ss: avoid passing negative numbers to malloc
Example:
$ ss state established \( sport = :4060 or sport = :4061 or sport = :4062 or sport = :4063 or sport = :4064 or sport = :4065 or sport = :4066 or sport = :4067 \) > /dev/null
Aborted
In the example above ssfilter_bytecompile(...) will return (int)136.
char l1 = 136; means -120 which will result in a negative number
being passed to malloc at misc/ss.c:913.
Simply declare l1 and l2 as integers to avoid the char overflow.
This is one of the issues originally reported in http://bugs.debian.org/511720
Fix the same problem in other code paths as well (thanks to Eric Dumazet).
Reported-by: Andreas Schuldei <andreas>
Signed-off-by: Andreas Henriksson <andreas>
Reviewed-by: Eric Dumazet <edumazet>
Since the problem described in this bug report should be
resolved in a recent advisory, it has been closed with a
resolution of ERRATA.
For information on the advisory, and where to find the updated
files, follow the link below.
If the solution does not work for you, open a new bug report.
https://access.redhat.com/errata/RHBA-2018:1867
Description of problem: ss crashes when the filter is too long/complex Version-Release number of selected component (if applicable): iproute-2.6.32-54.el6.x86_64 How reproducible: Very reliable Steps to Reproduce: 1. valgrind -q ss -tn state established 'src 10.0.0.31:22 || src 10.0.0.32:22 || src 10.0.0.33:22 || src 10.0.0.34:22 || src 10.0.0.35:22 || src 10.0.0.36:22 || src 10.0.0.37:22 || src 10.0.0.38:22' Actual results: Recv-Q Send-Q Local Address:Port Peer Address:Port ==4372== Warning: silly arg (-100) to malloc() Aborted (core dumped) Expected results: A possibly empty list of connections. valgrind still complains but it is at least non-fatal. Additional info: This can obviously be recreated without valgrind. However, valgrind points out the nature of the problem. The problem is that the following upstream commit is missing: commit 2a4fa1c305742e4bfbc2960c40e0d1ee55b30694 Author: Andreas Henriksson <andreas> Date: Wed Nov 13 09:46:42 2013 +0100 ss: avoid passing negative numbers to malloc Example: $ ss state established \( sport = :4060 or sport = :4061 or sport = :4062 or sport = :4063 or sport = :4064 or sport = :4065 or sport = :4066 or sport = :4067 \) > /dev/null Aborted In the example above ssfilter_bytecompile(...) will return (int)136. char l1 = 136; means -120 which will result in a negative number being passed to malloc at misc/ss.c:913. Simply declare l1 and l2 as integers to avoid the char overflow. This is one of the issues originally reported in http://bugs.debian.org/511720 Fix the same problem in other code paths as well (thanks to Eric Dumazet). Reported-by: Andreas Schuldei <andreas> Signed-off-by: Andreas Henriksson <andreas> Reviewed-by: Eric Dumazet <edumazet>