On Tue, Jan 26, 2021 at 03:56:11PM +0000, Stuart Henderson wrote: > On 2021/01/26 15:31, Clemens Gößnitzer wrote: > > January 26, 2021 3:44 PM, "Hiltjo Posthuma" <hil...@codemadness.org> wrote: > > > On Sat, Jan 16, 2021 at 04:29:27PM +0100, Hiltjo Posthuma wrote: > > >> On Mon, Jan 11, 2021 at 07:50:55PM +0100, Hiltjo Posthuma wrote: > > >> > > >> The below patch pledges the iconv binary in the libiconv package. The > > >> tool is > > >> useful for converting text-encoding of text data to UTF-8 for example. > > >> > > >> It now uses pledge("stdio", NULL) if only using stdin/stdout. It uses > > >> pledge("stdio rpath", NULL) when specifying files. > > >> > > >> I've tested many command-line option combinations and haven't found > > >> missing > > >> promises which cause an abort(). > > >> > > >> Patch: > .. > > >> +@@ -846,6 +849,9 @@ > > >> + struct iconv_hooks hooks; > > >> + int i; > > >> + int status; > > >> ++ > > >> ++ if (pledge(i == argc ? "stdio" : "stdio rpath", NULL) == -1) > > > > Wouldn't you use i uninitialised here? > > > > >> ++ err(1, "pledge"); > > >> + > > >> + set_program_name (argv[0]); > > >> + #if HAVE_SETLOCALE > > >> -- > > Yes, it needs to be done after parsing the arguments in the loop after > calling textdomain(). > > Looks like it was previously done like that but moved before sending out > the diff? I assume it was moved so that more of the code was moved under > pledge. Better approach might be to unconditionally pledge stdio rpath, > then, after the loop, conditionally pledge again to drop rpath if > possible. > > It would be nicer to use the error function used in the rest of > the file rather than pulling in another header for err(). >
Hi, Thanks both for the review! I updated the changes in the patch below. It was indeed a mistake in creating the patch, I'm sorry for the sloppiness. >From dbb04c280d8ca368da43c0fdf185c3e9a4a59050 Mon Sep 17 00:00:00 2001 From: Hiltjo Posthuma <hil...@codemadness.org> Date: Tue, 26 Jan 2021 20:21:32 +0100 Subject: [PATCH] libiconv: pledge iconv(1) binary --- converters/libiconv/Makefile | 3 +- converters/libiconv/patches/patch-src_iconv_c | 29 +++++++++++++++++++ 2 files changed, 31 insertions(+), 1 deletion(-) create mode 100644 converters/libiconv/patches/patch-src_iconv_c diff --git a/converters/libiconv/Makefile b/converters/libiconv/Makefile index 2ab58ea4519..5c8043270de 100644 --- a/converters/libiconv/Makefile +++ b/converters/libiconv/Makefile @@ -5,7 +5,7 @@ COMMENT= character set conversion library DISTNAME= libiconv-1.16 CATEGORIES= converters devel MASTER_SITES= ${MASTER_SITE_GNU:=libiconv/} -REVISION= 0 +REVISION= 1 SHARED_LIBS= charset 1.1 \ iconv 7.0 @@ -17,6 +17,7 @@ MAINTAINER= Brad Smith <b...@comstyle.com> # LGPLv2 and GPLv3 PERMIT_PACKAGE= Yes +# uses pledge() WANTLIB= c SEPARATE_BUILD= Yes diff --git a/converters/libiconv/patches/patch-src_iconv_c b/converters/libiconv/patches/patch-src_iconv_c new file mode 100644 index 00000000000..9b673fbe5db --- /dev/null +++ b/converters/libiconv/patches/patch-src_iconv_c @@ -0,0 +1,29 @@ +--- src/iconv.c.orig Fri Apr 26 20:50:13 2019 ++++ src/iconv.c Tue Jan 26 20:07:34 2021 +@@ -19,6 +19,8 @@ + # define ICONV_CONST + #endif + ++#include <unistd.h> ++ + #include <limits.h> + #include <stddef.h> + #include <stdio.h> +@@ -847,6 +849,8 @@ + int i; + int status; + ++ if (pledge("stdio rpath", NULL) == -1) ++ error(EXIT_FAILURE, errno, "pledge"); + set_program_name (argv[0]); + #if HAVE_SETLOCALE + /* Needed for the locale dependent encodings, "char" and "wchar_t", +@@ -1002,6 +1006,8 @@ + } + break; + } ++ if ((do_list || i == argc) && pledge("stdio", NULL) == -1) ++ error(EXIT_FAILURE, errno, "pledge"); + if (do_list) { + if (i != 2 || i != argc) + usage(1); -- 2.30.0 -- Kind regards, Hiltjo