netdev - Re: [PATCH net] selftests: pmtu.sh: Kill tcpdump processes launched by subshell.

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 8 Mar 2022 11:21:58 -0700
From:   David Ahern <dsahern@...il.com>
To:     Guillaume Nault <gnault@...hat.com>,
        David Miller <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>
Cc:     netdev@...r.kernel.org, Shuah Khan <shuah@...nel.org>,
        linux-kselftest@...r.kernel.org
Subject: Re: [PATCH net] selftests: pmtu.sh: Kill tcpdump processes launched
 by subshell.

On 3/7/22 3:38 PM, Guillaume Nault wrote:
> The cleanup() function takes care of killing processes launched by the
> test functions. It relies on variables like ${tcpdump_pids} to get the
> relevant PIDs. But tests are run in their own subshell, so updated
> *_pids values are invisible to other shells. Therefore cleanup() never
> sees any process to kill:
> 
> $ ./tools/testing/selftests/net/pmtu.sh -t pmtu_ipv4_exception
> TEST: ipv4: PMTU exceptions                                         [ OK ]
> TEST: ipv4: PMTU exceptions - nexthop objects                       [ OK ]
> 
> $ pgrep -af tcpdump
> 6084 tcpdump -s 0 -i veth_A-R1 -w pmtu_ipv4_exception_veth_A-R1.pcap
> 6085 tcpdump -s 0 -i veth_R1-A -w pmtu_ipv4_exception_veth_R1-A.pcap
> 6086 tcpdump -s 0 -i veth_R1-B -w pmtu_ipv4_exception_veth_R1-B.pcap
> 6087 tcpdump -s 0 -i veth_B-R1 -w pmtu_ipv4_exception_veth_B-R1.pcap
> 6088 tcpdump -s 0 -i veth_A-R2 -w pmtu_ipv4_exception_veth_A-R2.pcap
> 6089 tcpdump -s 0 -i veth_R2-A -w pmtu_ipv4_exception_veth_R2-A.pcap
> 6090 tcpdump -s 0 -i veth_R2-B -w pmtu_ipv4_exception_veth_R2-B.pcap
> 6091 tcpdump -s 0 -i veth_B-R2 -w pmtu_ipv4_exception_veth_B-R2.pcap
> 6228 tcpdump -s 0 -i veth_A-R1 -w pmtu_ipv4_exception_veth_A-R1.pcap
> 6229 tcpdump -s 0 -i veth_R1-A -w pmtu_ipv4_exception_veth_R1-A.pcap
> 6230 tcpdump -s 0 -i veth_R1-B -w pmtu_ipv4_exception_veth_R1-B.pcap
> 6231 tcpdump -s 0 -i veth_B-R1 -w pmtu_ipv4_exception_veth_B-R1.pcap
> 6232 tcpdump -s 0 -i veth_A-R2 -w pmtu_ipv4_exception_veth_A-R2.pcap
> 6233 tcpdump -s 0 -i veth_R2-A -w pmtu_ipv4_exception_veth_R2-A.pcap
> 6234 tcpdump -s 0 -i veth_R2-B -w pmtu_ipv4_exception_veth_R2-B.pcap
> 6235 tcpdump -s 0 -i veth_B-R2 -w pmtu_ipv4_exception_veth_B-R2.pcap
> 
> Fix this by running cleanup() in the context of the test subshell.
> Now that each test cleans the environment after completion, there's no
> need for calling cleanup() again when the next test starts. So let's
> drop it from the setup() function. This is okay because cleanup() is
> also called when pmtu.sh starts, so even the first test starts in a
> clean environment.
> 
> Note: PAUSE_ON_FAIL is still evaluated before cleanup(), so one can
> still inspect the test environment upon failure when using -p.
> 
> Fixes: a92a0a7b8e7c ("selftests: pmtu: Simplify cleanup and namespace names")
> Signed-off-by: Guillaume Nault <gnault@...hat.com>
> ---
>  tools/testing/selftests/net/pmtu.sh | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 

Reviewed-by: David Ahern <dsahern@...nel.org>