From mboxrd@z Thu Jan 1 00:00:00 1970 X-Msuck: nntp://news.gmane.org/gmane.linux.lib.musl.general/11553 Path: news.gmane.org!.POSTED!not-for-mail From: Omer Anson Newsgroups: gmane.linux.lib.musl.general Subject: [PATCH] Be more precise using va_arg in fcntl Date: Wed, 21 Jun 2017 20:22:39 +0300 Message-ID: <1498065759-4116-1-git-send-email-oaanson@gmail.com> References: <20170621010811.GC1627@brightrain.aerifal.cx> Reply-To: musl@lists.openwall.com NNTP-Posting-Host: blaine.gmane.org X-Trace: blaine.gmane.org 1498065803 9198 195.159.176.226 (21 Jun 2017 17:23:23 GMT) X-Complaints-To: usenet@blaine.gmane.org NNTP-Posting-Date: Wed, 21 Jun 2017 17:23:23 +0000 (UTC) Cc: Omer Anson To: musl@lists.openwall.com Original-X-From: musl-return-11566-gllmg-musl=m.gmane.org@lists.openwall.com Wed Jun 21 19:23:18 2017 Return-path: Envelope-to: gllmg-musl@m.gmane.org Original-Received: from mother.openwall.net ([195.42.179.200]) by blaine.gmane.org with smtp (Exim 4.84_2) (envelope-from ) id 1dNjLa-00024z-M4 for gllmg-musl@m.gmane.org; Wed, 21 Jun 2017 19:23:18 +0200 Original-Received: (qmail 9531 invoked by uid 550); 21 Jun 2017 17:23:21 -0000 Mailing-List: contact musl-help@lists.openwall.com; run by ezmlm Precedence: bulk List-Post: List-Help: List-Unsubscribe: List-Subscribe: List-ID: Original-Received: (qmail 9502 invoked from network); 21 Jun 2017 17:23:19 -0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:cc:subject:date:message-id:in-reply-to:references; bh=/GZ8/2wxRqBlzxKEYrWNwU+/frK46gjaT767CZOH4R8=; b=SgJxmmiyxVo628oLZHfy/viduzGRRA+6JVZcppRZXTGd7OOrALbR3RyWvDn8a5AARI 4WvhU/woqTn+3qqcWh3uv6EMio5og/12dZpIv/Ee2EOID7z6XKNdeEubo6QFQnL34L4s 2tKyJxyb0eS0CZhuGgN1jlnZ1vthiVg5jTxZgKwoGnGF3GMqyLMijAb1738nheSTctFy xliz26kOTi7/sD7tooTI1nNAOZZbuAOC1vA1AjMCUDSM57x1bgunyAWinfbwF60+f5cT kqcjKnf9oGb4t2lmSIXYcAzBXMLk3XegT3aBVxGLN9l1zfIHSLTujKwsxPkL8oUPaV+P 3tyg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references; bh=/GZ8/2wxRqBlzxKEYrWNwU+/frK46gjaT767CZOH4R8=; b=HIIPRTLcRBVuMHGTD2Kr/kUN4Kj7ztyEXhBAvhdskKUjHWH07dTylQR8nyE8XF/kwU aNBu/VKeY8hcSdvgFE/pbeC8U7IurcZLfUV0DtpYz82CXHMwZJ0EZcitqWTholfdzbHi pDmwv/gIYFRZTswrq+QnYNMSmt3YfAjzU955P3wy0wA7Q0XOGtScKpjG6o7UFZwwutvq R7b1PHOsiHM+lL9R9ZTd1fhPY4rxgsYOCnEtSnD9SfkRaSoV8chSJsm8w0+mGuzXKoS7 DkwsUOJRBUubEien7AxOnPBTiieeAmQsBKyHP/fbkziq8p8dqbVHvMVNF7vOxFr9j1tm DbRQ== X-Gm-Message-State: AKS2vOxKDLiO4RZ1zfKOT3NZuFBleoMKwBVCBYlmNPqYNEkd9/IFSQeo EWvbPX4uMYBiqU0u X-Received: by 10.223.155.141 with SMTP id d13mr2935109wrc.196.1498065788087; Wed, 21 Jun 2017 10:23:08 -0700 (PDT) X-Mailer: git-send-email 2.4.11 In-Reply-To: <20170621010811.GC1627@brightrain.aerifal.cx> Xref: news.gmane.org gmane.linux.lib.musl.general:11553 Archived-At: Currently, fcntl automatically calls va_arg with type unsigned long, regardless of the actual invoked command, and therefore ignoring whether such an argument exists, or its type. According to the C standard[1][2], calling va_arg if the types do not match, or beyond the end of the parameter list, causes an undefined behaviour. This change modifies fcntl to try and call va_arg only when necessary, and with the correct parameter type. It relies on the cmd argument to know what's the expected command. In case the cmd is unknown, it falls back to the previous behaviour. [1] http://www.open-std.org/jtc1/sc22/wg14/www/standards [2] http://pubs.opengroup.org/onlinepubs/9699919799/basedefs/stdarg.h.html This is version 2 of the patch which takes Rich's comments into account: * The macro was removed. va_arg is called explicitly in every case. I wasn't comfortable with the one va_start, many va_end solution, so I duplicated the calls to va_start as well (as in the original patch posted by Rich). * Case blocks were removed. Variables were moved up to beginning of function. * In this patch, unknown command handling is unchanged with respect to the original code. This is intentional. --- src/fcntl/fcntl.c | 78 +++++++++++++++++++++++++++++++++++++++++++------------ 1 file changed, 61 insertions(+), 17 deletions(-) diff --git a/src/fcntl/fcntl.c b/src/fcntl/fcntl.c index ce615d0..1daf4c5 100644 --- a/src/fcntl/fcntl.c +++ b/src/fcntl/fcntl.c @@ -7,22 +7,38 @@ int fcntl(int fd, int cmd, ...) { + int arg_int; + struct flock * arg_flock; + struct f_owner_ex * arg_owner_ex; + struct f_owner_ex ex; unsigned long arg; va_list ap; - va_start(ap, cmd); - arg = va_arg(ap, unsigned long); - va_end(ap); - if (cmd == F_SETFL) arg |= O_LARGEFILE; - if (cmd == F_SETLKW) return syscall_cp(SYS_fcntl, fd, cmd, (void *)arg); - if (cmd == F_GETOWN) { - struct f_owner_ex ex; - int ret = __syscall(SYS_fcntl, fd, F_GETOWN_EX, &ex); - if (ret == -EINVAL) return __syscall(SYS_fcntl, fd, cmd, (void *)arg); + int ret; + + switch (cmd) { + case F_GETFD: + case F_GETFL: + case F_GETSIG: + case F_GETLEASE: + case F_GET_SEALS: + case F_GETPIPE_SZ: + return syscall(SYS_fcntl, fd, cmd); + case F_GETOWN: + ret = __syscall(SYS_fcntl, fd, F_GETOWN_EX, &ex); + if (ret == -EINVAL) return __syscall(SYS_fcntl, fd, cmd); if (ret) return __syscall_ret(ret); return ex.type == F_OWNER_PGRP ? -ex.pid : ex.pid; - } - if (cmd == F_DUPFD_CLOEXEC) { - int ret = __syscall(SYS_fcntl, fd, F_DUPFD_CLOEXEC, arg); + case F_SETFL: + va_start(ap, cmd); + arg_int = va_arg(ap, int); + va_end(ap); + arg_int |= O_LARGEFILE; + return syscall(SYS_fcntl, fd, cmd, arg_int); + case F_DUPFD_CLOEXEC: + va_start(ap, cmd); + arg_int = va_arg(ap, int); + va_end(ap); + ret = __syscall(SYS_fcntl, fd, F_DUPFD_CLOEXEC, arg_int); if (ret != -EINVAL) { if (ret >= 0) __syscall(SYS_fcntl, ret, F_SETFD, FD_CLOEXEC); @@ -33,17 +49,45 @@ int fcntl(int fd, int cmd, ...) if (ret >= 0) __syscall(SYS_close, ret); return __syscall_ret(-EINVAL); } - ret = __syscall(SYS_fcntl, fd, F_DUPFD, arg); + ret = __syscall(SYS_fcntl, fd, F_DUPFD, arg_int); if (ret >= 0) __syscall(SYS_fcntl, ret, F_SETFD, FD_CLOEXEC); return __syscall_ret(ret); - } - switch (cmd) { - case F_SETLK: + case F_DUPFD: + case F_SETFD: + case F_SETSIG: + case F_SETLEASE: + case F_ADD_SEALS: + case F_SETOWN: + case F_SETPIPE_SZ: + case F_NOTIFY: + va_start(ap, cmd); + arg_int = va_arg(ap, int); + va_end(ap); + return syscall(SYS_fcntl, fd, cmd, arg_int); case F_GETLK: + case F_SETLK: + case F_OFD_SETLK: + case F_OFD_GETLK: + va_start(ap, cmd); + arg_flock = va_arg(ap, struct flock *); + va_end(ap); + return syscall(SYS_fcntl, fd, cmd, arg_flock); + case F_OFD_SETLKW: + case F_SETLKW: + va_start(ap, cmd); + arg_flock = va_arg(ap, struct flock *); + va_end(ap); + return syscall_cp(SYS_fcntl, fd, cmd, arg_flock); case F_GETOWN_EX: case F_SETOWN_EX: - return syscall(SYS_fcntl, fd, cmd, (void *)arg); + va_start(ap, cmd); + arg_owner_ex = va_arg(ap, struct f_owner_ex *); + va_end(ap); + return syscall(SYS_fcntl, fd, cmd, arg_owner_ex); default: + va_start(ap, cmd); + arg = va_arg(ap, unsigned long); + va_end(ap); return syscall(SYS_fcntl, fd, cmd, arg); } } -- 2.4.11