Check write(2) return value on Debian 10

[snimmagadda]

On Debian 10(buster), with _FORTIFY_SOURCE=2 and Werror, the compiler mandates checking write(2) return value and a void cast isn't adequate...

pkgsrc/wip/opendoas$ uname -a Linux debian 4.19.0-12-amd64 #1 SMP Debian 4.19.152-1 (2020-10-18) x86_64 GNU/Linux pkgsrc/wip/opendoas$ bmake [...] ===> Building for opendoas-6.6.1 yacc parse.y mv -f y.tab.c parse.c cc -O2 -D_FORTIFY_SOURCE=2 -I/usr/pkg/include -I/usr/include -Dlinux -D_DEFAULT_SOURCE -D_GNU_SOURCE -I. -Ilibopenbsd -Wall -Wextra -Werror -pedantic -I/usr/pkg/include -I/usr/include -c -o parse.o parse.c cc -O2 -D_FORTIFY_SOURCE=2 -I/usr/pkg/include -I/usr/include -Dlinux -D_DEFAULT_SOURCE -D_GNU_SOURCE -I. -Ilibopenbsd -Wall -Wextra -Werror -pedantic -I/usr/pkg/include -I/usr/include -c -o doas.o doas.c cc -O2 -D_FORTIFY_SOURCE=2 -I/usr/pkg/include -I/usr/include -Dlinux -D_DEFAULT_SOURCE -D_GNU_SOURCE -I. -Ilibopenbsd -Wall -Wextra -Werror -pedantic -I/usr/pkg/include -I/usr/include -c -o env.o env.c cc -O2 -D_FORTIFY_SOURCE=2 -I/usr/pkg/include -I/usr/include -Dlinux -D_DEFAULT_SOURCE -D_GNU_SOURCE -I. -Ilibopenbsd -Wall -Wextra -Werror -pedantic -I/usr/pkg/include -I/usr/include -c -o libopenbsd/strlcat.o libopenbsd/strlcat.c cc -O2 -D_FORTIFY_SOURCE=2 -I/usr/pkg/include -I/usr/include -Dlinux -D_DEFAULT_SOURCE -D_GNU_SOURCE -I. -Ilibopenbsd -Wall -Wextra -Werror -pedantic -I/usr/pkg/include -I/usr/include -c -o libopenbsd/strlcpy.o libopenbsd/strlcpy.c cc -O2 -D_FORTIFY_SOURCE=2 -I/usr/pkg/include -I/usr/include -Dlinux -D_DEFAULT_SOURCE -D_GNU_SOURCE -I. -Ilibopenbsd -Wall -Wextra -Werror -pedantic -I/usr/pkg/include -I/usr/include -c -o libopenbsd/errc.o libopenbsd/errc.c cc -O2 -D_FORTIFY_SOURCE=2 -I/usr/pkg/include -I/usr/include -Dlinux -D_DEFAULT_SOURCE -D_GNU_SOURCE -I. -Ilibopenbsd -Wall -Wextra -Werror -pedantic -I/usr/pkg/include -I/usr/include -c -o libopenbsd/verrc.o libopenbsd/verrc.c cc -O2 -D_FORTIFY_SOURCE=2 -I/usr/pkg/include -I/usr/include -Dlinux -D_DEFAULT_SOURCE -D_GNU_SOURCE -I. -Ilibopenbsd -Wall -Wextra -Werror -pedantic -I/usr/pkg/include -I/usr/include -c -o libopenbsd/progname.o libopenbsd/progname.c cc -O2 -D_FORTIFY_SOURCE=2 -I/usr/pkg/include -I/usr/include -Dlinux -D_DEFAULT_SOURCE -D_GNU_SOURCE -I. -Ilibopenbsd -Wall -Wextra -Werror -pedantic -I/usr/pkg/include -I/usr/include -c -o libopenbsd/readpassphrase.o libopenbsd/readpassphrase.c libopenbsd/readpassphrase.c: In function 'readpassphrase': libopenbsd/readpassphrase.c:135:9: error: ignoring return value of 'write', declared with attribute warn_unused_result [-Werror=unused-result] (void)write(output, prompt, strlen(prompt)); ^~~~~~~~~ libopenbsd/readpassphrase.c:154:9: error: ignoring return value of 'write', declared with attribute warn_unused_result [-Werror=unused-result] (void)write(output, "\n", 1); ^~~~~~ cc1: all warnings being treated as errors make: [: libopenbsd/readpassphrase.o] Error 1 rm parse.c Error code 2

