On Wed, Jan 11, 2023 at 6:30 PM Masahiro Yamada <masahiroy@xxxxxxxxxx> wrote:
>
> In the follow-up of commit fb3041d61f68 ("kbuild: fix SIGPIPE error
> message for AR=gcc-ar and AR=llvm-ar"), Kees Cook pointed out that
> tools should _not_ catch their own SIGPIPEs [1] [2].
>
> Based on his feedback, LLVM was fixed [3].
>
> However, Python's default behavior is to show noisy bracktrace when
> SIGPIPE is sent. So, scripts written in Python are basically in the
> same situation as the buggy llvm tools.
>
> Example:
>
> $ make -s allnoconfig
> $ make -s allmodconfig
> $ scripts/diffconfig .config.old .config | head -n1
> -ALIX n
> Traceback (most recent call last):
> File "/home/masahiro/linux/scripts/diffconfig", line 132, in <module>
> main()
> File "/home/masahiro/linux/scripts/diffconfig", line 130, in main
> print_config("+", config, None, b[config])
> File "/home/masahiro/linux/scripts/diffconfig", line 64, in print_config
> print("+%s %s" % (config, new_value))
> BrokenPipeError: [Errno 32] Broken pipe
>
> Python documentatin [4] notes how to make scripts die immediately and
typo: s/documentatin/documentation/
> silently:
>
> """
> Piping output of your program to tools like head(1) will cause a
> SIGPIPE signal to be sent to your process when the receiver of its
> standard output closes early. This results in an exception like
> BrokenPipeError: [Errno 32] Broken pipe. To handle this case,
> wrap your entry point to catch this exception as follows:
>
> import os
> import sys
>
> def main():
> try:
> # simulate large output (your code replaces this loop)
> for x in range(10000):
> print("y")
> # flush output here to force SIGPIPE to be triggered
> # while inside this try block.
> sys.stdout.flush()
> except BrokenPipeError:
> # Python flushes standard streams on exit; redirect remaining output
> # to devnull to avoid another BrokenPipeError at shutdown
> devnull = os.open(os.devnull, os.O_WRONLY)
> os.dup2(devnull, sys.stdout.fileno())
> sys.exit(1) # Python exits with error code 1 on EPIPE
>
> if __name__ == '__main__':
> main()
>
> Do not set SIGPIPE’s disposition to SIG_DFL in order to avoid
> BrokenPipeError. Doing that would cause your program to exit
> unexpectedly whenever any socket connection is interrupted while
> your program is still writing to it.
> """
>
> Currently, tools/perf/scripts/python/intel-pt-events.py seems the
> only script that fixes the issue that way.
>
> tools/perf/scripts/python/compaction-times.py uses another approach
> signal.signal(signal.SIGPIPE, signal.SIG_DFL) but the Python
> documentation clearly says "Don't do it".
>
> I cannot fix all Python scripts since there are so many.
> I fixed some in the scripts/ directory.
That's ok; "Rome wasn't built in a day." This is a good start!
Thank you for the patch!
Reviewed-by: Nick Desaulniers <ndesaulniers@xxxxxxxxxx>
>
> [1]: https://lore.kernel.org/all/202211161056.1B9611A@keescook/
> [2]: https://github.com/llvm/llvm-project/issues/59037
> [3]: https://github.com/llvm/llvm-project/commit/4787efa38066adb51e2c049499d25b3610c0877b
> [4]: https://docs.python.org/3/library/signal.html#note-on-sigpipe
>
> Cc: Kees Cook <keescook@xxxxxxxxxxxx>
> Signed-off-by: Masahiro Yamada <masahiroy@xxxxxxxxxx>
> ---
>
> scripts/checkkconfigsymbols.py | 13 ++++++++++++-
> scripts/clang-tools/run-clang-tools.py | 21 ++++++++++++++-------
> scripts/diffconfig | 16 ++++++++++++++--
> 3 files changed, 40 insertions(+), 10 deletions(-)
>
> diff --git a/scripts/checkkconfigsymbols.py b/scripts/checkkconfigsymbols.py
> index 217d21abc86e..36c920e71313 100755
> --- a/scripts/checkkconfigsymbols.py
> +++ b/scripts/checkkconfigsymbols.py
> @@ -115,7 +115,7 @@ def parse_options():
> return args
>
>
> -def main():
> +def print_undefined_symbols():
> """Main function of this module."""
> args = parse_options()
>
> @@ -467,5 +467,16 @@ def parse_kconfig_file(kfile):
> return defined, references
>
>
> +def main():
> + try:
> + print_undefined_symbols()
> + except BrokenPipeError:
> + # Python flushes standard streams on exit; redirect remaining output
> + # to devnull to avoid another BrokenPipeError at shutdown
> + devnull = os.open(os.devnull, os.O_WRONLY)
> + os.dup2(devnull, sys.stdout.fileno())
> + sys.exit(1) # Python exits with error code 1 on EPIPE
> +
> +
> if __name__ == "__main__":
> main()
> diff --git a/scripts/clang-tools/run-clang-tools.py b/scripts/clang-tools/run-clang-tools.py
> index 56f2ec8f0f40..3266708a8658 100755
> --- a/scripts/clang-tools/run-clang-tools.py
> +++ b/scripts/clang-tools/run-clang-tools.py
> @@ -61,14 +61,21 @@ def run_analysis(entry):
>
>
> def main():
> - args = parse_arguments()
> + try:
> + args = parse_arguments()
>
> - lock = multiprocessing.Lock()
> - pool = multiprocessing.Pool(initializer=init, initargs=(lock, args))
> - # Read JSON data into the datastore variable
> - with open(args.path, "r") as f:
> - datastore = json.load(f)
> - pool.map(run_analysis, datastore)
> + lock = multiprocessing.Lock()
> + pool = multiprocessing.Pool(initializer=init, initargs=(lock, args))
> + # Read JSON data into the datastore variable
> + with open(args.path, "r") as f:
> + datastore = json.load(f)
> + pool.map(run_analysis, datastore)
> + except BrokenPipeError:
> + # Python flushes standard streams on exit; redirect remaining output
> + # to devnull to avoid another BrokenPipeError at shutdown
> + devnull = os.open(os.devnull, os.O_WRONLY)
> + os.dup2(devnull, sys.stdout.fileno())
> + sys.exit(1) # Python exits with error code 1 on EPIPE
>
>
> if __name__ == "__main__":
> diff --git a/scripts/diffconfig b/scripts/diffconfig
> index d5da5fa05d1d..43f0f3d273ae 100755
> --- a/scripts/diffconfig
> +++ b/scripts/diffconfig
> @@ -65,7 +65,7 @@ def print_config(op, config, value, new_value):
> else:
> print(" %s %s -> %s" % (config, value, new_value))
>
> -def main():
> +def show_diff():
> global merge_style
>
> # parse command line args
> @@ -129,4 +129,16 @@ def main():
> for config in new:
> print_config("+", config, None, b[config])
>
> -main()
> +def main():
> + try:
> + show_diff()
> + except BrokenPipeError:
> + # Python flushes standard streams on exit; redirect remaining output
> + # to devnull to avoid another BrokenPipeError at shutdown
> + devnull = os.open(os.devnull, os.O_WRONLY)
> + os.dup2(devnull, sys.stdout.fileno())
> + sys.exit(1) # Python exits with error code 1 on EPIPE
> +
> +
> +if __name__ == '__main__':
> + main()
> --
> 2.34.1
>
--
Thanks,
~Nick Desaulniers
