netdev - Re: [PATCH net-next v2 01/12] selftests: net: py: coding style improvements

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]

Message-ID: <87o6oyaswg.fsf@nvidia.com>
Date: Wed, 19 Nov 2025 17:06:28 +0100
From: Petr Machata <petrm@...dia.com>
To: Jakub Kicinski <kuba@...nel.org>
CC: Petr Machata <petrm@...dia.com>, <davem@...emloft.net>,
	<netdev@...r.kernel.org>, <edumazet@...gle.com>, <pabeni@...hat.com>,
	<andrew+netdev@...n.ch>, <horms@...nel.org>,
	<willemdebruijn.kernel@...il.com>, <shuah@...nel.org>, <sdf@...ichev.me>,
	<krakauer@...gle.com>, <linux-kselftest@...r.kernel.org>,
	<matttbe@...nel.org>
Subject: Re: [PATCH net-next v2 01/12] selftests: net: py: coding style
 improvements


Jakub Kicinski <kuba@...nel.org> writes:

> On Wed, 19 Nov 2025 14:40:41 +0100 Petr Machata wrote:
>> > @@ -163,7 +162,7 @@ KSFT_DISRUPTIVE = True
>> >          entry = global_defer_queue.pop()
>> >          try:
>> >              entry.exec_only()
>> > -        except:
>> > +        except Exception:  
>> 
>> This used to catch KsftTerminate, which we use for SIGTERM handling, now
>> it doesn't anymore. I think it could legitimately appear in that context
>> if SIGTERM si delivered while exec_only() is running.
>> 
>> IMHO it should catch BaseException, like ksft_run() already does.
>
> TBH I haven't thought of this. Are you thinking that we shouldn't
> interrupt the execution of deferred cleanups when SIGTERM arrives?
> Fair point, but I think we'd need more code to handle that properly 🤔️
> Right now we ignore SIGTERM which isn't great. After this patch we'll
> no longer ignore it and have the whole test exit. Neither actually
> catches the exception and sets stop=True in ksft_run()..

Well, previously at least the rest of the defer queue would be run, now
it's skipped. Which -- OK, likely if you SIGTERM in the middle of a
cleanup, chances are the cleanup is stuck and the sigterm then skips it,
which is what you want.

It's a pick your poison. Ignore C-c or ignore deferred cleanups :)

Given this is supposed to be a cleanup patch, it would make sense to
just s/bare except/except BaseException/ and leave the behavior broken.
The commit message indicates it's a NOP make-linter-happy patch, but
then it's not really.

> WDYT about leaving this patch as is and doing this on top:

It's a good stop-gap.

> diff --git a/tools/testing/selftests/net/lib/py/ksft.py b/tools/testing/selftests/net/lib/py/ksft.py
> index 83b1574f7719..5a667ad22ef4 100644
> --- a/tools/testing/selftests/net/lib/py/ksft.py
> +++ b/tools/testing/selftests/net/lib/py/ksft.py
> @@ -268,7 +268,12 @@ KSFT_DISRUPTIVE = True
>              KSFT_RESULT = False
>              cnt_key = 'fail'
>  
> -        ksft_flush_defer()
> +        try:
> +            ksft_flush_defer()
> +        except BaseException as e:
> +            stop |= isinstance(e, KeyboardInterrupt)
> +            # Flush was interrupted, try to finish the job best we can
> +            ksft_flush_defer()
>  
>          if not cnt_key:
>              cnt_key = 'pass' if KSFT_RESULT else 'fail'