Stop. bmake[1]: stopped in /home/skn/pkgsrc/wip/opendoas *** Error code 1

Stop. bmake: stopped in /home/skn/pkgsrc/wip/opendoas

skn@debian:~/pkgsrc/wip/opendoas$ cc -v Using built-in specs. COLLECT_GCC=cc COLLECT_LTO_WRAPPER=/usr/lib/gcc/x86_64-linux-gnu/8/lto-wrapper OFFLOAD_TARGET_NAMES=nvptx-none OFFLOAD_TARGET_DEFAULT=1 Target: x86_64-linux-gnu Configured with: ../src/configure -v --with-pkgversion='Debian 8.3.0-6' --with-bugurl=file:///usr/share/doc/gcc-8/README.Bugs --enable-languages=c,ada,c++,go,brig,d,fortran,objc,obj-c++ --prefix=/usr --with-gcc-major-version-only --program-suffix=-8 --program-prefix=x86_64-linux-gnu- --enable-shared --enable-linker-build-id --libexecdir=/usr/lib --without-included-gettext --enable-threads=posix --libdir=/usr/lib --enable-nls --enable-bootstrap --enable-clocale=gnu --enable-libstdcxx-debug --enable-libstdcxx-time=yes --with-default-libstdcxx-abi=new --enable-gnu-unique-object --disable-vtable-verify --enable-libmpx --enable-plugin --enable-default-pie --with-system-zlib --with-target-system-zlib --enable-objc-gc=auto --enable-multiarch --disable-werror --with-arch-32=i686 --with-abi=m64 --with-multilib-list=m32,m64,mx32 --enable-multilib --with-tune=generic --enable-offload-targets=nvptx-none --without-cuda-driver --enable-checking=release --build=x86_64-linux-gnu --host=x86_64-linux-gnu --target=x86_64-linux-gnu Thread model: posix gcc version 8.3.0 (Debian 8.3.0-6)

With the surrounding signal handlers and tty settings, returning a NULL on -1 with the following diff feels inadequate. However, callers of readpassphrase exit on NULL so at least signal handlers aren't a problem, not sure about tty settings though.

diff --git a/libopenbsd/readpassphrase.c b/libopenbsd/readpassphrase.c
index 87675aa..2d10554 100644
--- a/libopenbsd/readpassphrase.c
+++ b/libopenbsd/readpassphrase.c
@@ -132,7 +132,9 @@ restart:
        (void)sigaction(SIGTTOU, &sa, &savettou);

        if (!(flags & RPP_STDIN))
-               (void)write(output, prompt, strlen(prompt));
+               if (write(output, prompt, strlen(prompt)) == -1)
+                       return(NULL);
+
        end = buf + bufsiz - 1;
        p = buf;
        while ((nr = read(input, &ch, 1)) == 1 && ch != '\n' && ch != '\r') {
@@ -151,7 +153,8 @@ restart:
        *p = '\0';
        save_errno = errno;
        if (!(term.c_lflag & ECHO))
-               (void)write(output, "\n", 1);
+               if (write(output, "\n", 1) == -1)
+                       return(NULL);

        /* Restore old terminal settings and signals. */
        if (memcmp(&term, &oterm, sizeof(term)) != 0) {

[Duncaen]

I think removing -Werror from the default CFLAGS is better, this file is a copy from openssh-portable not sure if its good to divert here.

[snimmagadda]

I agree, best to stay in sync with upstream.