From patchwork Tue Oct 18 08:01:19 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Riku Voipio X-Patchwork-Id: 77938 Delivered-To: patch@linaro.org Received: by 10.140.97.247 with SMTP id m110csp759647qge; Tue, 18 Oct 2016 01:03:04 -0700 (PDT) X-Received: by 10.55.203.23 with SMTP id d23mr1498241qkj.143.1476777784667; Tue, 18 Oct 2016 01:03:04 -0700 (PDT) Return-Path: Received: from lists.gnu.org (lists.gnu.org. [2001:4830:134:3::11]) by mx.google.com with ESMTPS id g124si20189959qkf.285.2016.10.18.01.03.04 for (version=TLS1 cipher=AES128-SHA bits=128/128); Tue, 18 Oct 2016 01:03:04 -0700 (PDT) Received-SPF: pass (google.com: domain of qemu-devel-bounces+patch=linaro.org@nongnu.org designates 2001:4830:134:3::11 as permitted sender) client-ip=2001:4830:134:3::11; Authentication-Results: mx.google.com; spf=pass (google.com: domain of qemu-devel-bounces+patch=linaro.org@nongnu.org designates 2001:4830:134:3::11 as permitted sender) smtp.mailfrom=qemu-devel-bounces+patch=linaro.org@nongnu.org Received: from localhost ([::1]:39443 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bwPMW-0006kZ-AG for patch@linaro.org; Tue, 18 Oct 2016 04:03:04 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:59483) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bwPKy-00067z-HY for qemu-devel@nongnu.org; Tue, 18 Oct 2016 04:01:35 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bwPKu-00077a-Gk for qemu-devel@nongnu.org; Tue, 18 Oct 2016 04:01:28 -0400 Received: from jessie.kos.to ([212.47.231.226]:47556 helo=pilvi.kos.to) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1bwPKu-000769-7a for qemu-devel@nongnu.org; Tue, 18 Oct 2016 04:01:24 -0400 Received: from kos.to (jessie.kos.to [212.47.231.226]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by pilvi.kos.to (Postfix) with ESMTPSA id AC6312C001A; Tue, 18 Oct 2016 08:01:19 +0000 (UTC) Received: (nullmailer pid 20036 invoked by uid 1000); Tue, 18 Oct 2016 08:01:19 -0000 Date: Tue, 18 Oct 2016 08:01:19 +0000 From: Riku Voipio To: riku.voipio@linaro.org Message-ID: <20161018080119.GA19984@kos.to> References: <079e313c3d8c9be4a30ddc22c1aa183bfd32c923.1476710352.git.riku.voipio@linaro.org> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <079e313c3d8c9be4a30ddc22c1aa183bfd32c923.1476710352.git.riku.voipio@linaro.org> User-Agent: Mutt/1.5.23 (2014-03-12) X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 212.47.231.226 Subject: Re: [Qemu-devel] [PULL 06/22] linux-user: Fix syslog() syscall support X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Peter Maydell , Aleksandar Markovic , qemu-devel@nongnu.org Errors-To: qemu-devel-bounces+patch=linaro.org@nongnu.org Sender: "Qemu-devel" On Mon, Oct 17, 2016 at 04:24:24PM +0300, riku.voipio@linaro.org wrote: > From: Aleksandar Markovic > > There are currently several problems related to syslog() support. > > For example, if the second argument "bufp" of target syslog() syscall > is NULL, the current implementation always returns error code EFAULT. > However, NULL is a perfectly valid value for the second argument for > many use cases of this syscall. This is, for example, visible from > this excerpt of man page for syslog(2): > > > EINVAL Bad arguments (e.g., bad type; or for type 2, 3, or 4, buf is > > NULL, or len is less than zero; or for type 8, the level is > > outside the range 1 to 8). > > Moreover, the argument "bufp" is ignored for all cases of values of the > first argument, except 2, 3 and 4. This means that for such cases > (the first argument is not 2, 3 or 4), there is no need to pass "buf" > between host and target, and it can be set to NULL while calling host's > syslog(), without loss of emulation accuracy. > > Note also that if "bufp" is NULL and the first argument is 2, 3 or 4, the > correct returned error code is EINVAL, not EFAULT. > > All these details are reflected in this patch. > > "#ifdef TARGET_NR_syslog" is also proprerly inserted when needed. > > Support for Qemu's "-strace" switch for syslog() syscall is included too. > > LTP tests syslog11 and syslog12 pass with this patch (while fail without > it), on any platform. > > Signed-off-by: Aleksandar Markovic > Signed-off-by: Riku Voipio > --- > linux-user/strace.c | 72 +++++++++++++++++++++++++++++++++++++++++++++++ > linux-user/strace.list | 2 +- > linux-user/syscall.c | 49 ++++++++++++++++++++++++++++---- > linux-user/syscall_defs.h | 25 ++++++++++++++++ > 4 files changed, 141 insertions(+), 7 deletions(-) > > diff --git a/linux-user/strace.c b/linux-user/strace.c > index a0e45b5..679f840 100644 > --- a/linux-user/strace.c > +++ b/linux-user/strace.c > @@ -1827,6 +1827,78 @@ print_rt_sigprocmask(const struct syscallname *name, > } > #endif > > +#ifdef TARGET_NR_syslog > +static void > +print_syslog_action(abi_ulong arg, int last) > +{ > + const char *type; > + > + switch (arg) { > + case TARGET_SYSLOG_ACTION_CLOSE: { > + type = "SYSLOG_ACTION_CLOSE"; > + break; > + } > + case TARGET_SYSLOG_ACTION_OPEN: { > + type = "SYSLOG_ACTION_OPEN"; > + break; > + } > + case TARGET_SYSLOG_ACTION_READ: { > + type = "SYSLOG_ACTION_READ"; > + break; > + } > + case TARGET_SYSLOG_ACTION_READ_ALL: { > + type = "SYSLOG_ACTION_READ_ALL"; > + break; > + } > + case TARGET_SYSLOG_ACTION_READ_CLEAR: { > + type = "SYSLOG_ACTION_READ_CLEAR"; > + break; > + } > + case TARGET_SYSLOG_ACTION_CLEAR: { > + type = "SYSLOG_ACTION_CLEAR"; > + break; > + } > + case TARGET_SYSLOG_ACTION_CONSOLE_OFF: { > + type = "SYSLOG_ACTION_CONSOLE_OFF"; > + break; > + } > + case TARGET_SYSLOG_ACTION_CONSOLE_ON: { > + type = "SYSLOG_ACTION_CONSOLE_ON"; > + break; > + } > + case TARGET_SYSLOG_ACTION_CONSOLE_LEVEL: { > + type = "SYSLOG_ACTION_CONSOLE_LEVEL"; > + break; > + } > + case TARGET_SYSLOG_ACTION_SIZE_UNREAD: { > + type = "SYSLOG_ACTION_SIZE_UNREAD"; > + break; > + } > + case TARGET_SYSLOG_ACTION_SIZE_BUFFER: { > + type = "SYSLOG_ACTION_SIZE_BUFFER"; > + break; > + } > + default: { > + print_raw_param("%ld", arg, last); > + return; > + } > + } > + gemu_log("%s%s", type, get_comma(last)); > +} > + > +static void > +print_syslog(const struct syscallname *name, > + abi_long arg0, abi_long arg1, abi_long arg2, > + abi_long arg3, abi_long arg4, abi_long arg5) > +{ > + print_syscall_prologue(name); > + print_syslog_action(arg0, 0); > + print_pointer(arg1, 0); > + print_raw_param("%d", arg2, 1); > + print_syscall_epilogue(name); > +} > +#endif > + > #ifdef TARGET_NR_mknod > static void > print_mknod(const struct syscallname *name, > diff --git a/linux-user/strace.list b/linux-user/strace.list > index f6dd044..2c7ad2b 100644 > --- a/linux-user/strace.list > +++ b/linux-user/strace.list > @@ -1486,7 +1486,7 @@ > { TARGET_NR_sys_kexec_load, "sys_kexec_load" , NULL, NULL, NULL }, > #endif > #ifdef TARGET_NR_syslog > -{ TARGET_NR_syslog, "syslog" , NULL, NULL, NULL }, > +{ TARGET_NR_syslog, "syslog" , NULL, print_syslog, NULL }, > #endif > #ifdef TARGET_NR_sysmips > { TARGET_NR_sysmips, "sysmips" , NULL, NULL, NULL }, > diff --git a/linux-user/syscall.c b/linux-user/syscall.c > index 05b4c41..a3e7d51 100644 > --- a/linux-user/syscall.c > +++ b/linux-user/syscall.c > @@ -9339,14 +9339,51 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1, > ret = do_setsockopt(arg1, arg2, arg3, arg4, (socklen_t) arg5); > break; > #endif > - > +#if defined(TARGET_NR_syslog) > case TARGET_NR_syslog: > - if (!(p = lock_user_string(arg2))) > - goto efault; > - ret = get_errno(sys_syslog((int)arg1, p, (int)arg3)); > - unlock_user(p, arg2, 0); > - break; > + { > + int len = arg2; > > + switch (arg1) { > + case TARGET_SYSLOG_ACTION_CLOSE: /* Close log */ > + case TARGET_SYSLOG_ACTION_OPEN: /* Open log */ > + case TARGET_SYSLOG_ACTION_CLEAR: /* Clear ring buffer */ > + case TARGET_SYSLOG_ACTION_CONSOLE_OFF: /* Disable logging */ > + case TARGET_SYSLOG_ACTION_CONSOLE_ON: /* Enable logging */ > + case TARGET_SYSLOG_ACTION_CONSOLE_LEVEL: /* Set messages level */ > + case TARGET_SYSLOG_ACTION_SIZE_UNREAD: /* Number of chars */ > + case TARGET_SYSLOG_ACTION_SIZE_BUFFER: /* Size of the buffer */ > + { > + ret = get_errno(sys_syslog((int)arg1, NULL, (int)arg3)); > + } > + break; > + case TARGET_SYSLOG_ACTION_READ: /* Read from log */ > + case TARGET_SYSLOG_ACTION_READ_CLEAR: /* Read/clear msgs */ > + case TARGET_SYSLOG_ACTION_READ_ALL: /* Read last messages */ > + { > + if (len < 0) { > + ret = -TARGET_EINVAL; > + goto fail; > + } > + if (len == 0) { > + break; > + } > + p = lock_user(VERIFY_WRITE, arg2, arg3, 0); > + if (!p) { > + ret = -TARGET_EINVAL; > + goto fail; > + } > + ret = get_errno(sys_syslog((int)arg1, p, (int)arg3)); The error paths above are incorrect, compared to: http://lxr.free-electrons.com/source/kernel/printk/printk.c#L1465 I'll squash in the following change to match the kernel behaviour: > + unlock_user(p, arg2, arg3); > + } > + break; > + default: > + ret = -EINVAL; > + break; > + } > + } > + break; > +#endif > case TARGET_NR_setitimer: > { > struct itimerval value, ovalue, *pvalue; > diff --git a/linux-user/syscall_defs.h b/linux-user/syscall_defs.h > index adb7153..61270ef 100644 > --- a/linux-user/syscall_defs.h > +++ b/linux-user/syscall_defs.h > @@ -2688,4 +2688,29 @@ struct target_user_cap_data { > uint32_t inheritable; > }; > > +/* from kernel's include/linux/syslog.h */ > + > +/* Close the log. Currently a NOP. */ > +#define TARGET_SYSLOG_ACTION_CLOSE 0 > +/* Open the log. Currently a NOP. */ > +#define TARGET_SYSLOG_ACTION_OPEN 1 > +/* Read from the log. */ > +#define TARGET_SYSLOG_ACTION_READ 2 > +/* Read all messages remaining in the ring buffer. */ > +#define TARGET_SYSLOG_ACTION_READ_ALL 3 > +/* Read and clear all messages remaining in the ring buffer */ > +#define TARGET_SYSLOG_ACTION_READ_CLEAR 4 > +/* Clear ring buffer. */ > +#define TARGET_SYSLOG_ACTION_CLEAR 5 > +/* Disable printk's to console */ > +#define TARGET_SYSLOG_ACTION_CONSOLE_OFF 6 > +/* Enable printk's to console */ > +#define TARGET_SYSLOG_ACTION_CONSOLE_ON 7 > +/* Set level of messages printed to console */ > +#define TARGET_SYSLOG_ACTION_CONSOLE_LEVEL 8 > +/* Return number of unread characters in the log buffer */ > +#define TARGET_SYSLOG_ACTION_SIZE_UNREAD 9 > +/* Return size of the log buffer */ > +#define TARGET_SYSLOG_ACTION_SIZE_BUFFER 10 > + > #endif > -- > 2.1.4 > > diff --git a/linux-user/syscall.c b/linux-user/syscall.c index 2072b1f..dfc483c 100644 --- a/linux-user/syscall.c +++ b/linux-user/syscall.c @@ -9377,16 +9377,17 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1, case TARGET_SYSLOG_ACTION_READ_CLEAR: /* Read/clear msgs */ case TARGET_SYSLOG_ACTION_READ_ALL: /* Read last messages */ { + ret = -TARGET_EINVAL; if (len < 0) { - ret = -TARGET_EINVAL; goto fail; } + ret = 0; if (len == 0) { break; } p = lock_user(VERIFY_WRITE, arg2, arg3, 0); if (!p) { - ret = -TARGET_EINVAL; + ret = -TARGET_EFAULT; goto fail; } ret = get_errno(sys_syslog((int)arg1, p, (int)arg3));