[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 1/2] linux-user: Add setsockopt(SO_ATTACH_FILTER
From: |
Peter Maydell |
Subject: |
Re: [Qemu-devel] [PATCH 1/2] linux-user: Add setsockopt(SO_ATTACH_FILTER) |
Date: |
Mon, 31 Dec 2012 20:56:23 +0000 |
On 31 December 2012 19:37, Laurent Vivier <address@hidden> wrote:
> This is needed to be able to run dhclient.
>
> Signed-off-by: Laurent Vivier <address@hidden>
> ---
> linux-user/syscall.c | 34 +++++++++++++++++++++++++++++++++-
> linux-user/syscall_defs.h | 12 ++++++++++++
> 2 files changed, 45 insertions(+), 1 deletion(-)
>
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index e99adab..000b640 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -98,6 +98,7 @@ int __clone2(int (*fn)(void *), void *child_stack_base,
> #include <linux/fb.h>
> #include <linux/vt.h>
> #include <linux/dm-ioctl.h>
> +#include <linux/filter.h>
> #include "linux_loop.h"
> #include "cpu-uname.h"
>
> @@ -1491,6 +1492,38 @@ static abi_long do_setsockopt(int sockfd, int level,
> int optname,
> break;
> case TARGET_SOL_SOCKET:
> switch (optname) {
> + case TARGET_SO_ATTACH_FILTER: {
This brace should go on a line of its own (lined up with the 'c' in case) IMHO.
> + struct target_sock_fprog *tfprog;
> + struct target_sock_filter *tfilter;
> + struct sock_fprog fprog;
> + struct sock_filter *filter;
> + int i;
> +
> + if (optlen != sizeof(*tfprog))
> + return -TARGET_EINVAL;
QEMU style requires braces on this if. You can use checkpatch.pl
to catch this kind of thing.
> + if (!lock_user_struct(VERIFY_READ, tfprog, optval_addr, 0))
> + return -TARGET_EFAULT;
> + if (!lock_user_struct(VERIFY_READ, tfilter,
> + tswapal(tfprog->filter), 0))
> + return -TARGET_EFAULT;
This will fail to unlock tfprog in the failure case.
> +
> + fprog.len = tswap16(tfprog->len);
> + filter = alloca(fprog.len * sizeof(*filter));
Not sure an unconstrained-size alloca based on data from
the guest binary is a fantastic idea (though we no doubt
do something similar for some other syscalls).
> + for (i = 0; i < fprog.len; i ++) {
> + filter[i].code = tswap16(tfilter[i].code);
> + filter[i].jt = tfilter[i].jt;
> + filter[i].jf = tfilter[i].jf;
> + filter[i].k = tswap32(tfilter[i].k);
> + }
> + fprog.filter = filter;
> +
> + ret = get_errno(setsockopt(sockfd, SOL_SOCKET,
> + SO_ATTACH_FILTER, &fprog, sizeof(fprog)));
> +
> + unlock_user_struct(tfilter, tfprog->filter, 1);
> + unlock_user_struct(tfprog, optval_addr, 1);
> + return ret;
> + }
> /* Options with 'int' argument. */
> case TARGET_SO_DEBUG:
> optname = SO_DEBUG;
> @@ -1548,7 +1581,6 @@ static abi_long do_setsockopt(int sockfd, int level,
> int optname,
> case TARGET_SO_SNDTIMEO:
> optname = SO_SNDTIMEO;
> break;
> - break;
Nice catch, but this is an unrelated change that should go in its own patch.
> default:
> goto unimplemented;
> }
-- PMM