Re: [PATCH 4/4] slirp: Add oob-eth-addr to -netdev options

2022-06-18 Thread Samuel Thibault
Peter Delevoryas, le mer. 15 juin 2022 18:05:26 -0700, a ecrit:
> With this change, you can now request the out-of-band MAC address from
> slirp in fby35-bmc:
> 
> wget 
> https://github.com/facebook/openbmc/releases/download/openbmc-e2294ff5d31d/fby35.mtd
> qemu-system-arm -machine fby35-bmc \
> -drive file=fby35.mtd,format=raw,if=mtd \
> -nographic \
> -netdev 
> user,id=nic,mfr-id=0x8119,oob-eth-addr=de:ad:be:ef:ca:fe,hostfwd=::-:22 \
> -net nic,model=ftgmac100,netdev=nic
> 
> ...
> username: root
> password: 0penBmc
> ...
> 
> root@bmc-oob:~# ncsi-util -n eth0 -c 0 0x50 0 0 0x81 0x19 0 0 0x1b 0
> NC-SI Command Response:
> cmd: NCSI_OEM_CMD(0x50)
> Response: COMMAND_COMPLETED(0x)  Reason: NO_ERROR(0x)
> Payload length = 24
> 
> 20: 0x00 0x00 0x81 0x19
> 24: 0x01 0x00 0x1b 0x00
> 28: 0x00 0x00 0x00 0x00
> 32: 0xde 0xad 0xbe 0xef
> 36: 0xca 0xfe 0x00 0x00
> 
> root@bmc-oob:~# ifconfig
> eth0  Link encap:Ethernet  HWaddr DE:AD:BE:EF:CA:FE
> inet addr:10.0.2.15  Bcast:10.0.2.255  Mask:255.255.255.0
> inet6 addr: fec0::dcad:beff:feef:cafe/64 Scope:Site
> inet6 addr: fe80::dcad:beff:feef:cafe/64 Scope:Link
> UP BROADCAST RUNNING MULTICAST  MTU:1500  Metric:1
> RX packets:253 errors:0 dropped:0 overruns:0 frame:0
> TX packets:271 errors:0 dropped:0 overruns:0 carrier:0
> collisions:0 txqueuelen:1000
> RX bytes:24638 (24.0 KiB)  TX bytes:18876 (18.4 KiB)
> Interrupt:32
> 
> loLink encap:Local Loopback
> inet addr:127.0.0.1  Mask:255.0.0.0
> inet6 addr: ::1/128 Scope:Host
> UP LOOPBACK RUNNING  MTU:65536  Metric:1
> RX packets:2 errors:0 dropped:0 overruns:0 frame:0
> TX packets:2 errors:0 dropped:0 overruns:0 carrier:0
> collisions:0 txqueuelen:1000
> RX bytes:120 (120.0 B)  TX bytes:120 (120.0 B)
> 
> Signed-off-by: Peter Delevoryas 
> ---
>  net/slirp.c   | 13 +++--
>  qapi/net.json |  5 -
>  2 files changed, 15 insertions(+), 3 deletions(-)
> 
> diff --git a/net/slirp.c b/net/slirp.c
> index 231068c1e2..858d3da859 100644
> --- a/net/slirp.c
> +++ b/net/slirp.c
> @@ -414,7 +414,7 @@ static int net_slirp_init(NetClientState *peer, const 
> char *model,
>const char *smb_export, const char *vsmbserver,
>const char **dnssearch, const char *vdomainname,
>const char *tftp_server_name, uint32_t mfr_id,
> -  Error **errp)
> +  uint8_t oob_eth_addr[ETH_ALEN], Error **errp)
>  {
>  /* default settings according to historic slirp */
>  struct in_addr net  = { .s_addr = htonl(0x0a000200) }; /* 10.0.2.0 */
> @@ -637,6 +637,7 @@ static int net_slirp_init(NetClientState *peer, const 
> char *model,
>  cfg.vdnssearch = dnssearch;
>  cfg.vdomainname = vdomainname;
>  cfg.mfr_id = mfr_id;
> +memcpy(cfg.oob_eth_addr, oob_eth_addr, ETH_ALEN);

And similarly here.

>  s->slirp = slirp_new(, _cb, s);
>  QTAILQ_INSERT_TAIL(_stacks, s, entry);
>  
> @@ -1142,6 +1143,7 @@ int net_init_slirp(const Netdev *netdev, const char 
> *name,
>  const NetdevUserOptions *user;
>  const char **dnssearch;
>  bool ipv4 = true, ipv6 = true;
> +MACAddr oob_eth_addr = {};
>  
>  assert(netdev->type == NET_CLIENT_DRIVER_USER);
>  user = >u.user;
> @@ -1166,6 +1168,12 @@ int net_init_slirp(const Netdev *netdev, const char 
> *name,
>  net_init_slirp_configs(user->hostfwd, SLIRP_CFG_HOSTFWD);
>  net_init_slirp_configs(user->guestfwd, 0);
>  
> +if (user->has_oob_eth_addr &&
> +net_parse_macaddr(oob_eth_addr.a, user->oob_eth_addr) < 0) {
> +error_setg(errp, "invalid syntax for OOB ethernet address");
> +return -1;
> +}
> +
>  ret = net_slirp_init(peer, "user", name, user->q_restrict,
>   ipv4, vnet, user->host,
>   ipv6, user->ipv6_prefix, user->ipv6_prefixlen,
> @@ -1173,7 +1181,8 @@ int net_init_slirp(const Netdev *netdev, const char 
> *name,
>   user->bootfile, user->dhcpstart,
>   user->dns, user->ipv6_dns, user->smb,
>   user->smbserver, dnssearch, user->domainname,
> - user->tftp_server_name, user->mfr_id, errp);
> + user->tftp_server_name, user->mfr_id, 
> oob_eth_addr.a,
> + errp);
>  
>  while (slirp_configs) {
>  config = slirp_configs;
> diff --git a/qapi/net.json b/qapi/net.json
> index efc5cb3fb6..7b2c3c205c 100644
> --- a/qapi/net.json
> +++ b/qapi/net.json
> @@ -169,6 +169,8 @@
>  #
>  # @mfr-id: Manufacturer ID (Private Enterprise Number: IANA)
>  #
> +# @oob-eth-addr: 

Re: [PATCH 3/4] slirp: Add mfr-id to -netdev options

2022-06-18 Thread Samuel Thibault
Peter Delevoryas, le mer. 15 juin 2022 18:05:25 -0700, a ecrit:
> This lets you set the manufacturer's ID for a slirp netdev, which can be
> queried from the guest through the Get Version ID NC-SI command. For
> example, by setting the manufacturer's ID to 0x8119:
> 
> wget 
> https://github.com/facebook/openbmc/releases/download/openbmc-e2294ff5d31d/fby35.mtd
> qemu-system-arm -machine fby35-bmc \
> -drive file=fby35.mtd,format=raw,if=mtd -nographic \
> -netdev user,id=nic,mfr-id=0x8119,hostfwd=::-:22 \
> -net nic,model=ftgmac100,netdev=nic
> ...
> username: root
> password: 0penBmc
> ...
> root@bmc-oob:~# ncsi-util 0x15
> NC-SI Command Response:
> cmd: GET_VERSION_ID(0x15)
> Response: COMMAND_COMPLETED(0x)  Reason: NO_ERROR(0x)
> Payload length = 40
> 
> 20: 0xf1 0xf0 0xf0 0x00
> 24: 0x00 0x00 0x00 0x00
> 28: 0x00 0x00 0x00 0x00
> 32: 0x00 0x00 0x00 0x00
> 36: 0x00 0x00 0x00 0x00
> 40: 0x00 0x00 0x00 0x00
> 44: 0x00 0x00 0x00 0x00
> 48: 0x00 0x00 0x00 0x00
> 52: 0x00 0x00 0x81 0x19
> 
> Signed-off-by: Peter Delevoryas 
> ---
>  net/slirp.c   | 5 +++--
>  qapi/net.json | 5 -
>  2 files changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/net/slirp.c b/net/slirp.c
> index 75e5ccafd9..231068c1e2 100644
> --- a/net/slirp.c
> +++ b/net/slirp.c
> @@ -413,7 +413,7 @@ static int net_slirp_init(NetClientState *peer, const 
> char *model,
>const char *vnameserver, const char *vnameserver6,
>const char *smb_export, const char *vsmbserver,
>const char **dnssearch, const char *vdomainname,
> -  const char *tftp_server_name,
> +  const char *tftp_server_name, uint32_t mfr_id,
>Error **errp)
>  {
>  /* default settings according to historic slirp */
> @@ -636,6 +636,7 @@ static int net_slirp_init(NetClientState *peer, const 
> char *model,
>  cfg.vnameserver6 = ip6_dns;
>  cfg.vdnssearch = dnssearch;
>  cfg.vdomainname = vdomainname;
> +cfg.mfr_id = mfr_id;

You will need a #if to only include it with slirp 4.8.0 indeed.
Otherwise the mfr_id field won't exist.

In the #else part, it would probably useful to give the user at least a
warning that tells her to upgrade slirp to 4.8.0 to get the mfr_id
functionality working.

>  s->slirp = slirp_new(, _cb, s);
>  QTAILQ_INSERT_TAIL(_stacks, s, entry);
>  
> @@ -1172,7 +1173,7 @@ int net_init_slirp(const Netdev *netdev, const char 
> *name,
>   user->bootfile, user->dhcpstart,
>   user->dns, user->ipv6_dns, user->smb,
>   user->smbserver, dnssearch, user->domainname,
> - user->tftp_server_name, errp);
> + user->tftp_server_name, user->mfr_id, errp);
>  
>  while (slirp_configs) {
>  config = slirp_configs;
> diff --git a/qapi/net.json b/qapi/net.json
> index d6f7cfd4d6..efc5cb3fb6 100644
> --- a/qapi/net.json
> +++ b/qapi/net.json
> @@ -167,6 +167,8 @@
>  #
>  # @tftp-server-name: RFC2132 "TFTP server name" string (Since 3.1)
>  #
> +# @mfr-id: Manufacturer ID (Private Enterprise Number: IANA)
> +#
>  # Since: 1.2
>  ##
>  { 'struct': 'NetdevUserOptions',
> @@ -192,7 +194,8 @@
>  '*smbserver': 'str',
>  '*hostfwd':   ['String'],
>  '*guestfwd':  ['String'],
> -'*tftp-server-name': 'str' } }
> +'*tftp-server-name': 'str',
> +'*mfr-id': 'uint32' } }
>  
>  ##
>  # @NetdevTapOptions:
> -- 
> 2.30.2
> 



Re: [PATCH 2/4] slirp: Update SlirpConfig version to 5

2022-06-18 Thread Samuel Thibault
Hello,

Peter Delevoryas, le mer. 15 juin 2022 18:05:24 -0700, a ecrit:
> I think we probably need a new Slirp release
> (4.8.0) and a switch statement here instead, right?
> 
> So that we can preserve the behavior for 4.7.0?

Yes, that's the idea.

Samuel



Re: [RFC PATCH 3/4] net: slirp: add support for CFI-friendly timer API

2022-04-26 Thread Samuel Thibault
Paolo Bonzini, le mar. 12 avril 2022 14:13:36 +0200, a ecrit:
> libslirp 4.7 introduces a CFI-friendly version of the .timer_new callback.
> The new callback replaces the function pointer with an enum; invoking the
> callback is done with a new function slirp_handle_timer.
> 
> Support the new API so that CFI can be made compatible with using a system
> libslirp.
> 
> Signed-off-by: Paolo Bonzini 

Reviewed-by: Samuel Thibault 

> ---
>  net/slirp.c | 41 -
>  1 file changed, 40 insertions(+), 1 deletion(-)
> 
> diff --git a/net/slirp.c b/net/slirp.c
> index b3a92d6e38..57af42299d 100644
> --- a/net/slirp.c
> +++ b/net/slirp.c
> @@ -184,10 +184,43 @@ static int64_t net_slirp_clock_get_ns(void *opaque)
>  return qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
>  }
>  
> +typedef struct SlirpTimer SlirpTimer;
>  struct SlirpTimer {
>  QEMUTimer timer;
> +#if SLIRP_CHECK_VERSION(4,7,0)
> +Slirp *slirp;
> +SlirpTimerId id;
> +void *cb_opaque;
> +#endif
> +};
> +
> +#if SLIRP_CHECK_VERSION(4,7,0)
> +static void net_slirp_init_completed(Slirp *slirp, void *opaque)
> +{
> +SlirpState *s = opaque;
> +s->slirp = slirp;
>  }
>  
> +static void net_slirp_timer_cb(void *opaque)
> +{
> +SlirpTimer *t = opaque;
> +slirp_handle_timer(t->slirp, t->id, t->cb_opaque);
> +}
> +
> +static void *net_slirp_timer_new_opaque(SlirpTimerId id,
> +void *cb_opaque, void *opaque)
> +{
> +SlirpState *s = opaque;
> +SlirpTimer *t = g_new(SlirpTimer, 1);
> +t->slirp = s->slirp;
> +t->id = id;
> +t->cb_opaque = cb_opaque;
> +timer_init_full(>timer, NULL, QEMU_CLOCK_VIRTUAL,
> +SCALE_MS, QEMU_TIMER_ATTR_EXTERNAL,
> +net_slirp_timer_cb, t);
> +return t;
> +}
> +#else
>  static void *net_slirp_timer_new(SlirpTimerCb cb,
>   void *cb_opaque, void *opaque)
>  {
> @@ -197,6 +230,7 @@ static void *net_slirp_timer_new(SlirpTimerCb cb,
>  cb, cb_opaque);
>  return t;
>  }
> +#endif
>  
>  static void net_slirp_timer_free(void *timer, void *opaque)
>  {
> @@ -231,7 +265,12 @@ static const SlirpCb slirp_cb = {
>  .send_packet = net_slirp_send_packet,
>  .guest_error = net_slirp_guest_error,
>  .clock_get_ns = net_slirp_clock_get_ns,
> +#if SLIRP_CHECK_VERSION(4,7,0)
> +.init_completed = net_slirp_init_completed,
> +.timer_new_opaque = net_slirp_timer_new_opaque,
> +#else
>  .timer_new = net_slirp_timer_new,
> +#endif
>  .timer_free = net_slirp_timer_free,
>  .timer_mod = net_slirp_timer_mod,
>  .register_poll_fd = net_slirp_register_poll_fd,
> @@ -578,7 +617,7 @@ static int net_slirp_init(NetClientState *peer, const 
> char *model,
>  
>  s = DO_UPCAST(SlirpState, nc, nc);
>  
> -cfg.version = 3;
> +cfg.version = SLIRP_CHECK_VERSION(4,7,0) ? 4 : 3;
>  cfg.restricted = restricted;
>  cfg.in_enabled = ipv4;
>  cfg.vnetwork = net;
> -- 
> 2.35.1
> 
> 

-- 
Samuel
---
Pour une évaluation indépendante, transparente et rigoureuse !
Je soutiens la Commission d'Évaluation de l'Inria.



Re: [RFC PATCH 2/4] net: slirp: switch to slirp_new

2022-04-26 Thread Samuel Thibault
Paolo Bonzini, le mar. 12 avril 2022 14:13:35 +0200, a ecrit:
> Replace slirp_init with slirp_new, so that a more recent cfg.version
> can be specified.
> 
> Signed-off-by: Paolo Bonzini 

Reviewed-by: Samuel Thibault 

> ---
>  net/slirp.c | 27 +--
>  1 file changed, 21 insertions(+), 6 deletions(-)
> 
> diff --git a/net/slirp.c b/net/slirp.c
> index f1e25d741f..b3a92d6e38 100644
> --- a/net/slirp.c
> +++ b/net/slirp.c
> @@ -389,6 +389,7 @@ static int net_slirp_init(NetClientState *peer, const 
> char *model,
>  #if defined(CONFIG_SMBD_COMMAND)
>  struct in_addr smbsrv = { .s_addr = 0 };
>  #endif
> +SlirpConfig cfg = { 0 };
>  NetClientState *nc;
>  SlirpState *s;
>  char buf[20];
> @@ -577,12 +578,26 @@ static int net_slirp_init(NetClientState *peer, const 
> char *model,
>  
>  s = DO_UPCAST(SlirpState, nc, nc);
>  
> -s->slirp = slirp_init(restricted, ipv4, net, mask, host,
> -  ipv6, ip6_prefix, vprefix6_len, ip6_host,
> -  vhostname, tftp_server_name,
> -  tftp_export, bootfile, dhcp,
> -  dns, ip6_dns, dnssearch, vdomainname,
> -  _cb, s);
> +cfg.version = 3;
> +cfg.restricted = restricted;
> +cfg.in_enabled = ipv4;
> +cfg.vnetwork = net;
> +cfg.vnetmask = mask;
> +cfg.vhost = host;
> +cfg.in6_enabled = ipv6;
> +cfg.vprefix_addr6 = ip6_prefix;
> +cfg.vprefix_len = vprefix6_len;
> +cfg.vhost6 = ip6_host;
> +cfg.vhostname = vhostname;
> +cfg.tftp_server_name = tftp_server_name;
> +cfg.tftp_path = tftp_export;
> +cfg.bootfile = bootfile;
> +cfg.vdhcp_start = dhcp;
> +cfg.vnameserver = dns;
> +cfg.vnameserver6 = ip6_dns;
> +cfg.vdnssearch = dnssearch;
> +cfg.vdomainname = vdomainname;
> +s->slirp = slirp_new(, _cb, s);
>  QTAILQ_INSERT_TAIL(_stacks, s, entry);
>  
>  /*
> -- 
> 2.35.1
> 
> 

-- 
Samuel
---
Pour une évaluation indépendante, transparente et rigoureuse !
Je soutiens la Commission d'Évaluation de l'Inria.



Re: [RFC PATCH 4/4] net: slirp: allow CFI with libslirp >= 4.7

2022-04-26 Thread Samuel Thibault
Paolo Bonzini, le mar. 12 avril 2022 14:13:37 +0200, a ecrit:
> slirp 4.7 introduces a new CFI-friendly timer callback that does
> not pass function pointers within libslirp as callbacks for timers.
> Check the version number and, if it is new enough, allow using CFI
> even with a system libslirp.
> 
> Signed-off-by: Paolo Bonzini 

Reviewed-by: Samuel Thibault 

> ---
>  meson.build | 24 
>  1 file changed, 12 insertions(+), 12 deletions(-)
> 
> diff --git a/meson.build b/meson.build
> index 861de93c4f..92a83580a3 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -2485,21 +2485,21 @@ if have_system
>  slirp = declare_dependency(link_with: libslirp,
> dependencies: slirp_deps,
> include_directories: slirp_inc)
> +  else
> +# slirp <4.7 is incompatible with CFI support in QEMU.  This is because
> +# it passes function pointers within libslirp as callbacks for timers.
> +# When using a system-wide shared libslirp, the type information for the
> +# callback is missing and the timer call produces a false positive with 
> CFI.
> +#
> +# Now that slirp_opt has been defined, check if the selected slirp is 
> compatible
> +# with control-flow integrity.
> +if get_option('cfi') and slirp.found() and 
> slirp.version().version_compare('<4.7')
> +  error('Control-Flow Integrity is not compatible with system-wide 
> slirp.' \
> + + ' Please configure with --enable-slirp=git or upgrade to 
> libslirp 4.7')
> +endif
>endif
>  endif
>  
> -# For CFI, we need to compile slirp as a static library together with qemu.
> -# This is because we register slirp functions as callbacks for QEMU Timers.
> -# When using a system-wide shared libslirp, the type information for the
> -# callback is missing and the timer call produces a false positive with CFI.
> -#
> -# Now that slirp_opt has been defined, check if the selected slirp is 
> compatible
> -# with control-flow integrity.
> -if get_option('cfi') and slirp_opt == 'system'
> -  error('Control-Flow Integrity is not compatible with system-wide slirp.' \
> - + ' Please configure with --enable-slirp=git')
> -endif
> -
>  fdt = not_found
>  if have_system
>fdt_opt = get_option('fdt')
> -- 
> 2.35.1
> 

-- 
Samuel
---
Pour une évaluation indépendante, transparente et rigoureuse !
Je soutiens la Commission d'Évaluation de l'Inria.



Re: [RFC PATCH 1/4] net: slirp: introduce a wrapper struct for QemuTimer

2022-04-26 Thread Samuel Thibault
Paolo Bonzini, le mar. 12 avril 2022 14:13:34 +0200, a ecrit:
> This struct will be extended in the next few patches to support the
> new slirp_handle_timer() call.  For that we need to store an additional
> "int" for each SLIRP timer, in addition to the cb_opaque.
> 
> Signed-off-by: Paolo Bonzini 

Reviewed-by: Samuel Thibault 

> ---
>  net/slirp.c | 19 ++-
>  1 file changed, 14 insertions(+), 5 deletions(-)
> 
> diff --git a/net/slirp.c b/net/slirp.c
> index bc5e9e4f77..f1e25d741f 100644
> --- a/net/slirp.c
> +++ b/net/slirp.c
> @@ -184,23 +184,32 @@ static int64_t net_slirp_clock_get_ns(void *opaque)
>  return qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
>  }
>  
> +struct SlirpTimer {
> +QEMUTimer timer;
> +}
> +
>  static void *net_slirp_timer_new(SlirpTimerCb cb,
>   void *cb_opaque, void *opaque)
>  {
> -return timer_new_full(NULL, QEMU_CLOCK_VIRTUAL,
> -  SCALE_MS, QEMU_TIMER_ATTR_EXTERNAL,
> -  cb, cb_opaque);
> +SlirpTimer *t = g_new(SlirpTimer, 1);
> +timer_init_full(>timer, NULL, QEMU_CLOCK_VIRTUAL,
> +SCALE_MS, QEMU_TIMER_ATTR_EXTERNAL,
> +cb, cb_opaque);
> +return t;
>  }
>  
>  static void net_slirp_timer_free(void *timer, void *opaque)
>  {
> -timer_free(timer);
> +SlirpTimer *t = timer;
> +timer_del(>timer);
> +g_free(t);
>  }
>  
>  static void net_slirp_timer_mod(void *timer, int64_t expire_timer,
>  void *opaque)
>  {
> -timer_mod(timer, expire_timer);
> +SlirpTimer *t = timer;
> +timer_mod(>timer, expire_timer);
>  }
>  
>  static void net_slirp_register_poll_fd(int fd, void *opaque)
> -- 
> 2.35.1
> 
> 

-- 
Samuel
---
Pour une évaluation indépendante, transparente et rigoureuse !
Je soutiens la Commission d'Évaluation de l'Inria.



Re: [PATCH] configure: Disable capstone and slirp in the --without-default-features mode

2022-02-21 Thread Samuel Thibault
Thomas Huth, le lun. 21 févr. 2022 10:06:47 +0100, a ecrit:
> For the users, it looks a little bit weird that capstone and slirp are
> not disabled automatically if they run the configure script with the
> "--without-default-features" option, so let's do that now.
> Note: fdt is *not* changed accordingly since this affects the targets
> that we can build, so disabling fdt automatically here might have
> unexpected side-effects for the users.
> 
> Signed-off-by: Thomas Huth 

Acked-by: Samuel Thibault 

> ---
>  I thought I sent out that patch a couple of weeks ago already, but
>  I cannot find it in the archives, so I likely missed to send it
>  correctly. Anyway, sorry if you've got this twice!
> 
>  configure | 9 +++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/configure b/configure
> index 3a29eff5cc..36d10d95bb 100755
> --- a/configure
> +++ b/configure
> @@ -361,9 +361,14 @@ slirp_smbd="$default_feature"
>  # are included in the automatically generated help message)
>  
>  # 1. Track which submodules are needed
> -capstone="auto"
> +if test "$default_feature" = no ; then
> +  capstone="disabled"
> +  slirp="disabled"
> +else
> +  capstone="auto"
> +  slirp="auto"
> +fi
>  fdt="auto"
> -slirp="auto"
>  
>  # 2. Support --with/--without option
>  default_devices="true"
> -- 
> 2.27.0



Re: [PATCH] ide: Cap LBA28 capacity announcement to 2^28-1

2021-10-05 Thread Samuel Thibault
Ping?

Samuel Thibault, le mar. 24 août 2021 12:43:44 +0200, a ecrit:
> The LBA28 capacity (at offsets 60/61 of identification) is supposed to
> express the maximum size supported by LBA28 commands. If the device is
> larger than this, we have to cap it to 2^28-1.
> 
> At least NetBSD happens to be using this value to determine whether to use
> LBA28 or LBA48 for its commands, using LBA28 for sectors that don't need
> LBA48. This commit thus fixes NetBSD access to disks larger than 128GiB.
> 
> Signed-off-by: Samuel Thibault 
> ---
>  hw/ide/core.c | 8 ++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/ide/core.c b/hw/ide/core.c
> index fd69ca3167..e28f8aad61 100644
> --- a/hw/ide/core.c
> +++ b/hw/ide/core.c
> @@ -98,8 +98,12 @@ static void put_le16(uint16_t *p, unsigned int v)
>  static void ide_identify_size(IDEState *s)
>  {
>  uint16_t *p = (uint16_t *)s->identify_data;
> -put_le16(p + 60, s->nb_sectors);
> -put_le16(p + 61, s->nb_sectors >> 16);
> +int64_t nb_sectors_lba28 = s->nb_sectors;
> +if (nb_sectors_lba28 >= 1 << 28) {
> +nb_sectors_lba28 = (1 << 28) - 1;
> +}
> +put_le16(p + 60, nb_sectors_lba28);
> +put_le16(p + 61, nb_sectors_lba28 >> 16);
>  put_le16(p + 100, s->nb_sectors);
>  put_le16(p + 101, s->nb_sectors >> 16);
>  put_le16(p + 102, s->nb_sectors >> 32);
> -- 
> 2.32.0
> 



Re: [PATCH] net/slirp: Use newer slirp_*_hostxfwd API

2021-10-05 Thread Samuel Thibault
Nicholas Ngai, le sam. 25 sept. 2021 16:22:02 -0700, a ecrit:
> Sorry for the duplicate email. The cc’s for the maintainers on the email
> didn’t go through the first time.
> 
> Nicholas Ngai
> 
> On 9/25/21 2:48 PM, Nicholas Ngai wrote:
> > libslirp provides a newer slirp_*_hostxfwd API meant for
> > address-agnostic forwarding instead of the is_udp parameter which is
> > limited to just TCP/UDP.
> > 
> > Signed-off-by: Nicholas Ngai 

Reviewed-by: Samuel Thibault 

> > ---
> >   net/slirp.c | 64 +++--
> >   1 file changed, 42 insertions(+), 22 deletions(-)
> > 
> > diff --git a/net/slirp.c b/net/slirp.c
> > index ad3a838e0b..49ae01a2f0 100644
> > --- a/net/slirp.c
> > +++ b/net/slirp.c
> > @@ -643,12 +643,17 @@ static SlirpState *slirp_lookup(Monitor *mon, const 
> > char *id)
> >   void hmp_hostfwd_remove(Monitor *mon, const QDict *qdict)
> >   {
> > -struct in_addr host_addr = { .s_addr = INADDR_ANY };
> > -int host_port;
> > +struct sockaddr_in host_addr = {
> > +.sin_family = AF_INET,
> > +.sin_addr = {
> > +.s_addr = INADDR_ANY,
> > +},
> > +};
> > +int port;
> > +int flags = 0;
> >   char buf[256];
> >   const char *src_str, *p;
> >   SlirpState *s;
> > -int is_udp = 0;
> >   int err;
> >   const char *arg1 = qdict_get_str(qdict, "arg1");
> >   const char *arg2 = qdict_get_try_str(qdict, "arg2");
> > @@ -670,9 +675,9 @@ void hmp_hostfwd_remove(Monitor *mon, const QDict 
> > *qdict)
> >   }
> >   if (!strcmp(buf, "tcp") || buf[0] == '\0') {
> > -is_udp = 0;
> > +/* Do nothing; already TCP. */
> >   } else if (!strcmp(buf, "udp")) {
> > -is_udp = 1;
> > +flags |= SLIRP_HOSTFWD_UDP;
> >   } else {
> >   goto fail_syntax;
> >   }
> > @@ -680,15 +685,17 @@ void hmp_hostfwd_remove(Monitor *mon, const QDict 
> > *qdict)
> >   if (get_str_sep(buf, sizeof(buf), , ':') < 0) {
> >   goto fail_syntax;
> >   }
> > -if (buf[0] != '\0' && !inet_aton(buf, _addr)) {
> > +if (buf[0] != '\0' && !inet_aton(buf, _addr.sin_addr)) {
> >   goto fail_syntax;
> >   }
> > -if (qemu_strtoi(p, NULL, 10, _port)) {
> > +if (qemu_strtoi(p, NULL, 10, )) {
> >   goto fail_syntax;
> >   }
> > +host_addr.sin_port = htons(port);
> > -err = slirp_remove_hostfwd(s->slirp, is_udp, host_addr, host_port);
> > +err = slirp_remove_hostxfwd(s->slirp, (struct sockaddr *) _addr,
> > +sizeof(host_addr), flags);
> >   monitor_printf(mon, "host forwarding rule for %s %s\n", src_str,
> >  err ? "not found" : "removed");
> > @@ -700,12 +707,22 @@ void hmp_hostfwd_remove(Monitor *mon, const QDict 
> > *qdict)
> >   static int slirp_hostfwd(SlirpState *s, const char *redir_str, Error 
> > **errp)
> >   {
> > -struct in_addr host_addr = { .s_addr = INADDR_ANY };
> > -struct in_addr guest_addr = { .s_addr = 0 };
> > -int host_port, guest_port;
> > +struct sockaddr_in host_addr = {
> > +.sin_family = AF_INET,
> > +.sin_addr = {
> > +.s_addr = INADDR_ANY,
> > +},
> > +};
> > +struct sockaddr_in guest_addr = {
> > +.sin_family = AF_INET,
> > +.sin_addr = {
> > +.s_addr = 0,
> > +},
> > +};
> > +int flags = 0;
> > +int port;
> >   const char *p;
> >   char buf[256];
> > -int is_udp;
> >   char *end;
> >   const char *fail_reason = "Unknown reason";
> > @@ -715,9 +732,9 @@ static int slirp_hostfwd(SlirpState *s, const char 
> > *redir_str, Error **errp)
> >   goto fail_syntax;
> >   }
> >   if (!strcmp(buf, "tcp") || buf[0] == '\0') {
> > -is_udp = 0;
> > +/* Do nothing; already TCP. */
> >   } else if (!strcmp(buf, "udp")) {
> > -is_udp = 1;
> > +flags |= SLIRP_HOSTFWD_UDP;
> >   } else {
> >   fail_reason = "Bad protocol name";
> >   goto fail_syntax;
> > @@ -727,7 +744,7 @@ static int slirp_hostfwd(SlirpState *s, const char 
> > *redir_str, Error **

Re: [PATCH] ide: Cap LBA28 capacity announcement to 2^28-1

2021-09-05 Thread Samuel Thibault
Ping?

Samuel Thibault, le mar. 24 août 2021 12:43:44 +0200, a ecrit:
> The LBA28 capacity (at offsets 60/61 of identification) is supposed to
> express the maximum size supported by LBA28 commands. If the device is
> larger than this, we have to cap it to 2^28-1.
> 
> At least NetBSD happens to be using this value to determine whether to use
> LBA28 or LBA48 for its commands, using LBA28 for sectors that don't need
> LBA48. This commit thus fixes NetBSD access to disks larger than 128GiB.
> 
> Signed-off-by: Samuel Thibault 
> ---
>  hw/ide/core.c | 8 ++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/ide/core.c b/hw/ide/core.c
> index fd69ca3167..e28f8aad61 100644
> --- a/hw/ide/core.c
> +++ b/hw/ide/core.c
> @@ -98,8 +98,12 @@ static void put_le16(uint16_t *p, unsigned int v)
>  static void ide_identify_size(IDEState *s)
>  {
>  uint16_t *p = (uint16_t *)s->identify_data;
> -put_le16(p + 60, s->nb_sectors);
> -put_le16(p + 61, s->nb_sectors >> 16);
> +int64_t nb_sectors_lba28 = s->nb_sectors;
> +if (nb_sectors_lba28 >= 1 << 28) {
> +nb_sectors_lba28 = (1 << 28) - 1;
> +}
> +put_le16(p + 60, nb_sectors_lba28);
> +put_le16(p + 61, nb_sectors_lba28 >> 16);
>  put_le16(p + 100, s->nb_sectors);
>  put_le16(p + 101, s->nb_sectors >> 16);
>  put_le16(p + 102, s->nb_sectors >> 32);
> -- 
> 2.32.0
> 



[PATCH] ide: Cap LBA28 capacity announcement to 2^28-1

2021-08-24 Thread Samuel Thibault
The LBA28 capacity (at offsets 60/61 of identification) is supposed to
express the maximum size supported by LBA28 commands. If the device is
larger than this, we have to cap it to 2^28-1.

At least NetBSD happens to be using this value to determine whether to use
LBA28 or LBA48 for its commands, using LBA28 for sectors that don't need
LBA48. This commit thus fixes NetBSD access to disks larger than 128GiB.

Signed-off-by: Samuel Thibault 
---
 hw/ide/core.c | 8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/hw/ide/core.c b/hw/ide/core.c
index fd69ca3167..e28f8aad61 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -98,8 +98,12 @@ static void put_le16(uint16_t *p, unsigned int v)
 static void ide_identify_size(IDEState *s)
 {
 uint16_t *p = (uint16_t *)s->identify_data;
-put_le16(p + 60, s->nb_sectors);
-put_le16(p + 61, s->nb_sectors >> 16);
+int64_t nb_sectors_lba28 = s->nb_sectors;
+if (nb_sectors_lba28 >= 1 << 28) {
+nb_sectors_lba28 = (1 << 28) - 1;
+}
+put_le16(p + 60, nb_sectors_lba28);
+put_le16(p + 61, nb_sectors_lba28 >> 16);
 put_le16(p + 100, s->nb_sectors);
 put_le16(p + 101, s->nb_sectors >> 16);
 put_le16(p + 102, s->nb_sectors >> 32);
-- 
2.32.0




Re: [OSS-Fuzz] Assertion Failure: !in6_zero(_addr) (#111)

2021-05-08 Thread Samuel Thibault
Hello,

Alexander Bulekov, le lun. 03 mai 2021 16:09:33 -0400, a ecrit:
> Forwarding this along to the list, so it doesn't get burried during the
> gitlab issue migration.

Thanks!

Pushed a proposed fix on

https://gitlab.freedesktop.org/slirp/libslirp/-/merge_requests/86

Samuel

> - Forwarded message from "Alexander Bulekov (@a1xndr)" 
>  -
> 
> Alexander Bulekov created an issue: 
> https://gitlab.com/qemu-project/qemu/-/issues/111
> 
> Hello,
> Reproducer
> ```
> cat << EOF | ./qemu-system-i386 -display none -machine accel=qtest, -m \
> 512M -M q35 -nodefaults -device e1000e,netdev=net0 -netdev user,id=net0 \
> -qtest stdio
> outl 0xcf8 0x8813
> outl 0xcfc 0x56
> outl 0xcf8 0x8801
> outl 0xcfc 0x0600
> write 0x56000403 0x1 0x02
> write 0x5600042b 0x1 0x80
> write 0x20a 0x1 0x86
> write 0x20b 0x1 0xdd
> write 0x20c 0x1 0x60
> write 0x212 0x1 0x11
> write 0x213 0x1 0x01
> write 0x224 0x1 0xfe
> write 0x225 0x1 0xc0
> write 0x233 0x1 0x02
> write 0x237 0x1 0x45
> write 0x23d 0x1 0x01
> write 0xb 0x1 0x24
> write 0x10 0x1 0xfe
> write 0x11 0x1 0x01
> write 0x19 0x1 0x01
> write 0x1a 0x1 0x10
> write 0x1b 0x1 0x25
> write 0x5600043a 0x1 0x04
> EOF
> ```
> 
> Stack-trace:
> ```
> SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior ../net/eth.c:374:27 in
> ../net/eth.c:375:27: runtime error: member access within misaligned address 
> 0x63114846 for type 'struct ip6_header', which requires 4 byte alignment
> 0x63114846: note: pointer points here
>  00 00 11 11 60 00  00 00 00 00 11 11 00 00  00 00 00 00 00 00 00 00  00 00 
> 00 00 00 00 fe c0  00 00
>  ^
> SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior ../net/eth.c:375:27 in
> qemu-fuzz-i386: ../slirp/src/ndp_table.c:59: _Bool ndp_table_search(Slirp *, 
> struct in6_addr, uint8_t *): Assertion `!in6_zero(_addr)' failed.
> 
> #8 in __assert_fail assert/assert.c:101:3
> #9 in ndp_table_search /slirp/src/ndp_table.c:59:5
> #10 in if_encap6 /slirp/src/slirp.c:926:10
> #11 in if_encap /slirp/src/slirp.c:967:15
> #12 in if_start /slirp/src/if.c:183:45
> #13 in ip6_output /slirp/src/ip6_output.c:35:9
> #14 in tftp_udp_output /slirp/src/tftp.c:161:9
> #15 in tftp_send_error /slirp/src/tftp.c:223:5
> #16 in tftp_handle_rrq /slirp/src/tftp.c
> #17 in tftp_input /slirp/src/tftp.c:453:9
> #18 in udp6_input /slirp/src/udp6.c:81:9
> #19 in slirp_input /slirp/src/slirp.c:847:13
> #20 in net_slirp_receive /net/slirp.c:136:5
> #21 in nc_sendv_compat /net/net.c
> #22 in qemu_deliver_packet_iov /net/net.c:765:15
> #23 in qemu_net_queue_deliver_iov /net/queue.c:179:11
> #24 in qemu_net_queue_send_iov /net/queue.c:246:11
> #25 in net_tx_pkt_sendv /hw/net/net_tx_pkt.c:558:9
> #26 in net_tx_pkt_send /hw/net/net_tx_pkt.c:633:9
> #27 in e1000e_tx_pkt_send /hw/net/e1000e_core.c:659:16
> #28 in e1000e_process_tx_desc /hw/net/e1000e_core.c:736:17
> #29 in e1000e_start_xmit /hw/net/e1000e_core.c:927:9
> #30 in e1000e_set_tdt /hw/net/e1000e_core.c:2444:9
> #31 in e1000e_core_write /hw/net/e1000e_core.c:3256:9
> #32 in memory_region_write_accessor /softmmu/memory.c:491:5
> #33 in access_with_adjusted_size /softmmu/memory.c:552:18
> #34 in memory_region_dispatch_write /softmmu/memory.c
> #35 in flatview_write_continue /softmmu/physmem.c:2746:23
> #36 in flatview_write /softmmu/physmem.c:2786:14
> #37 in address_space_write /softmmu/physmem.c:2878:18
> ```
> 
> Test-case:
> ```
> /*
>  * Autogenerated Fuzzer Test Case
>  *
>  * Copyright (c) 2021 
>  *
>  * This work is licensed under the terms of the GNU GPL, version 2 or later.
>  * See the COPYING file in the top-level directory.
>  */
> 
> #include "qemu/osdep.h"
> 
> #include "libqos/libqtest.h"
> 
> static void test_fuzz(void)
> {
> QTestState *s = qtest_init("-display none , -m 512M -M q35 -nodefaults 
> -device "
>"e1000e,netdev=net0 -netdev user,id=net0");
> qtest_outl(s, 0xcf8, 0x8813);
> qtest_outl(s, 0xcfc, 0x56);
> qtest_outl(s, 0xcf8, 0x8801);
> qtest_outl(s, 0xcfc, 0x0600);
> qtest_bufwrite(s, 0x56000403, "\x02", 0x1);
> qtest_bufwrite(s, 0x5600042b, "\x80", 0x1);
> qtest_bufwrite(s, 0x20a, "\x86", 0x1);
> qtest_bufwrite(s, 0x20b, "\xdd", 0x1);
> qtest_bufwrite(s, 0x20c, "\x60", 0x1);
> qtest_bufwrite(s, 0x212, "\x11", 0x1);
> qtest_bufwrite(s, 0x213, "\x01", 0x1);
> qtest_bufwrite(s, 0x224, "\xfe", 0x1);
> qtest_bufwrite(s, 0x225, "\xc0", 0x1);
> qtest_bufwrite(s, 0x233, "\x02", 0x1);
> qtest_bufwrite(s, 0x237, "\x45", 0x1);
> qtest_bufwrite(s, 0x23d, "\x01", 0x1);
> qtest_bufwrite(s, 0xb, "\x24", 0x1);
> qtest_bufwrite(s, 0x10, "\xfe", 0x1);
> qtest_bufwrite(s, 0x11, "\x01", 0x1);
> qtest_bufwrite(s, 0x19, "\x01", 0x1);
> qtest_bufwrite(s, 0x1a, "\x10", 0x1);
> qtest_bufwrite(s, 0x1b, "\x25", 0x1);
> qtest_bufwrite(s, 0x5600043a, "\x04", 0x1);
> qtest_quit(s);
> }
> int main(int argc, char **argv)
> {
> const char *arch = 

Re: [PATCH 03/23] chardev/baum: Use definitions to avoid dynamic stack allocation

2021-05-05 Thread Samuel Thibault
Marc-André Lureau, le jeu. 06 mai 2021 01:27:25 +0400, a ecrit:
> @@ -408,7 +408,7 @@ static int baum_eat_packet(BaumChardev *baum, const
> uint8_t *buf, int len)
>          }
>          timer_del(baum->cellCount_timer);
> 
> -        memset(zero, 0, sizeof(zero));
> +        memset(zero, 0, baum->x * baum->y);
> 
> 
> eh, I would have left the sizeof(zero)..

I was wondering too, but below this it's clear that only
baum->x * baum->y is getting used.

Samuel



Re: [PATCH 04/23] chardev/baum: Avoid dynamic stack allocation

2021-05-05 Thread Samuel Thibault
Philippe Mathieu-Daudé, le mer. 05 mai 2021 23:10:28 +0200, a ecrit:
> Use autofree heap allocation instead of variable-length
> array on the stack.
> 
> Signed-off-by: Philippe Mathieu-Daudé 

Reviewed-by: Samuel Thibault 

> ---
>  chardev/baum.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/chardev/baum.c b/chardev/baum.c
> index 0822e9ed5f3..bc09cda3471 100644
> --- a/chardev/baum.c
> +++ b/chardev/baum.c
> @@ -299,7 +299,8 @@ static void baum_chr_accept_input(struct Chardev *chr)
>  static void baum_write_packet(BaumChardev *baum, const uint8_t *buf, int len)
>  {
>  Chardev *chr = CHARDEV(baum);
> -uint8_t io_buf[1 + 2 * len], *cur = io_buf;
> +g_autofree uint8_t *io_buf = g_malloc(1 + 2 * len);
> +uint8_t *cur = io_buf;
>  int room;
>  *cur++ = ESC;
>  while (len--)
> -- 
> 2.26.3
> 



Re: [PATCH 03/23] chardev/baum: Use definitions to avoid dynamic stack allocation

2021-05-05 Thread Samuel Thibault
Philippe Mathieu-Daudé, le mer. 05 mai 2021 23:10:27 +0200, a ecrit:
> We know 'x * y' will be at most 'X_MAX * Y_MAX' (which is not
> a big value, it is actually 84). Instead of having the compiler
> use variable-length array, declare an array able to hold the
> maximum 'x * y'.
> 
> Signed-off-by: Philippe Mathieu-Daudé 

Reviewed-by: Samuel Thibault 

> ---
>  chardev/baum.c | 8 
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/chardev/baum.c b/chardev/baum.c
> index adc3d7b3b56..0822e9ed5f3 100644
> --- a/chardev/baum.c
> +++ b/chardev/baum.c
> @@ -383,9 +383,9 @@ static int baum_eat_packet(BaumChardev *baum, const 
> uint8_t *buf, int len)
>  switch (req) {
>  case BAUM_REQ_DisplayData:
>  {
> -uint8_t cells[baum->x * baum->y], c;
> -uint8_t text[baum->x * baum->y];
> -uint8_t zero[baum->x * baum->y];
> +uint8_t cells[X_MAX * Y_MAX], c;
> +uint8_t text[X_MAX * Y_MAX];
> +uint8_t zero[X_MAX * Y_MAX];
>  int cursor = BRLAPI_CURSOR_OFF;
>  int i;
>  
> @@ -408,7 +408,7 @@ static int baum_eat_packet(BaumChardev *baum, const 
> uint8_t *buf, int len)
>  }
>  timer_del(baum->cellCount_timer);
>  
> -memset(zero, 0, sizeof(zero));
> +memset(zero, 0, baum->x * baum->y);
>  
>  brlapi_writeArguments_t wa = {
>  .displayNumber = BRLAPI_DISPLAY_DEFAULT,
> -- 
> 2.26.3
> 



Re: [PATCH 02/23] chardev/baum: Replace magic values by X_MAX / Y_MAX definitions

2021-05-05 Thread Samuel Thibault
Philippe Mathieu-Daudé, le mer. 05 mai 2021 23:10:26 +0200, a ecrit:
> Replace '84' magic value by the X_MAX definition, and '1' by Y_MAX.
> 
> Signed-off-by: Philippe Mathieu-Daudé 

Reviewed-by: Samuel Thibault 

> ---
>  chardev/baum.c | 11 +++
>  1 file changed, 7 insertions(+), 4 deletions(-)
> 
> diff --git a/chardev/baum.c b/chardev/baum.c
> index 5deca778bc4..adc3d7b3b56 100644
> --- a/chardev/baum.c
> +++ b/chardev/baum.c
> @@ -87,6 +87,9 @@
>  
>  #define BUF_SIZE 256
>  
> +#define X_MAX   84
> +#define Y_MAX   1
> +
>  struct BaumChardev {
>  Chardev parent;
>  
> @@ -244,11 +247,11 @@ static int baum_deferred_init(BaumChardev *baum)
>  brlapi_perror("baum: brlapi__getDisplaySize");
>  return 0;
>  }
> -if (baum->y > 1) {
> -baum->y = 1;
> +if (baum->y > Y_MAX) {
> +baum->y = Y_MAX;
>  }
> -if (baum->x > 84) {
> -baum->x = 84;
> +if (baum->x > X_MAX) {
> +baum->x = X_MAX;
>  }
>  
>  con = qemu_console_lookup_by_index(0);
> -- 
> 2.26.3
> 



Re: [PATCH] net/slirp: Fix incorrect permissions on samba >= 2.0.5

2021-04-30 Thread Samuel Thibault
Laurent Vivier, le ven. 30 avril 2021 18:48:29 +0200, a ecrit:
> CC: +Samuel

I don't know the smb code at all.

> Le 23/02/2021 à 03:41, Niklas Hambüchen a écrit :
> > As the added commend and `man smb.conf` explain, starting
> > with that samba version, `force user` must be configured
> > in `[global]` in order to access the configured `smb_dir`.
> > 
> > This broke `-net user,smb=/path/to/folder`:
> > 
> > The `chdir` into e.g. `/run/user/0/qemu-smb.DCZ8Y0` failed.
> > In verbose logs, this manifested as:
> > 
> > [..., effective(65534, 65534), real(65534, 0)] 
> > /source3/smbd/service.c:159(chdir_current_service)
> >   chdir (/run/user/0) failed, reason: Permission denied
> > 
> > [..., effective(65534, 65534), real(65534, 0)] 
> > /source3/smbd/service.c:167(chdir_current_service)
> >   chdir (/run/user/0) failed, reason: Permission denied
> > 
> > [..., effective(65534, 65534), real(65534, 0)] 
> > /source3/smbd/uid.c:448(change_to_user_internal)
> >   change_to_user_internal: chdir_current_service() failed!
> > 
> > This commit fixes it by setting the `[global]` force user to
> > the user that owns the directories `smbd` needs to access.
> > 
> > Signed-off-by: Niklas Hambüchen 
> > ---
> >  net/slirp.c | 6 ++
> >  1 file changed, 6 insertions(+)
> > 
> > diff --git a/net/slirp.c b/net/slirp.c
> > index be914c0be0..82387bdb19 100644
> > --- a/net/slirp.c
> > +++ b/net/slirp.c
> > @@ -850,6 +850,11 @@ static int slirp_smb(SlirpState* s, const char 
> > *exported_dir,
> >  }
> >  fprintf(f,
> >  "[global]\n"
> > +"# In Samba 2.0.5 and above the 'force user' parameter\n"
> > +"# also causes the primary group of the forced user to be 
> > used\n"
> > +"# as the primary group for all file activity.\n"
> > +"# This includes the various directories set below.\n"
> > +"force user=%s\n"
> >  "private dir=%s\n"
> >  "interfaces=127.0.0.1\n"
> >  "bind interfaces only=yes\n"
> > @@ -871,6 +876,7 @@ static int slirp_smb(SlirpState* s, const char 
> > *exported_dir,
> >  "read only=no\n"
> >  "guest ok=yes\n"
> >  "force user=%s\n",
> > +passwd->pw_name,
> >  s->smb_dir,
> >  s->smb_dir,
> >  s->smb_dir,
> > 
> 



Re: [PATCH 3/4] usb: Un-deprecate -usbdevice (except for -usbdevice audio which gets removed)

2021-03-11 Thread Samuel Thibault
Gerd Hoffmann, le jeu. 11 mars 2021 12:37:38 +0100, a ecrit:
> Which would also drop support for serial braille devices.  Not sure
> how much of a problem that would be these days.

It is an important concern: we also need to be able to test braille
devices connected through serial.

Samuel



Re: [PATCH] baum: Fix crash when Braille output is not available

2021-03-10 Thread Samuel Thibault
Peter Maydell, le mer. 10 mars 2021 17:18:11 +, a ecrit:
> On Wed, 10 Mar 2021 at 16:08, Samuel Thibault
>  wrote:
> >
> > When Braille output is not available, the backend properly reports being
> > unable to be created, but 5f8e93c3e262 ("util/qemu-timer: Make timer_free()
> > imply timer_del()") made the timer_free() call now refuse any NULL
> > parameter. char_braille_finalize thus now has to be more careful with
> > calling it on baum->cellCount_timer.
> 
> It wasn't the intention of that commit to make freeing a NULL
> timer invalid;

Ok!

Samuel



[PATCH] baum: Fix crash when Braille output is not available

2021-03-10 Thread Samuel Thibault
When Braille output is not available, the backend properly reports being
unable to be created, but 5f8e93c3e262 ("util/qemu-timer: Make timer_free()
imply timer_del()") made the timer_free() call now refuse any NULL
parameter. char_braille_finalize thus now has to be more careful with
calling it on baum->cellCount_timer.

Signed-off-by: Samuel Thibault 
---
 chardev/baum.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/chardev/baum.c b/chardev/baum.c
index 5deca778bc..aca5bf12fb 100644
--- a/chardev/baum.c
+++ b/chardev/baum.c
@@ -631,7 +631,9 @@ static void char_braille_finalize(Object *obj)
 {
 BaumChardev *baum = BAUM_CHARDEV(obj);
 
-timer_free(baum->cellCount_timer);
+if (baum->cellCount_timer) {
+timer_free(baum->cellCount_timer);
+}
 if (baum->brlapi) {
 brlapi__closeConnection(baum->brlapi);
 g_free(baum->brlapi);
-- 
2.30.1




Re: [PATCH v2] usb: Un-deprecate -usbdevice (except for -usbdevice audio which gets removed)

2021-03-10 Thread Samuel Thibault
Daniel P. Berrangé, le mer. 10 mars 2021 15:31:48 +, a ecrit:
> On Wed, Mar 10, 2021 at 04:26:46PM +0100, Paolo Bonzini wrote:
> > On 10/03/21 16:02, Samuel Thibault wrote:
> > > > > When trying to remove the -usbdevice option, there were complaints 
> > > > > that
> > > > > "-usbdevice braille" is still a very useful shortcut for some people.
> > > > Pointer?  I missed it.
> > > 
> > > For instance
> > > https://lists.nongnu.org/archive/html/qemu-devel/2018-01/msg00693.html
> > 
> > In one sentence: "Braille is worth a special case because a subset of our
> > user base (blind people) will use it 100% of the time, plus it is not
> > supported by libvirt and hence virt-manager".
> 
> If simplicity of enabling braille support is critical, we could get
> something even simpler than "-usbdevice braille", and just provide
> a bare  "-braille" with no args required as a "do the right thing"
> option ?

That was discussed a bit earlier in the thread:

https://lists.nongnu.org/archive/html/qemu-devel/2018-01/msg00681.html
https://lists.nongnu.org/archive/html/qemu-devel/2018-01/msg00686.html
https://lists.nongnu.org/archive/html/qemu-devel/2018-01/msg00687.html

Just like keyboard/mouse, one would still want to specify whether the
braille device is to be connected through usb or serial, so at least
"-braille usb" and "-braille serial".

Note

https://lists.nongnu.org/archive/html/qemu-devel/2018-01/msg00689.html

Paolo wrote:
> Adding magic to "-device usb-braille" that creates both a
> front-end and a back-end is completely the opposite of sane...

The thing is: creating one without the other does not make sense.

Samuel



Re: [PATCH v2] usb: Un-deprecate -usbdevice (except for -usbdevice audio which gets removed)

2021-03-10 Thread Samuel Thibault
Markus Armbruster, le mer. 10 mars 2021 14:17:48 +0100, a ecrit:
> Thomas Huth  writes:
> > When trying to remove the -usbdevice option, there were complaints that
> > "-usbdevice braille" is still a very useful shortcut for some people.
> 
> Pointer?  I missed it.

For instance
https://lists.nongnu.org/archive/html/qemu-devel/2018-01/msg00693.html

> "braille" is the only driver with a factory.  "-usbdevice braille" is
> sugar for
> 
>   -device usb-braille,chardev=braille -chardev braille,id=braille
>   -machine usb=on
> 
> It's buggy:
> 
> $ qemu-system-x86_64 -S -usbdevice braille
> qemu-system-x86_64: -usbdevice braille: '-usbdevice' is deprecated, 
> please use '-device usb-...' instead
> [three seconds tick by...]
> Segmentation fault (core dumped)

I don't have time to keep testing the usbdevice braille option of qemu
from git indeed.

But I do use the option at least weekly/monthly with releases. And some
braille users do depend on it for their daily work.

> It neglects to actually parse PARAMS:
> 
> $ qemu-system-x86_64 -S -usbdevice braille:"I'm a Little Teapot"
> qemu-system-x86_64: -usbdevice braille:I'm a Little Teapot: '-usbdevice' 
> is deprecated, please use '-device usb-...' instead

I wasn't aware of the issue, thus not fixed of course.

> The only one that has something approaching a leg to stand on is
> braille.  Still, I fail to see why having to specify a backend is fine
> for any number of other devices, but not for braille.

Because people who use this option have no idea how they would be
supposed to make it work otherwise.

>   -device usb-braille,chardev=braille -chardev braille,id=braille
>   -machine usb=on

This is *way* beyond what people would work out, even if documented. For
them a braille device is just one device, there is no need for notion of
frontend/backend, this is all in just one "device" for them. Put another
way, it does not even make sense to build anything different from what
you wrote: using another chardev backend with usb-braille frontend
doesn't make sense, and exposing the braille chardev backend on another
usb frontend doesn't make sense either.

> If users need more time, we can extend the grace period.

They don't need time, they need things that are simple to use. That
three-option black magic really isn't.

Samuel



Re: [PATCH v2 4/4] slirp: feature detection for smbd

2021-03-09 Thread Samuel Thibault
Joelle van Dyne, le mar. 09 mars 2021 10:11:31 -0800, a ecrit:
> On Tue, Mar 9, 2021 at 6:09 AM Philippe Mathieu-Daudé  
> wrote:
> > On 3/9/21 1:27 AM, Joelle van Dyne wrote:
> > > Replace Windows specific macro with a more generic feature detection
> > > macro. Allows slirp smb feature to be disabled manually as well.
> > >
> > > Signed-off-by: Joelle van Dyne 
> > > ---
> > >  configure   | 26 +++---
> > >  meson.build |  2 +-
> > >  net/slirp.c | 16 
> > >  3 files changed, 32 insertions(+), 12 deletions(-)
> >
> > Hmm v1 had: Acked-by: Samuel Thibault 
> > did you change something to not include Samuel A-b tag?
> 
> Sorry, I must have missed it!

NP :)

Samuel



Re: [PATCH 4/4] slirp: feature detection for smbd

2021-03-08 Thread Samuel Thibault
Joelle van Dyne, le dim. 07 mars 2021 22:48:21 -0800, a ecrit:
> Replace Windows specific macro with a more generic feature detection
> macro. Allows slirp smb feature to be disabled manually as well.
> 
> Signed-off-by: Joelle van Dyne 

Acked-by: Samuel Thibault 

> ---
>  configure   | 26 +++---
>  meson.build |  3 +++
>  net/slirp.c | 16 
>  3 files changed, 34 insertions(+), 11 deletions(-)
> 
> diff --git a/configure b/configure
> index 34fccaa2ba..8335a3e6a0 100755
> --- a/configure
> +++ b/configure
> @@ -465,6 +465,7 @@ fuse_lseek="auto"
>  multiprocess="auto"
>  
>  malloc_trim="auto"
> +slirp_smbd="auto"
>  
>  # parse CC options second
>  for opt do
> @@ -834,8 +835,6 @@ do
>  fi
>  done
>  
> -: ${smbd=${SMBD-/usr/sbin/smbd}}
> -
>  # Default objcc to clang if available, otherwise use CC
>  if has clang; then
>objcc=clang
> @@ -1560,6 +1559,10 @@ for opt do
>;;
>--disable-multiprocess) multiprocess="disabled"
>;;
> +  --enable-slirp-smbd) slirp_smbd=yes
> +  ;;
> +  --disable-slirp-smbd) slirp_smbd=no
> +  ;;
>*)
>echo "ERROR: unknown option $opt"
>echo "Try '$0 --help' for more information"
> @@ -1913,6 +1916,7 @@ disabled with --disable-FEATURE, default is enabled if 
> available
>fuseFUSE block device export
>fuse-lseek  SEEK_HOLE/SEEK_DATA support for FUSE exports
>multiprocessOut of process device emulation support
> +  slirp-smbd  use smbd (at path --smbd=*) in slirp networking
>  
>  NOTE: The object files are built at the place where configure is launched
>  EOF
> @@ -5252,6 +5256,19 @@ case "$slirp" in
>  ;;
>  esac
>  
> +# Check for slirp smbd dupport
> +: ${smbd=${SMBD-/usr/sbin/smbd}}
> +if test "$slirp_smbd" != "no" ; then
> +  if test "$mingw32" = "yes" ; then
> +if test "$slirp_smbd" = "yes" ; then
> +  error_exit "Host smbd not supported on this platform."
> +fi
> +slirp_smbd=no
> +  else
> +slirp_smbd=yes
> +  fi
> +fi
> +
>  ##
>  # check for usable __NR_keyctl syscall
>  
> @@ -5527,7 +5544,10 @@ fi
>  if test "$guest_agent" = "yes" ; then
>echo "CONFIG_GUEST_AGENT=y" >> $config_host_mak
>  fi
> -echo "CONFIG_SMBD_COMMAND=\"$smbd\"" >> $config_host_mak
> +if test "$slirp_smbd" = "yes" ; then
> +  echo "CONFIG_SLIRP_SMBD=y" >> $config_host_mak
> +  echo "CONFIG_SMBD_COMMAND=\"$smbd\"" >> $config_host_mak
> +fi
>  if test "$vde" = "yes" ; then
>echo "CONFIG_VDE=y" >> $config_host_mak
>echo "VDE_LIBS=$vde_libs" >> $config_host_mak
> diff --git a/meson.build b/meson.build
> index ba0db9fa1f..773ce512c7 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -2629,6 +2629,9 @@ summary_info += {'pixman':pixman.found()}
>  summary_info += {'VTE support':   config_host.has_key('CONFIG_VTE')}
>  # TODO: add back version
>  summary_info += {'slirp support': slirp_opt == 'disabled' ? false : 
> slirp_opt}
> +if slirp_opt != 'disabled' and 'CONFIG_SLIRP_SMBD' in config_host
> +  summary_info += {'smbd':config_host['CONFIG_SMBD_COMMAND']}
> +endif
>  summary_info += {'libtasn1':  config_host.has_key('CONFIG_TASN1')}
>  summary_info += {'PAM':   config_host.has_key('CONFIG_AUTH_PAM')}
>  summary_info += {'iconv support': iconv.found()}
> diff --git a/net/slirp.c b/net/slirp.c
> index be914c0be0..b3ded2aac1 100644
> --- a/net/slirp.c
> +++ b/net/slirp.c
> @@ -27,7 +27,7 @@
>  #include "net/slirp.h"
>  
>  
> -#ifndef _WIN32
> +#if defined(CONFIG_SLIRP_SMBD)
>  #include 
>  #include 
>  #endif
> @@ -90,7 +90,7 @@ typedef struct SlirpState {
>  Slirp *slirp;
>  Notifier poll_notifier;
>  Notifier exit_notifier;
> -#ifndef _WIN32
> +#if defined(CONFIG_SLIRP_SMBD)
>  gchar *smb_dir;
>  #endif
>  GSList *fwd;
> @@ -103,7 +103,7 @@ static QTAILQ_HEAD(, SlirpState) slirp_stacks =
>  static int slirp_hostfwd(SlirpState *s, const char *redir_str, Error **errp);
>  static int slirp_guestfwd(SlirpState *s, const char *config_str, Error 
> **errp);
>  
> -#ifndef _WIN32
> +#if defined(CONFIG_SLIRP_SMBD)
>  static int slirp_smb(SlirpState *s, const char *exported_dir,
>   struct in_addr vserver_addr, Error **errp);
&

Re: [PATCH v4 2/4] util/qemu-sockets.c: Split host:port parsing out of inet_parse

2021-03-06 Thread Samuel Thibault
Hello,

Doug Evans, le ven. 05 mars 2021 17:00:13 -0800, a ecrit:
> Is it possible for QEMU to lazily determine the guest's IPv6
> address? I.e., postpone the ""->guest address mapping until it's
> needed and then, say, take the first entry in the NDP table?

That would probably be possible, yes, by moving the 

if (!guest_addr.s_addr) {
guest_addr = slirp->vdhcp_startaddr;
}

from slirp_add_hostfwd() and alike to tcp_connect() and sorecvfrom()
(along the other sotranslate call).

> That feels a bit fragile: what if someone else gets the first entry in
> the NDP table? But is that any more fragile than assuming the first
> handed out DHCP address is to the guest?

I don't think it's really more fragile.

> [<<-- Honest question, can we assume the first handed out DHCP address
> will necessarily be the guest?]

It "cannot" be anything else. What could happen is a PXE loader that
uses DHCP/NDP, and then the OS that does it again.

> But that would mean the defaults for the guest would have to be
> different than for the host. E.g.,
> host: ",ipv4" means both,

Why would it mean both? I don't follow you here.

> whereas guest: ",ipv4" (ideally) means ipv4 (since both is meaningless)

Samuel



Re: [PATCH v4 2/4] util/qemu-sockets.c: Split host:port parsing out of inet_parse

2021-03-05 Thread Samuel Thibault
Doug Evans, le ven. 05 mars 2021 16:05:05 -0800, a ecrit:
> Given that the code is not supposed to be able to know brackets were present
> (they're stripped off early on), what does the above mean w.r.t. the guest?
> For the host we can have "" mean listen on both IPv4 and IPv6
> (by default, absent extra options like ipv4=off).
> But what does a guest address of "" mean?
> IPv4? IPv6? Both?

It cannot really be "both" since it'd need to know.

The 0.0.0.0 address used to mean the dhcp-provided address, and we don't
have a way to know the ipv6 address used with the stateless selection,
so I would say that empty address would be just the dhcp-provided
address. Most probably the guest will pick it up anyway, and guests
usually listen the same on ipv4 and ipv6, so I'd say empty most probably
means the user wants to just connect to ipv4 (whatever protocol was used
to connect to the host).

Samuel



Re: [PATCH v4 2/4] util/qemu-sockets.c: Split host:port parsing out of inet_parse

2021-03-05 Thread Samuel Thibault
Daniel P. Berrangé, le mer. 03 mars 2021 18:11:41 +, a ecrit:
> On Wed, Mar 03, 2021 at 10:06:50AM -0800, Doug Evans wrote:
> > On Sun, Feb 28, 2021 at 1:40 PM Samuel Thibault 
> > wrote:
> > 
> > > > +  Examples:
> > > > +  hostfwd_add net0 tcp:127.0.0.1:10022-:22
> > > > +  hostfwd_add net0 tcp:[::1]:10022-[fe80::1:2:3:4]:22
> > >
> > > Yep, that looks good to me.
> > 
> > Daniel, you wanted me to use inet_parse().
> > Is the above syntax ok with you?
> > You must have had some expectation that at least some of
> > the various flags that inet_parse() recognizes would be needed here.
> 
> It feels like the ,ipv4=on|off,ipv6=on|off flags are relevant here,
> especially in the empty address case. eg
> 
>tcp::10022  - attempt to listen on both ipv4 + ipv6
>tcp::10022,ipv4=off - listen on default address, but only for ipv6
>tcp::10022,ipv6=off - listen on default address, but only for ipv4
> 
> Basically this ends up bringing the hostfwd stuff into alignment with
> the way other backends deal with this

I'm fine with this yes, better have a coherent user interface.

Samuel



[Bug 1917442] Re: [AHCI] crash when running a GNU/Hurd guest

2021-03-02 Thread Samuel thibault
Note: this is using the rump ahci driver.

-- 
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1917442

Title:
  [AHCI] crash when running a GNU/Hurd guest

Status in QEMU:
  New

Bug description:
  QEMU git hash = 51db2d7cf2

  Running guest OS using:

  $ gdb --args /extra/qemu/bin/qemu-system-i386 -M q35,accel=kvm -m 4096
  -net user,hostfwd=tcp::-:22 -net nic -drive
  id=udisk,file=/dev/sdd,format=raw,if=none -device ide-
  drive,drive=udisk,bootindex=1 -curses

  ...

  root@zamhurd:~# .ahcisata0 channel 5: setting WDCTL_RST failed for
  drive 0

  
  Thread 1 "qemu-system-i38" received signal SIGSEGV, Segmentation fault.
 
[Switching to Thread 0x74f7bf00 (LWP 590666)]
  ahci_commit_buf (dma=0x57335870, tx_bytes=2048) at ../hw/ide/ahci.c:1462
  1462tx_bytes += le32_to_cpu(ad->cur_cmd->status);
  (gdb) bt full
  #0  ahci_commit_buf (dma=0x57335870, tx_bytes=2048)
  at ../hw/ide/ahci.c:1462
  ad = 0x57335870
  #1  0x55893171 in dma_buf_commit (s=0x57335930, tx_bytes=2048)
  at ../hw/ide/core.c:805
  #2  0x558934f8 in ide_dma_cb (opaque=0x57335930, ret=0)
  at ../hw/ide/core.c:887
  s = 0x57335930
  n = 4
  sector_num = 4491160
  offset = 140732794753312
  stay_active = false
  prep_size = 0
  __PRETTY_FUNCTION__ = "ide_dma_cb"
  #3  0x55830720 in dma_complete (dbs=0x7ffee83d5120, ret=0)
  at ../softmmu/dma-helpers.c:121
  __PRETTY_FUNCTION__ = "dma_complete"
  #4  0x558307cd in dma_blk_cb (opaque=0x7ffee83d5120, ret=0)
  at ../softmmu/dma-helpers.c:139
  dbs = 0x7ffee83d5120
  cur_addr = 140732794753408
  cur_len = 93825013280880
  mem = 0x7ffeeccfef00
  __PRETTY_FUNCTION__ = "dma_blk_cb"
  #5  0x55d92bce in blk_aio_complete (acb=0x7ffee847bbe0)
  at ../block/block-backend.c:1412
  #6  0x55d92df0 in blk_aio_read_entry (opaque=0x7ffee847bbe0)
  at ../block/block-backend.c:1466
  acb = 0x7ffee847bbe0
  rwco = 0x7ffee847bc08
  qiov = 0x7ffee83d5180
  __PRETTY_FUNCTION__ = "blk_aio_read_entry"
  #7  0x55e85580 in coroutine_trampoline (i0=-398117056, i1=32766)
  at ../util/coroutine-ucontext.c:173
  arg = {p = 0x7ffee8453740, i = {-398117056, 32766}}
  self = 0x7ffee8453740
  co = 0x7ffee8453740
  fake_stack_save = 0x0
  #8  0x76544020 in __start_context () at /lib64/libc.so.6
  #9  0x7ffeefdfd680 in  ()
  #10 0x in  ()
  (gdb)
  (gdb) l
  1457   */
  1458  static void ahci_commit_buf(const IDEDMA *dma, uint32_t tx_bytes)
  1459  {
  1460  AHCIDevice *ad = DO_UPCAST(AHCIDevice, dma, dma);
  1461  
  1462  tx_bytes += le32_to_cpu(ad->cur_cmd->status);
  1463  ad->cur_cmd->status = cpu_to_le32(tx_bytes);
  1464  }
  1465  
  1466  static int ahci_dma_rw_buf(const IDEDMA *dma, bool is_write)
  (gdb) p ad
  $1 = (AHCIDevice *) 0x57335870
  (gdb) p ad->cur_cmd
  $2 = (AHCICmdHdr *) 0x0
  (gdb)

To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/1917442/+subscriptions



Re: [PATCH v4 2/4] util/qemu-sockets.c: Split host:port parsing out of inet_parse

2021-03-01 Thread Samuel Thibault
Samuel Thibault, le lun. 01 mars 2021 17:27:47 +0100, a ecrit:
> Doug Evans, le lun. 01 mars 2021 08:23:03 -0800, a ecrit:
> > On Sun, Feb 28, 2021 at 1:40 PM Samuel Thibault <[1]samuel.thiba...@gnu.org>
> > wrote:
> > 
> > [...]
> > > Note that one issue I am leaving for later (i.e., I don't want to drag
> > this
> > > patch series out to include it), is whether and how to support
> > ipv4-host->
> > > ipv6-guest forwarding and vice versa. Can libslirp support this?
> > 
> > That would be feasible yes: since the data flow is completely rebuilt
> > between the host and the guest, there is no remnant of the IP version.
> > It was simpler to have e.g. udp_listen and udp6_listen separate to keep
> > uint32_t / in6_addr parameters, but there is no strict reason for this:
> > the haddr is only passed to the bind() call, and the laddr is only
> > recorded in the so. Put another way, a refactoring patch could be to
> > just hand udp_listen two sockaddrs, and it will just work fine. We'd
> > then introduce a slirp_add_hostfwd that takes two sockaddr instead of
> > host/port.
> > 
> > 
> > 
> > I guess I'm not familiar enough with this code.
> > Help me understand how passing two addresses to udp_listen is simpler.
> > That feels confusing from an API viewpoint.
> 
> ? udp_listen already takes two addresses + two ports. I just mean
> replacing that with two sockaddr, which contains both, but also contains
> the address family. I submitted this to 
> 
> https://gitlab.freedesktop.org/slirp/libslirp/-/merge_requests/74

And the public API to 
https://gitlab.freedesktop.org/slirp/libslirp/-/merge_requests/75

Ideally we'd then just drop the ipv6-only public API.

Samuel



Re: [PATCH v4 2/4] util/qemu-sockets.c: Split host:port parsing out of inet_parse

2021-03-01 Thread Samuel Thibault
Samuel Thibault, le lun. 01 mars 2021 17:26:23 +0100, a ecrit:
> We could make libslirp enable the IPV6ONLY flag to avoid that, and
> make qemu pass an AF_UNSPEC address for the ipv4+ipv6 case, in which
> case libslirp wouldn't set IPV6ONLY.

Ah, no, AF_UNSPEC would not allow to specify the port. But we can use a
flag in the slirp_add_hostxfwd call.

Samuel



Re: [PATCH v4 2/4] util/qemu-sockets.c: Split host:port parsing out of inet_parse

2021-03-01 Thread Samuel Thibault
Doug Evans, le lun. 01 mars 2021 08:23:03 -0800, a ecrit:
> On Sun, Feb 28, 2021 at 1:40 PM Samuel Thibault <[1]samuel.thiba...@gnu.org>
> wrote:
> 
> [...]
> > Note that one issue I am leaving for later (i.e., I don't want to drag
> this
> > patch series out to include it), is whether and how to support
> ipv4-host->
> > ipv6-guest forwarding and vice versa. Can libslirp support this?
> 
> That would be feasible yes: since the data flow is completely rebuilt
> between the host and the guest, there is no remnant of the IP version.
> It was simpler to have e.g. udp_listen and udp6_listen separate to keep
> uint32_t / in6_addr parameters, but there is no strict reason for this:
> the haddr is only passed to the bind() call, and the laddr is only
> recorded in the so. Put another way, a refactoring patch could be to
> just hand udp_listen two sockaddrs, and it will just work fine. We'd
> then introduce a slirp_add_hostfwd that takes two sockaddr instead of
> host/port.
> 
> 
> 
> I guess I'm not familiar enough with this code.
> Help me understand how passing two addresses to udp_listen is simpler.
> That feels confusing from an API viewpoint.

? udp_listen already takes two addresses + two ports. I just mean
replacing that with two sockaddr, which contains both, but also contains
the address family. I submitted this to 

https://gitlab.freedesktop.org/slirp/libslirp/-/merge_requests/74

Samuel



Re: [PATCH v4 2/4] util/qemu-sockets.c: Split host:port parsing out of inet_parse

2021-03-01 Thread Samuel Thibault
Doug Evans, le lun. 01 mars 2021 08:07:19 -0800, a ecrit:
> Are there any users that this functional change would break?
> [Previously the empty address meant qemu would only listen on ipv4 addr-any.]

One case that could be broken would be a user having already another
service listening on ipv6-only along qemu listening on ipv4-only. But I
find this very little probable.

> What if a user wants only ipv4 addr-any (or only ipv6 addr-any) ?

"0.0.0.0" would get ipv4 addr-any.

Without anything done in particular, "::" would get both ipv6 and
ipv4. We could make libslirp enable the IPV6ONLY flag to avoid that, and
make qemu pass an AF_UNSPEC address for the ipv4+ipv6 case, in which
case libslirp wouldn't set IPV6ONLY.

make that ipv6-only through a flag passed to 

> What does hostfwd "::12345-6.7.8.9:10" mean?
> Does the presence of the empty host address mean forward both ipv4 and ipv6 to
> guest ipv4 6.7.8.9?

I'd say so, yes.

Samuel



Re: [PATCH v4 2/4] util/qemu-sockets.c: Split host:port parsing out of inet_parse

2021-03-01 Thread Samuel Thibault
Markus Armbruster, le lun. 01 mars 2021 09:15:41 +0100, a ecrit:
> Samuel Thibault  writes:
> > Specifying [127.0.0.1] would be odd, but for instance 
> >
> > ssh localhost -D '[127.0.0.1]':23456
> >
> > happens to listen on 127.0.0.1. So I would say that common practice
> > really is that [] only matters for syntax, and not semantic.
> 
> I believe common syntactic practice is to use [brackets] only around
> numeric IPv6 addresses.  E.g. socat(1):
> 
>IP address
>   An IPv4 address in numbers-and-dots notation, an IPv6 address in
>   hex notation enclosed in brackets, or a hostname  that  resolves
>   to an IPv4 or an IPv6 address.
>   Examples: 127.0.0.1, [::1], www.dest-unreach.org, dns1

Yes and that's also what ssh documents, but in ssh the brackets happen
to also work for an IPv4 address.

Samuel



Re: [PATCH v2 1/2] net/slirp.c: Refactor address parsing

2021-02-28 Thread Samuel Thibault
Hello,

Doug Evans, le lun. 08 févr. 2021 10:59:01 -0800, a ecrit:
> Samuel, how do qemu patches involving libslirp changes usually work?

Well, we haven't had many yet actually :)

> Should I have held off submitting the qemu patch until the libslirp
> prerequisite has been added to qemu's tree,

No, as this example shows, there are iterations that can happen on the
qemu side before we have something we can commit, so better do both in
parallel.

I don't know what qemu people think about the slirp submodule: do qemu
prefers to only track stable branchs, or would it be fine to track the
master branch? I guess you'd prefer to have a slirp stable release you
can depend on when releasing qemu, so perhaps better wait for a slirp
release before bumping in qemu, just to be safe? Which doesn't mean
avoiding submitting patchqueues that depend on it before that, better go
in parallel.

> or maybe I should include the libslirp patch so that people can at least apply
> it (with a caveat saying the patch is already in libslirp.git) until it's 
> added
> to the qemu tree?

Not sure what is best here. We at least need something so that people
are not confused by the patchqueue calling some function that doesn't
exist in the submodule yet.

Samuel



Re: [PATCH v4 2/4] util/qemu-sockets.c: Split host:port parsing out of inet_parse

2021-02-28 Thread Samuel Thibault
Samuel Thibault, le dim. 28 févr. 2021 22:39:57 +0100, a ecrit:
> It was simpler to have e.g. udp_listen and udp6_listen separate to keep
> uint32_t / in6_addr parameters, but there is no strict reason for this:
> the haddr is only passed to the bind() call, and the laddr is only
> recorded in the so. Put another way, a refactoring patch could be to
> just hand udp_listen two sockaddrs, and it will just work fine.

I have submitted that part to
https://gitlab.freedesktop.org/slirp/libslirp/-/merge_requests/74
Could you review it?

> We'd then introduce a slirp_add_xhostfwd that takes two sockaddr
> instead of host/port.

That should then be easy to introduce indeed, and immediately more
powerful than the slirp_add/remove_ipv6_hostfwd. Possibly you could just
replace in qemu the existing slirp_add/remove_hostfwd call, and thus
make the whole code simpler: ideally it's the address parsing function
that would produce a sockaddr.

Samuel



Re: [PATCH v4 2/4] util/qemu-sockets.c: Split host:port parsing out of inet_parse

2021-02-28 Thread Samuel Thibault
Hello,

Daniel P. Berrangé, le lun. 22 févr. 2021 09:39:41 +, a ecrit:
> In general callers shouldn't care about which format was parsed. The use
> of [] is just a mechanism to reliably separate the port from the address.
> Once you have the address part getaddrinfo() will reliably parse the
> address into a sockaddr struct on its own.

Agreed.

> The is_v6 flag is only needed
> for the legacy compat needs in slirp, even that is only if we want to 
> have strict equivalence with historical behaviour, as opposed to changing
> empty string to mean to listen on both IPv4+6 concurrently..

I would say that empty address meaning ipv4+6 looks better to me.

Doug Evans, le lun. 22 févr. 2021 09:55:09 -0800, a ecrit:
> Hi guys. I think before I submit yet another patchset in this series I need
> someone with authority to define the user API for ipv6 host forwarding.
> Since the hostfwd syntax is parsed in net/slirp.c, Samuel I think that means
> you (based on what I'm reading in MAINTAINERS).

Well, I'm not maintainer of the user API actually. That'd rather be
Markus Armbruster, now Cc-ed, who devises the command-line options,
QAPI, etc.

> Based on what Maxim originally wrote I was going with addresses wrapped in []
> mean ipv6, but Daniel does not want that.

Specifying [127.0.0.1] would be odd, but for instance 

ssh localhost -D '[127.0.0.1]':23456

happens to listen on 127.0.0.1. So I would say that common practice
really is that [] only matters for syntax, and not semantic.

> There are issues to consider of course.
> Note that one issue I am leaving for later (i.e., I don't want to drag this
> patch series out to include it), is whether and how to support ipv4-host->
> ipv6-guest forwarding and vice versa. Can libslirp support this?

That would be feasible yes: since the data flow is completely rebuilt
between the host and the guest, there is no remnant of the IP version.
It was simpler to have e.g. udp_listen and udp6_listen separate to keep
uint32_t / in6_addr parameters, but there is no strict reason for this:
the haddr is only passed to the bind() call, and the laddr is only
recorded in the so. Put another way, a refactoring patch could be to
just hand udp_listen two sockaddrs, and it will just work fine. We'd
then introduce a slirp_add_hostfwd that takes two sockaddr instead of
host/port.

> Setting cross-forwarding aside, we can't break existing uses of course, so 
> that
> means that one issue is that if [] is not used to identify ipv6 addresses then
> something like ",ipv6" will be needed as a separate argument; otherwise the
> empty address "" will be ambiguous.

(as I mentioned above, I'd say empty address "" should mean ipv4+ipv6)

> +  Examples:
> +  hostfwd_add net0 tcp:127.0.0.1:10022-:22
> +  hostfwd_add net0 tcp:[::1]:10022-[fe80::1:2:3:4]:22

Yep, that looks good to me.

Samuel



Re: [PATCH v2 2/2] net: Add -ipv6-hostfwd option, ipv6_hostfwd_add/remove commands

2021-02-03 Thread Samuel Thibault
Doug Evans, le mer. 03 févr. 2021 13:37:29 -0800, a ecrit:
> @@ -1392,6 +1392,34 @@ SRST
>Remove host-to-guest TCP or UDP redirection.
>  ERST
>  
> +#ifdef CONFIG_SLIRP
> +{
> +.name   = "ipv6_hostfwd_add",
> +.args_type  = "arg1:s,arg2:s?",
> +.params = "[netdev_id] 
> [tcp|udp]:[hostaddr6]:hostport-[guestaddr6]:guestport",

Perhaps explicit that the IPv6 address should be enclosed with [] ?

> +/* Ignore the part between the ']' and addr_sep. */
> +if (get_str_sep(buf, sizeof(buf), , addr_sep) < 0) {

Mmm, I would say that we do not want to just ignore it, and rather make
sure that it is empty, so that we can possibly make extensions later
without breaking existing misuse.

Samuel



Re: [PATCH v2 1/2] net/slirp.c: Refactor address parsing

2021-02-03 Thread Samuel Thibault
Doug Evans, le mer. 03 févr. 2021 13:37:28 -0800, a ecrit:
> ... in preparation for adding ipv6 host forwarding support.

Reviewed-by: Samuel Thibault 

except

> diff --git a/slirp b/slirp
> index 8f43a99191..358c0827d4 16
> --- a/slirp
> +++ b/slirp
> @@ -1 +1 @@
> -Subproject commit 8f43a99191afb47ca3f3c6972f6306209f367ece
> +Subproject commit 358c0827d49778f016312bfb4167fe639900681f

Which, I would say, deserves its own commit?



Re: [RFC] Change default ipv6 network from fec0/10 (site local) to fe80/10 (link local)

2021-01-27 Thread Samuel Thibault
Hello,

Philippe Mathieu-Daudé, le mer. 27 janv. 2021 22:46:13 +0100, a ecrit:
> On 1/27/21 8:13 PM, Doug Evans wrote:
> > I happened to notice QEMU's default for the ipv6 network is fec0::/10
> > which is deprecated (RFC3879).
> > I think(!) an obvious replacement is fe80::/10, link local.

fe80::/10 is really a different thing, I don't think we want to use it.

We can use some prefix in fc00::/7, such as fd00::/8.
It "just" needs checking with various guest OS, to check that it doesn't
break the default behavior.

Samuel



Re: [PATCH] net/slirp.c: Fix spelling error in error message

2021-01-21 Thread Samuel Thibault
Doug Evans, le jeu. 21 janv. 2021 16:42:51 -0800, a ecrit:
> DNS should be DHCP
> 
> Signed-off-by: Doug Evans 

Reviewed-by: Samuel Thibault 

> ---
>  net/slirp.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/net/slirp.c b/net/slirp.c
> index 8350c6d45f..be914c0be0 100644
> --- a/net/slirp.c
> +++ b/net/slirp.c
> @@ -473,7 +473,7 @@ static int net_slirp_init(NetClientState *peer, const 
> char *model,
>  return -1;
>  }
>  if (dhcp.s_addr == host.s_addr || dhcp.s_addr == dns.s_addr) {
> -error_setg(errp, "DNS must be different from host and DNS");
> +error_setg(errp, "DHCP must be different from host and DNS");
>  return -1;
>  }
>  
> -- 
> 2.30.0.280.ga3ce27912f-goog
> 



Re: [PATCH v2] meson: fix ncurses detection on macOS

2020-12-30 Thread Samuel Thibault
Chris Hofstaedtler, le mer. 30 déc. 2020 23:17:27 +0100, a ecrit:
> Without this, meson fails with "curses package not usable" when using ncurses
> 6.2. Apparently the wide functions (addwstr, etc) are hidden behind the extra
> define, and meson does not define it at that detection stage.
> 
> Regression from b01a4fd3bd7d6f2 ("configure: Define NCURSES_WIDECHAR if we're
> using curses"). The meson conversion has seen many iterations of the curses
> check, so pinpointing the exact commit breaking this is not so easy.
> 
> Signed-off-by: Chris Hofstaedtler 
> Cc: Peter Maydell 
> Cc: Philippe Mathieu-Daudé 
> Cc: Samuel Thibault 
> Cc: Yonggang Luo 

Reviewed-by: Samuel Thibault 

> ---
>  meson.build | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/meson.build b/meson.build
> index 372576f82c..fd74728674 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -500,16 +500,16 @@ if have_system and not get_option('curses').disabled()
>  endif
>endforeach
>msg = get_option('curses').enabled() ? 'curses library not found' : ''
> +  curses_compile_args = ['-DNCURSES_WIDECHAR']
>if curses.found()
> -if cc.links(curses_test, dependencies: [curses])
> -  curses = declare_dependency(compile_args: '-DNCURSES_WIDECHAR', 
> dependencies: [curses])
> +if cc.links(curses_test, args: curses_compile_args, dependencies: 
> [curses])
> +  curses = declare_dependency(compile_args: curses_compile_args, 
> dependencies: [curses])
>  else
>msg = 'curses package not usable'
>curses = not_found
>  endif
>endif
>if not curses.found()
> -curses_compile_args = ['-DNCURSES_WIDECHAR']
>  has_curses_h = cc.has_header('curses.h', args: curses_compile_args)
>  if targetos != 'windows' and not has_curses_h
>message('Trying with /usr/include/ncursesw')
> -- 
> 2.29.2
> 



Re: [PATCH] meson: fix ncurses detection on macOS

2020-12-28 Thread Samuel Thibault
Philippe Mathieu-Daudé, le lun. 28 déc. 2020 18:20:13 +0100, a ecrit:
> On 12/28/20 4:16 PM, Chris Hofstaedtler wrote:
> > Without this, meson fails with "curses package not usable"
> > when using ncurses 6.2. Apparently the wide functions
> > (addwstr, etc) are hidden behind the extra define, and
> > meson does not define it at that detection stage.
> 
> Seems reasonable, but still Cc'ing more developers.

That looks sensible indeed.


> > Signed-off-by: Chris Hofstaedtler 
> > ---
> >  meson.build | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/meson.build b/meson.build
> > index 9c152a85bd..7b9d92c14a 100644
> > --- a/meson.build
> > +++ b/meson.build
> > @@ -510,7 +510,7 @@ if have_system and not get_option('curses').disabled()
> >endforeach
> >msg = get_option('curses').enabled() ? 'curses library not found' : ''
> >if curses.found()
> > -if cc.links(curses_test, dependencies: [curses])
> > +if cc.links(curses_test, args: '-DNCURSES_WIDECHAR', dependencies: 
> > [curses])
> >curses = declare_dependency(compile_args: '-DNCURSES_WIDECHAR', 
> > dependencies: [curses])
> >  else
> >msg = 'curses package not usable'



Re: Does QEMU's coverity-scan run need to track coverity issues in dtb or slirp ?

2020-11-02 Thread Samuel Thibault
Hello,

Peter Maydell, le lun. 02 nov. 2020 19:54:14 +, a ecrit:
> Do dtc and slirp as upstream projects already track Coverity issues

We started tracking them yes.

Samuel



Re: [PATCH 8/9] dev-serial: fix FTDI_GET_MDM_ST response

2020-10-26 Thread Samuel Thibault
Mark Cave-Ayland, le lun. 26 oct. 2020 13:40:05 +, a ecrit:
> On 26/10/2020 13:00, Jason Andryuk wrote:
> > On Mon, Oct 26, 2020 at 7:21 AM Samuel Thibault  
> > wrote:
> > > Aurelien, you introduced the "| 1" in
> > > 
> > > commit abb8a13918ecc1e8160aa78582de9d5224ea70df
> > > Author: Aurelien Jarno 
> > > Date:   Wed Aug 13 04:23:17 2008 +
> > > 
> > >  usb-serial: add support for modem lines
> > > 
> > > [...]
> > > @@ -357,9 +393,9 @@ static int usb_serial_handle_control(USBDevice *dev, 
> > > int request, int value,
> > >   /* TODO: TX ON/OFF */
> > >   break;
> > >   case DeviceInVendor | FTDI_GET_MDM_ST:
> > > -/* TODO: return modem status */
> > > -data[0] = 0;
> > > -ret = 1;
> > > +data[0] = usb_get_modem_lines(s) | 1;
> > > +data[1] = 0;
> > > +ret = 2;
> > >   break;
> > 
> > I'm not particularly familiar with the FTDI USB serial devices.  I
> > found setting FTDI_THRE | FTDI_TEMT by comparing with real hardware.
> > 
> > A little searching found this:
> > https://elixir.bootlin.com/linux/latest/source/drivers/usb/serial/ftdi_sio.h#L541
> > 
> > That shows "B0   Reserved - must be 1", so maybe that is why "| 1" was 
> > added?
> 
> Right - that's for the modem status returned as part of the first 2 status
> bytes for incoming data which is slightly different from modem status
> returned directly from FTDI_SIO_GET_MODEM_STATUS: 
> https://elixir.bootlin.com/linux/latest/source/drivers/usb/serial/ftdi_sio.h#L423.
> 
> It is the latter which this patch changes and appears to match what I see on
> my Chipi-X hardware here.

Aurelien, do you remember the reason for the addition of 1 here? It does
look like the confusion between the incoming data bytes and the modem
status bytes.

Samuel



Re: [PATCH 8/9] dev-serial: fix FTDI_GET_MDM_ST response

2020-10-26 Thread Samuel Thibault
Mark Cave-Ayland, le lun. 26 oct. 2020 10:58:43 +, a ecrit:
> On 26/10/2020 09:54, Samuel Thibault wrote:
> > Mark Cave-Ayland, le lun. 26 oct. 2020 08:34:00 +, a ecrit:
> > > The FTDI_GET_MDM_ST response should only return a single byte indicating 
> > > the
> > > modem status with bit 0 cleared (as documented in the Linux ftdi_sio.h 
> > > header
> > > file).
> > > 
> > > Signed-off-by: Mark Cave-Ayland 
> > > ---
> > >   hw/usb/dev-serial.c | 5 ++---
> > >   1 file changed, 2 insertions(+), 3 deletions(-)
> > > 
> > > diff --git a/hw/usb/dev-serial.c b/hw/usb/dev-serial.c
> > > index 4c374d0790..fa734bcf54 100644
> > > --- a/hw/usb/dev-serial.c
> > > +++ b/hw/usb/dev-serial.c
> > > @@ -360,9 +360,8 @@ static void usb_serial_handle_control(USBDevice *dev, 
> > > USBPacket *p,
> > >   /* TODO: TX ON/OFF */
> > >   break;
> > >   case VendorDeviceRequest | FTDI_GET_MDM_ST:
> > > -data[0] = usb_get_modem_lines(s) | 1;
> > > -data[1] = FTDI_THRE | FTDI_TEMT;
> > > -p->actual_length = 2;
> > > +data[0] = usb_get_modem_lines(s);
> > > +p->actual_length = 1;
> > 
[...]
> A quick test shows my Chipi-X returns 0x1 0x60 with nothing attached in
> response to FTDI_SIO_GET_MODEM_STATUS_REQUEST: assuming the reply length
> should be 2 bytes, the comment about B0-B3 being zero and the response from
> my Chip-X above suggests that the "| 1" should still be dropped from the
> response.

Aurelien, you introduced the "| 1" in 

commit abb8a13918ecc1e8160aa78582de9d5224ea70df
Author: Aurelien Jarno 
Date:   Wed Aug 13 04:23:17 2008 +

usb-serial: add support for modem lines

[...]
@@ -357,9 +393,9 @@ static int usb_serial_handle_control(USBDevice *dev, int 
request, int value,
 /* TODO: TX ON/OFF */
 break;
 case DeviceInVendor | FTDI_GET_MDM_ST:
-/* TODO: return modem status */
-data[0] = 0;
-ret = 1;
+data[0] = usb_get_modem_lines(s) | 1;
+data[1] = 0;
+ret = 2;
 break;

do you know exactly what it is for?

Samuel



Re: [PATCH 0/9] dev-serial: minor fixes and improvements

2020-10-26 Thread Samuel Thibault
Hello,

Mark Cave-Ayland, le lun. 26 oct. 2020 08:33:52 +, a ecrit:
> This series comes from a client project that I have been working on over the
> past few months which involves communicating with serial hardware and
> associated simulators using the QEMU USB serial device.

Thanks for these!

I only had concerned with the MDM_ST change, see the corresponding mail.

The other patches can be applied independently from that change.

Samuel



Re: [PATCH 9/9] dev-serial: store flow control and xon/xoff characters

2020-10-26 Thread Samuel Thibault
Mark Cave-Ayland, le lun. 26 oct. 2020 08:34:01 +, a ecrit:
> Note that whilst the device does not do anything with these values, they are
> logged with trace events and stored to allow future implementation.
> 
> The default flow control is set to none at reset as documented in the Linux
> ftdi_sio.h header file.
> 
> Signed-off-by: Mark Cave-Ayland 

Reviewed-by: Samuel Thibault 

> ---
>  hw/usb/dev-serial.c | 38 +++---
>  hw/usb/trace-events |  2 ++
>  2 files changed, 37 insertions(+), 3 deletions(-)
> 
> diff --git a/hw/usb/dev-serial.c b/hw/usb/dev-serial.c
> index fa734bcf54..3dbc56d77a 100644
> --- a/hw/usb/dev-serial.c
> +++ b/hw/usb/dev-serial.c
> @@ -52,6 +52,7 @@
>  
>  /* SET_FLOW_CTRL */
>  
> +#define FTDI_NO_HS 0
>  #define FTDI_RTS_CTS_HS1
>  #define FTDI_DTR_DSR_HS2
>  #define FTDI_XON_XOFF_HS   4
> @@ -98,6 +99,9 @@ struct USBSerialState {
>  uint8_t error_chr;
>  uint8_t event_trigger;
>  bool always_plugged;
> +uint8_t flow_control;
> +uint8_t xon;
> +uint8_t xoff;
>  QEMUSerialSetParams params;
>  int latency;/* ms */
>  CharBackend cs;
> @@ -181,14 +185,36 @@ static const USBDesc desc_braille = {
>  .str  = desc_strings,
>  };
>  
> +static void usb_serial_set_flow_control(USBSerialState *s,
> +uint8_t flow_control)
> +{
> +USBDevice *dev = USB_DEVICE(s);
> +USBBus *bus = usb_bus_from_device(dev);
> +
> +/* TODO: ioctl */
> +s->flow_control = flow_control;
> +trace_usb_serial_set_flow_control(bus->busnr, dev->addr, flow_control);
> +}
> +
> +static void usb_serial_set_xonxoff(USBSerialState *s, int xonxoff)
> +{
> +USBDevice *dev = USB_DEVICE(s);
> +USBBus *bus = usb_bus_from_device(dev);
> +
> +s->xon = xonxoff & 0xff;
> +s->xoff = (xonxoff >> 8) & 0xff;
> +
> +trace_usb_serial_set_xonxoff(bus->busnr, dev->addr, s->xon, s->xoff);
> +}
> +
>  static void usb_serial_reset(USBSerialState *s)
>  {
> -/* TODO: Set flow control to none */
>  s->event_chr = 0x0d;
>  s->event_trigger = 0;
>  s->recv_ptr = 0;
>  s->recv_used = 0;
>  /* TODO: purge in char driver */
> +usb_serial_set_flow_control(s, FTDI_NO_HS);
>  }
>  
>  static void usb_serial_handle_reset(USBDevice *dev)
> @@ -285,9 +311,15 @@ static void usb_serial_handle_control(USBDevice *dev, 
> USBPacket *p,
>  qemu_chr_fe_ioctl(>cs, CHR_IOCTL_SERIAL_SET_TIOCM, );
>  break;
>  }
> -case VendorDeviceOutRequest | FTDI_SET_FLOW_CTRL:
> -/* TODO: ioctl */
> +case VendorDeviceOutRequest | FTDI_SET_FLOW_CTRL: {
> +uint8_t flow_control = index >> 8;
> +
> +usb_serial_set_flow_control(s, flow_control);
> +if (flow_control & FTDI_XON_XOFF_HS) {
> +usb_serial_set_xonxoff(s, value);
> +}
>  break;
> +}
>  case VendorDeviceOutRequest | FTDI_SET_BAUD: {
>  static const int subdivisors8[8] = { 0, 4, 2, 1, 3, 5, 6, 7 };
>  int subdivisor8 = subdivisors8[((value & 0xc000) >> 14)
> diff --git a/hw/usb/trace-events b/hw/usb/trace-events
> index 0d0a3e5f2a..725b7077ad 100644
> --- a/hw/usb/trace-events
> +++ b/hw/usb/trace-events
> @@ -331,3 +331,5 @@ usb_serial_unsupported_data_bits(int bus, int addr, int 
> value) "dev %d:%d unsupp
>  usb_serial_bad_token(int bus, int addr) "dev %d:%d bad token"
>  usb_serial_set_baud(int bus, int addr, int baud) "dev %d:%d baud rate %d"
>  usb_serial_set_data(int bus, int addr, int parity, int data, int stop) "dev 
> %d:%d parity %c, data bits %d, stop bits %d"
> +usb_serial_set_flow_control(int bus, int addr, int index) "dev %d:%d flow 
> control %d"
> +usb_serial_set_xonxoff(int bus, int addr, uint8_t xon, uint8_t xoff) "dev 
> %d:%d xon 0x%x xoff 0x%x"
> -- 
> 2.20.1
> 

-- 
Samuel
>   dvips -o $@ $< 
Faut faire gffe de pas te couper avec ton truc, t'as mis des ciseaux ($<)
partout :))
-+- Dom in Guide du linuxien pervers - "J'aime pas les Makefile !" -+-



Re: [PATCH 8/9] dev-serial: fix FTDI_GET_MDM_ST response

2020-10-26 Thread Samuel Thibault
Hello,

(Cc-ing Aurelien who introduced the support for modem control, and Jason
who added the missing THRE and TEMT flags).

Mark Cave-Ayland, le lun. 26 oct. 2020 08:34:00 +, a ecrit:
> The FTDI_GET_MDM_ST response should only return a single byte indicating the
> modem status with bit 0 cleared (as documented in the Linux ftdi_sio.h header
> file).
> 
> Signed-off-by: Mark Cave-Ayland 
> ---
>  hw/usb/dev-serial.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/hw/usb/dev-serial.c b/hw/usb/dev-serial.c
> index 4c374d0790..fa734bcf54 100644
> --- a/hw/usb/dev-serial.c
> +++ b/hw/usb/dev-serial.c
> @@ -360,9 +360,8 @@ static void usb_serial_handle_control(USBDevice *dev, 
> USBPacket *p,
>  /* TODO: TX ON/OFF */
>  break;
>  case VendorDeviceRequest | FTDI_GET_MDM_ST:
> -data[0] = usb_get_modem_lines(s) | 1;
> -data[1] = FTDI_THRE | FTDI_TEMT;
> -p->actual_length = 2;
> +data[0] = usb_get_modem_lines(s);
> +p->actual_length = 1;

Err, but Linux' drivers/usb/serial/ftdi_sio.c:ftdi_process_packet()
contradicts this: 

if (len < 2) {
dev_dbg(>dev, "malformed packet\n");
return 0;
}

status = buf[0] & FTDI_STATUS_B0_MASK;
if (status != priv->prev_status) {
char diff_status = status ^ priv->prev_status;

if (diff_status & FTDI_RS0_CTS)
port->icount.cts++;

[...]

/* save if the transmitter is empty or not */
if (buf[1] & FTDI_RS_TEMT)
priv->transmit_empty = 1;
else
priv->transmit_empty = 0;

Did you actually get an issue with seeing this packet contain two bytes?

Samuel



Re: [PATCH 7/9] dev-serial: add support for setting data_bits in QEMUSerialSetParams

2020-10-26 Thread Samuel Thibault
Mark Cave-Ayland, le lun. 26 oct. 2020 08:33:59 +, a ecrit:
> Also implement the behaviour reported in Linux's ftdi_sio.c whereby if an 
> invalid
> data_bits value is provided then the hardware defaults to using 8.
> 
> Signed-off-by: Mark Cave-Ayland 

Reviewed-by: Samuel Thibault 

> ---
>  hw/usb/dev-serial.c | 17 +
>  hw/usb/trace-events |  1 +
>  2 files changed, 18 insertions(+)
> 
> diff --git a/hw/usb/dev-serial.c b/hw/usb/dev-serial.c
> index 919e25e1d9..4c374d0790 100644
> --- a/hw/usb/dev-serial.c
> +++ b/hw/usb/dev-serial.c
> @@ -308,6 +308,23 @@ static void usb_serial_handle_control(USBDevice *dev, 
> USBPacket *p,
>  break;
>  }
>  case VendorDeviceOutRequest | FTDI_SET_DATA:
> +switch (value & 0xff) {
> +case 7:
> +s->params.data_bits = 7;
> +break;
> +case 8:
> +s->params.data_bits = 8;
> +break;
> +default:
> +/*
> + * According to a comment in Linux's ftdi_sio.c original FTDI
> + * chips fall back to 8 data bits for unsupported data_bits
> + */
> +trace_usb_serial_unsupported_data_bits(bus->busnr, dev->addr,
> +   value & 0xff);
> +s->params.data_bits = 8;
> +}
> +
>  switch (value & FTDI_PARITY) {
>  case 0:
>  s->params.parity = 'N';
> diff --git a/hw/usb/trace-events b/hw/usb/trace-events
> index 9e984b2e0c..0d0a3e5f2a 100644
> --- a/hw/usb/trace-events
> +++ b/hw/usb/trace-events
> @@ -327,6 +327,7 @@ usb_serial_handle_control(int bus, int addr, int request, 
> int value) "dev %d:%d
>  usb_serial_unsupported_parity(int bus, int addr, int value) "dev %d:%d 
> unsupported parity %d"
>  usb_serial_unsupported_stopbits(int bus, int addr, int value) "dev %d:%d 
> unsupported stop bits %d"
>  usb_serial_unsupported_control(int bus, int addr, int request, int value) 
> "dev %d:%d got unsupported/bogus control 0x%x, value 0x%x"
> +usb_serial_unsupported_data_bits(int bus, int addr, int value) "dev %d:%d 
> unsupported data bits %d, falling back to 8"
>  usb_serial_bad_token(int bus, int addr) "dev %d:%d bad token"
>  usb_serial_set_baud(int bus, int addr, int baud) "dev %d:%d baud rate %d"
>  usb_serial_set_data(int bus, int addr, int parity, int data, int stop) "dev 
> %d:%d parity %c, data bits %d, stop bits %d"
> -- 
> 2.20.1
> 

-- 
Samuel
 > Il [e2fsck] a bien démarré, mais il m'a rendu la main aussitot en me
 > disant "houlala, c'est pas beau à voir votre truc, je préfèrerai que
 > vous teniez vous même la tronçonneuse" (traduction libre)
 NC in Guide du linuxien pervers : "Bien configurer sa tronçonneuse."



Re: [PATCH 6/9] dev-serial: add always-plugged property to ensure USB device is always attached

2020-10-26 Thread Samuel Thibault
Mark Cave-Ayland, le lun. 26 oct. 2020 08:33:58 +, a ecrit:
> Some operating systems will generate a new device ID when a USB device is 
> unplugged
> and then replugged into the USB. If this is done whilst switching between 
> multiple
> applications over a virtual serial port, the change of device ID requires 
> going
> back into the OS/application to locate the new device accordingly.
> 
> Add a new always-plugged property that if specified will ensure that the 
> device
> always remains attached to the USB regardless of the state of the backend
> chardev.
> 
> Signed-off-by: Mark Cave-Ayland 

Reviewed-by: Samuel Thibault 

> ---
>  hw/usb/dev-serial.c | 9 ++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/hw/usb/dev-serial.c b/hw/usb/dev-serial.c
> index 92c35615eb..919e25e1d9 100644
> --- a/hw/usb/dev-serial.c
> +++ b/hw/usb/dev-serial.c
> @@ -97,6 +97,7 @@ struct USBSerialState {
>  uint8_t event_chr;
>  uint8_t error_chr;
>  uint8_t event_trigger;
> +bool always_plugged;
>  QEMUSerialSetParams params;
>  int latency;/* ms */
>  CharBackend cs;
> @@ -516,12 +517,12 @@ static void usb_serial_event(void *opaque, QEMUChrEvent 
> event)
>  s->event_trigger |= FTDI_BI;
>  break;
>  case CHR_EVENT_OPENED:
> -if (!s->dev.attached) {
> +if (!s->always_plugged && !s->dev.attached) {
>  usb_device_attach(>dev, _abort);
>  }
>  break;
>  case CHR_EVENT_CLOSED:
> -if (s->dev.attached) {
> +if (!s->always_plugged && s->dev.attached) {
>  usb_device_detach(>dev);
>  }
>  break;
> @@ -556,7 +557,8 @@ static void usb_serial_realize(USBDevice *dev, Error 
> **errp)
>   usb_serial_event, NULL, s, NULL, true);
>  usb_serial_handle_reset(dev);
>  
> -if (qemu_chr_fe_backend_open(>cs) && !dev->attached) {
> +if (s->always_plugged || (qemu_chr_fe_backend_open(>cs) &&
> +  !dev->attached)) {
>  usb_device_attach(dev, _abort);
>  }
>  s->intr = usb_ep_get(dev, USB_TOKEN_IN, 1);
> @@ -584,6 +586,7 @@ static const VMStateDescription vmstate_usb_serial = {
>  
>  static Property serial_properties[] = {
>  DEFINE_PROP_CHR("chardev", USBSerialState, cs),
> +DEFINE_PROP_BOOL("always-plugged", USBSerialState, always_plugged, 
> false),
>  DEFINE_PROP_END_OF_LIST(),
>  };
>  
> -- 
> 2.20.1
> 

-- 
Samuel
"...[Linux's] capacity to talk via any medium except smoke signals."
(By Dr. Greg Wettstein, Roger Maris Cancer Center)



Re: [PATCH 4/9] dev-serial: add trace-events for baud rate and data parameters

2020-10-26 Thread Samuel Thibault
Mark Cave-Ayland, le lun. 26 oct. 2020 08:33:56 +, a ecrit:
> Signed-off-by: Mark Cave-Ayland 

Reviewed-by: Samuel Thibault 

> ---
>  hw/usb/dev-serial.c | 3 +++
>  hw/usb/trace-events | 2 ++
>  2 files changed, 5 insertions(+)
> 
> diff --git a/hw/usb/dev-serial.c b/hw/usb/dev-serial.c
> index abc316c7bf..badf8785db 100644
> --- a/hw/usb/dev-serial.c
> +++ b/hw/usb/dev-serial.c
> @@ -307,6 +307,7 @@ static void usb_serial_handle_control(USBDevice *dev, 
> USBPacket *p,
>  }
>  
>  s->params.speed = (4800 / 2) / (8 * divisor + subdivisor8);
> +trace_usb_serial_set_baud(bus->busnr, dev->addr, s->params.speed);
>  qemu_chr_fe_ioctl(>cs, CHR_IOCTL_SERIAL_SET_PARAMS, >params);
>  break;
>  }
> @@ -340,6 +341,8 @@ static void usb_serial_handle_control(USBDevice *dev, 
> USBPacket *p,
>  goto fail;
>  }
>  
> +trace_usb_serial_set_data(bus->busnr, dev->addr, s->params.parity,
> +  s->params.data_bits, s->params.stop_bits);
>  qemu_chr_fe_ioctl(>cs, CHR_IOCTL_SERIAL_SET_PARAMS, >params);
>  /* TODO: TX ON/OFF */
>  break;
> diff --git a/hw/usb/trace-events b/hw/usb/trace-events
> index e5871cbbbc..9e984b2e0c 100644
> --- a/hw/usb/trace-events
> +++ b/hw/usb/trace-events
> @@ -328,3 +328,5 @@ usb_serial_unsupported_parity(int bus, int addr, int 
> value) "dev %d:%d unsupport
>  usb_serial_unsupported_stopbits(int bus, int addr, int value) "dev %d:%d 
> unsupported stop bits %d"
>  usb_serial_unsupported_control(int bus, int addr, int request, int value) 
> "dev %d:%d got unsupported/bogus control 0x%x, value 0x%x"
>  usb_serial_bad_token(int bus, int addr) "dev %d:%d bad token"
> +usb_serial_set_baud(int bus, int addr, int baud) "dev %d:%d baud rate %d"
> +usb_serial_set_data(int bus, int addr, int parity, int data, int stop) "dev 
> %d:%d parity %c, data bits %d, stop bits %d"
> -- 
> 2.20.1
> 

-- 
Samuel
 J'ai un gros problème: j'ai cet exercice à rendre demain lundi, mais ma
 TI 89 ne sait pas le faire...
 Est-ce que quelqu'un pourrait m'aider??
 -+- OD In Guide du Neuneu Usenet : Comment ça ! Il faut réfléchir ?-+-



Re: [PATCH 5/9] dev-serial: replace DeviceOutVendor/DeviceInVendor with equivalent macros from usb.h

2020-10-26 Thread Samuel Thibault
Mark Cave-Ayland, le lun. 26 oct. 2020 08:33:57 +, a ecrit:
> The DeviceOutVendor and DeviceInVendor macros can be replaced with their
> equivalent VendorDeviceOutRequest and VendorDeviceRequest macros from usb.h.
> 
> Signed-off-by: Mark Cave-Ayland 

Reviewed-by: Samuel Thibault 

> ---
>  hw/usb/dev-serial.c | 25 ++---
>  1 file changed, 10 insertions(+), 15 deletions(-)
> 
> diff --git a/hw/usb/dev-serial.c b/hw/usb/dev-serial.c
> index badf8785db..92c35615eb 100644
> --- a/hw/usb/dev-serial.c
> +++ b/hw/usb/dev-serial.c
> @@ -37,11 +37,6 @@
>  #define FTDI_SET_LATENCY   9
>  #define FTDI_GET_LATENCY   10
>  
> -#define DeviceOutVendor \
> -   ((USB_DIR_OUT | USB_TYPE_VENDOR | USB_RECIP_DEVICE) << 8)
> -#define DeviceInVendor \
> -   ((USB_DIR_IN | USB_TYPE_VENDOR | USB_RECIP_DEVICE) << 8)
> -
>  /* RESET */
>  
>  #define FTDI_RESET_SIO 0
> @@ -253,7 +248,7 @@ static void usb_serial_handle_control(USBDevice *dev, 
> USBPacket *p,
>  break;
>  
>  /* Class specific requests.  */
> -case DeviceOutVendor | FTDI_RESET:
> +case VendorDeviceOutRequest | FTDI_RESET:
>  switch (value) {
>  case FTDI_RESET_SIO:
>  usb_serial_reset(s);
> @@ -268,7 +263,7 @@ static void usb_serial_handle_control(USBDevice *dev, 
> USBPacket *p,
>  break;
>  }
>  break;
> -case DeviceOutVendor | FTDI_SET_MDM_CTRL:
> +case VendorDeviceOutRequest | FTDI_SET_MDM_CTRL:
>  {
>  static int flags;
>  qemu_chr_fe_ioctl(>cs, CHR_IOCTL_SERIAL_GET_TIOCM, );
> @@ -289,10 +284,10 @@ static void usb_serial_handle_control(USBDevice *dev, 
> USBPacket *p,
>  qemu_chr_fe_ioctl(>cs, CHR_IOCTL_SERIAL_SET_TIOCM, );
>  break;
>  }
> -case DeviceOutVendor | FTDI_SET_FLOW_CTRL:
> +case VendorDeviceOutRequest | FTDI_SET_FLOW_CTRL:
>  /* TODO: ioctl */
>  break;
> -case DeviceOutVendor | FTDI_SET_BAUD: {
> +case VendorDeviceOutRequest | FTDI_SET_BAUD: {
>  static const int subdivisors8[8] = { 0, 4, 2, 1, 3, 5, 6, 7 };
>  int subdivisor8 = subdivisors8[((value & 0xc000) >> 14)
>   | ((index & 1) << 2)];
> @@ -311,7 +306,7 @@ static void usb_serial_handle_control(USBDevice *dev, 
> USBPacket *p,
>  qemu_chr_fe_ioctl(>cs, CHR_IOCTL_SERIAL_SET_PARAMS, >params);
>  break;
>  }
> -case DeviceOutVendor | FTDI_SET_DATA:
> +case VendorDeviceOutRequest | FTDI_SET_DATA:
>  switch (value & FTDI_PARITY) {
>  case 0:
>  s->params.parity = 'N';
> @@ -346,23 +341,23 @@ static void usb_serial_handle_control(USBDevice *dev, 
> USBPacket *p,
>  qemu_chr_fe_ioctl(>cs, CHR_IOCTL_SERIAL_SET_PARAMS, >params);
>  /* TODO: TX ON/OFF */
>  break;
> -case DeviceInVendor | FTDI_GET_MDM_ST:
> +case VendorDeviceRequest | FTDI_GET_MDM_ST:
>  data[0] = usb_get_modem_lines(s) | 1;
>  data[1] = FTDI_THRE | FTDI_TEMT;
>  p->actual_length = 2;
>  break;
> -case DeviceOutVendor | FTDI_SET_EVENT_CHR:
> +case VendorDeviceOutRequest | FTDI_SET_EVENT_CHR:
>  /* TODO: handle it */
>  s->event_chr = value;
>  break;
> -case DeviceOutVendor | FTDI_SET_ERROR_CHR:
> +case VendorDeviceOutRequest | FTDI_SET_ERROR_CHR:
>  /* TODO: handle it */
>  s->error_chr = value;
>  break;
> -case DeviceOutVendor | FTDI_SET_LATENCY:
> +case VendorDeviceOutRequest | FTDI_SET_LATENCY:
>  s->latency = value;
>  break;
> -case DeviceInVendor | FTDI_GET_LATENCY:
> +case VendorDeviceRequest | FTDI_GET_LATENCY:
>  data[0] = s->latency;
>  p->actual_length = 1;
>  break;
> -- 
> 2.20.1
> 

-- 
Samuel
How do I type "for i in *.dvi do xdvi i done" in a GUI?
(Discussion in comp.os.linux.misc on the intuitiveness of interfaces.)



Re: [PATCH 3/9] dev-serial: convert from DPRINTF to trace-events

2020-10-26 Thread Samuel Thibault
Mark Cave-Ayland, le lun. 26 oct. 2020 08:33:55 +, a ecrit:
> Signed-off-by: Mark Cave-Ayland 

Reviewed-by: Samuel Thibault 

> ---
>  hw/usb/dev-serial.c | 28 ++--
>  hw/usb/trace-events |  8 
>  2 files changed, 22 insertions(+), 14 deletions(-)
> 
> diff --git a/hw/usb/dev-serial.c b/hw/usb/dev-serial.c
> index 77ce89d38b..abc316c7bf 100644
> --- a/hw/usb/dev-serial.c
> +++ b/hw/usb/dev-serial.c
> @@ -20,15 +20,8 @@
>  #include "chardev/char-serial.h"
>  #include "chardev/char-fe.h"
>  #include "qom/object.h"
> +#include "trace.h"
>  
> -//#define DEBUG_Serial
> -
> -#ifdef DEBUG_Serial
> -#define DPRINTF(fmt, ...) \
> -do { printf("usb-serial: " fmt , ## __VA_ARGS__); } while (0)
> -#else
> -#define DPRINTF(fmt, ...) do {} while(0)
> -#endif
>  
>  #define RECV_BUF (512 - (2 * 8))
>  
> @@ -205,8 +198,9 @@ static void usb_serial_reset(USBSerialState *s)
>  static void usb_serial_handle_reset(USBDevice *dev)
>  {
>  USBSerialState *s = USB_SERIAL(dev);
> +USBBus *bus = usb_bus_from_device(dev);
>  
> -DPRINTF("Reset\n");
> +trace_usb_serial_reset(bus->busnr, dev->addr);
>  
>  usb_serial_reset(s);
>  /* TODO: Reset char device, send BREAK? */
> @@ -244,9 +238,11 @@ static void usb_serial_handle_control(USBDevice *dev, 
> USBPacket *p,
>int length, uint8_t *data)
>  {
>  USBSerialState *s = USB_SERIAL(dev);
> +USBBus *bus = usb_bus_from_device(dev);
>  int ret;
>  
> -DPRINTF("got control %x, value %x\n", request, value);
> +trace_usb_serial_handle_control(bus->busnr, dev->addr, request, value);
> +
>  ret = usb_desc_handle_control(dev, p, request, value, index, length, 
> data);
>  if (ret >= 0) {
>  return;
> @@ -326,7 +322,8 @@ static void usb_serial_handle_control(USBDevice *dev, 
> USBPacket *p,
>  s->params.parity = 'E';
>  break;
>  default:
> -DPRINTF("unsupported parity %d\n", value & FTDI_PARITY);
> +trace_usb_serial_unsupported_parity(bus->busnr, dev->addr,
> +value & FTDI_PARITY);
>  goto fail;
>  }
>  
> @@ -338,7 +335,8 @@ static void usb_serial_handle_control(USBDevice *dev, 
> USBPacket *p,
>  s->params.stop_bits = 2;
>  break;
>  default:
> -DPRINTF("unsupported stop bits %d\n", value & FTDI_STOP);
> +trace_usb_serial_unsupported_stopbits(bus->busnr, dev->addr,
> +  value & FTDI_STOP);
>  goto fail;
>  }
>  
> @@ -367,7 +365,8 @@ static void usb_serial_handle_control(USBDevice *dev, 
> USBPacket *p,
>  break;
>  default:
>  fail:
> -DPRINTF("got unsupported/bogus control %x, value %x\n", request, 
> value);
> +trace_usb_serial_unsupported_control(bus->busnr, dev->addr, request,
> + value);
>  p->status = USB_RET_STALL;
>  break;
>  }
> @@ -431,6 +430,7 @@ static void usb_serial_token_in(USBSerialState *s, 
> USBPacket *p)
>  static void usb_serial_handle_data(USBDevice *dev, USBPacket *p)
>  {
>  USBSerialState *s = USB_SERIAL(dev);
> +USBBus *bus = usb_bus_from_device(dev);
>  uint8_t devep = p->ep->nr;
>  struct iovec *iov;
>  int i;
> @@ -459,7 +459,7 @@ static void usb_serial_handle_data(USBDevice *dev, 
> USBPacket *p)
>  break;
>  
>  default:
> -DPRINTF("Bad token\n");
> +trace_usb_serial_bad_token(bus->busnr, dev->addr);
>  fail:
>  p->status = USB_RET_STALL;
>  break;
> diff --git a/hw/usb/trace-events b/hw/usb/trace-events
> index 72e4298780..e5871cbbbc 100644
> --- a/hw/usb/trace-events
> +++ b/hw/usb/trace-events
> @@ -320,3 +320,11 @@ usb_host_parse_interface(int bus, int addr, int num, int 
> alt, int active) "dev %
>  usb_host_parse_endpoint(int bus, int addr, int ep, const char *dir, const 
> char *type, int active) "dev %d:%d, ep %d, %s, %s, active %d"
>  usb_host_parse_error(int bus, int addr, const char *errmsg) "dev %d:%d, msg 
> %s"
>  usb_host_remote_wakeup_removed(int bus, int addr) "dev %d:%d"
> +
> +# dev-serial.c
> +usb_serial_reset(int bus, int addr) "dev %d:%d reset"
> +usb_serial_handle_control(int bus, int addr, int request, int valu

Re: [PATCH 2/9] dev-serial: use USB_SERIAL QOM macro for USBSerialState assignments

2020-10-26 Thread Samuel Thibault
Mark Cave-Ayland, le lun. 26 oct. 2020 08:33:54 +, a ecrit:
> Signed-off-by: Mark Cave-Ayland 

Reviewed-by: Samuel Thibault 

> ---
>  hw/usb/dev-serial.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/hw/usb/dev-serial.c b/hw/usb/dev-serial.c
> index 7a5fa3770e..77ce89d38b 100644
> --- a/hw/usb/dev-serial.c
> +++ b/hw/usb/dev-serial.c
> @@ -204,7 +204,7 @@ static void usb_serial_reset(USBSerialState *s)
>  
>  static void usb_serial_handle_reset(USBDevice *dev)
>  {
> -USBSerialState *s = (USBSerialState *)dev;
> +USBSerialState *s = USB_SERIAL(dev);
>  
>  DPRINTF("Reset\n");
>  
> @@ -243,7 +243,7 @@ static void usb_serial_handle_control(USBDevice *dev, 
> USBPacket *p,
>int request, int value, int index,
>int length, uint8_t *data)
>  {
> -USBSerialState *s = (USBSerialState *)dev;
> +USBSerialState *s = USB_SERIAL(dev);
>  int ret;
>  
>  DPRINTF("got control %x, value %x\n", request, value);
> @@ -430,7 +430,7 @@ static void usb_serial_token_in(USBSerialState *s, 
> USBPacket *p)
>  
>  static void usb_serial_handle_data(USBDevice *dev, USBPacket *p)
>  {
> -USBSerialState *s = (USBSerialState *)dev;
> +USBSerialState *s = USB_SERIAL(dev);
>  uint8_t devep = p->ep->nr;
>  struct iovec *iov;
>  int i;
> -- 
> 2.20.1
> 

-- 
Samuel
 t: bah c'est tendre le pattern pour se faire matcher, hein



Re: [PATCH 1/9] dev-serial: style changes to improve readability and checkpatch fixes

2020-10-26 Thread Samuel Thibault
Mark Cave-Ayland, le lun. 26 oct. 2020 08:33:53 +, a ecrit:
> Signed-off-by: Mark Cave-Ayland 

Reviewed-by: Samuel thibault 

> ---
>  hw/usb/dev-serial.c | 230 
>  1 file changed, 126 insertions(+), 104 deletions(-)
> 
> diff --git a/hw/usb/dev-serial.c b/hw/usb/dev-serial.c
> index b1622b7c7f..7a5fa3770e 100644
> --- a/hw/usb/dev-serial.c
> +++ b/hw/usb/dev-serial.c
> @@ -33,72 +33,75 @@ do { printf("usb-serial: " fmt , ## __VA_ARGS__); } while 
> (0)
>  #define RECV_BUF (512 - (2 * 8))
>  
>  /* Commands */
> -#define FTDI_RESET   0
> -#define FTDI_SET_MDM_CTRL1
> -#define FTDI_SET_FLOW_CTRL   2
> -#define FTDI_SET_BAUD3
> -#define FTDI_SET_DATA4
> -#define FTDI_GET_MDM_ST  5
> -#define FTDI_SET_EVENT_CHR   6
> -#define FTDI_SET_ERROR_CHR   7
> -#define FTDI_SET_LATENCY 9
> -#define FTDI_GET_LATENCY 10
> -
> -#define DeviceOutVendor  
> ((USB_DIR_OUT|USB_TYPE_VENDOR|USB_RECIP_DEVICE)<<8)
> -#define DeviceInVendor   ((USB_DIR_IN 
> |USB_TYPE_VENDOR|USB_RECIP_DEVICE)<<8)
> +#define FTDI_RESET 0
> +#define FTDI_SET_MDM_CTRL  1
> +#define FTDI_SET_FLOW_CTRL 2
> +#define FTDI_SET_BAUD  3
> +#define FTDI_SET_DATA  4
> +#define FTDI_GET_MDM_ST5
> +#define FTDI_SET_EVENT_CHR 6
> +#define FTDI_SET_ERROR_CHR 7
> +#define FTDI_SET_LATENCY   9
> +#define FTDI_GET_LATENCY   10
> +
> +#define DeviceOutVendor \
> +   ((USB_DIR_OUT | USB_TYPE_VENDOR | USB_RECIP_DEVICE) << 8)
> +#define DeviceInVendor \
> +   ((USB_DIR_IN | USB_TYPE_VENDOR | USB_RECIP_DEVICE) << 8)
>  
>  /* RESET */
>  
> -#define FTDI_RESET_SIO   0
> -#define FTDI_RESET_RX1
> -#define FTDI_RESET_TX2
> +#define FTDI_RESET_SIO 0
> +#define FTDI_RESET_RX  1
> +#define FTDI_RESET_TX  2
>  
>  /* SET_MDM_CTRL */
>  
> -#define FTDI_DTR 1
> -#define FTDI_SET_DTR (FTDI_DTR << 8)
> -#define FTDI_RTS 2
> -#define FTDI_SET_RTS (FTDI_RTS << 8)
> +#define FTDI_DTR   1
> +#define FTDI_SET_DTR   (FTDI_DTR << 8)
> +#define FTDI_RTS   2
> +#define FTDI_SET_RTS   (FTDI_RTS << 8)
>  
>  /* SET_FLOW_CTRL */
>  
> -#define FTDI_RTS_CTS_HS  1
> -#define FTDI_DTR_DSR_HS  2
> -#define FTDI_XON_XOFF_HS 4
> +#define FTDI_RTS_CTS_HS1
> +#define FTDI_DTR_DSR_HS2
> +#define FTDI_XON_XOFF_HS   4
>  
>  /* SET_DATA */
>  
> -#define FTDI_PARITY  (0x7 << 8)
> -#define FTDI_ODD (0x1 << 8)
> -#define FTDI_EVEN(0x2 << 8)
> -#define FTDI_MARK(0x3 << 8)
> -#define FTDI_SPACE   (0x4 << 8)
> +#define FTDI_PARITY(0x7 << 8)
> +#define FTDI_ODD   (0x1 << 8)
> +#define FTDI_EVEN  (0x2 << 8)
> +#define FTDI_MARK  (0x3 << 8)
> +#define FTDI_SPACE (0x4 << 8)
>  
> -#define FTDI_STOP(0x3 << 11)
> -#define FTDI_STOP1   (0x0 << 11)
> -#define FTDI_STOP15  (0x1 << 11)
> -#define FTDI_STOP2   (0x2 << 11)
> +#define FTDI_STOP  (0x3 << 11)
> +#define FTDI_STOP1 (0x0 << 11)
> +#define FTDI_STOP15(0x1 << 11)
> +#define FTDI_STOP2 (0x2 << 11)
>  
>  /* GET_MDM_ST */
>  /* TODO: should be sent every 40ms */
> -#define FTDI_CTS  (1<<4)// CTS line status
> -#define FTDI_DSR  (1<<5)// DSR line status
> -#define FTDI_RI   (1<<6)// RI line status
> -#define FTDI_RLSD (1<<7)// Receive Line Signal Detect
> +#define FTDI_CTS   (1 << 4)/* CTS line status */
> +#define FTDI_DSR   (1 << 5)/* DSR line status */
> +#define FTDI_RI(1 << 6)/* RI line status */
> +#define FTDI_RLSD  (1 << 7)/* Receive Line Signal Detect */
>  
>  /* Status */
>  
> -#define FTDI_DR   (1<<0)// Data Ready
> -#define FTDI_OE   (1<<1)// Overrun Err
> -#define FTDI_PE   (1<<2)// Parity Err
> -#define FTDI_FE   (1<<3)// Framing Err
> -#define FTDI_BI   (1<<4)// Break Interrupt
> -#define FTDI_THRE (1<<5)// Transmitter Holding Register
> -#define FTDI_TEMT (1<<6)// Transmitter Empty
> -#define FTDI_FIFO (1<<7)// Error in FIFO
> +#define FTDI_DR(1 << 0)/* Data Ready */
> +#define FTDI_OE(1 << 1)/* Overrun Err */
> +#define FTDI_PE(1 << 2)/* Parity Err */
> +#define FTDI_FE(1 << 3)/* Framing Err */
> +#define FTDI_BI(1 << 4)/* B

[PATCH] ui: Fix default window_id value

2020-09-22 Thread Samuel Thibault
./chardev/baum.c expects the default window_id value to be -1, and not 0
which could be confused with a proper window id (when numbered from 0 by
the ui backend).

This fixes getting Braille output with the curses and gtk frontends.

Signed-off-by: Samuel Thibault 
Fixes: f29b3431f62 ("console: move window ID code from baum to sdl")
Reviewed-by: Philippe Mathieu-Daudé 

---
It would be useful to backport this to stable trees.

diff --git a/ui/console.c b/ui/console.c
index f8d7643fe4..beb733c833 100644
--- a/ui/console.c
+++ b/ui/console.c
@@ -1310,6 +1310,7 @@ static QemuConsole *new_console(DisplayState *ds, 
console_type_t console_type,
 }
 s->ds = ds;
 s->console_type = console_type;
+s->window_id = -1;
 
 if (QTAILQ_EMPTY()) {
 s->index = 0;



[PATCH] ui: Fix default window_id value

2020-09-14 Thread Samuel Thibault
./chardev/baum.c expects the default window_id value to be -1, and not 0
which could be confused with a proper window id (when numbered from 0 by
the ui backend).

This fixes getting Braille output with the curses and gtk frontends.

Signed-off-by: Samuel Thibault 

---
It would be useful to backport this to stable trees.

diff --git a/ui/console.c b/ui/console.c
index f8d7643fe4..beb733c833 100644
--- a/ui/console.c
+++ b/ui/console.c
@@ -1310,6 +1310,7 @@ static QemuConsole *new_console(DisplayState *ds, 
console_type_t console_type,
 }
 s->ds = ds;
 s->console_type = console_type;
+s->window_id = -1;
 
 if (QTAILQ_EMPTY()) {
 s->index = 0;



Re: Is traceroute supposed to work in user mode networking (slirp) ?

2020-07-19 Thread Samuel Thibault
Ottavio Caruso, le dim. 19 juil. 2020 12:07:21 +0100, a ecrit:
> On Sun, 19 Jul 2020 at 03:50, Samuel Thibault  wrote:
> > Ottavio Caruso, le mar. 14 juil. 2020 12:15:48 +0100, a ecrit:
> > > I cannot get traceroute to work with standard udp from any of my
> > > guests.
> > >
> > > $ traceroute 8.8.8.8
> > > traceroute to 8.8.8.8 (8.8.8.8), 64 hops max, 40 byte packets
> > >  1  * * *
> >
> > That was because
> >
> > - libslirp was not forwarding the ttl value, thus always set to 64 hops
> > - libslirp was not reporting icmp errors.
> >
> > I had a try at both, and that indeed seems to be fixing the issue:
> > https://gitlab.freedesktop.org/slirp/libslirp/-/merge_requests/48
> > https://gitlab.freedesktop.org/slirp/libslirp/-/merge_requests/49
> >
> > > Any clues? Is this intended behaviour? Any workarounds that don't
> > > involve tap/tun/bridges?
> >
> > Not without updating libslirp with the abovementioned patches.
> 
> Thanks Samuel. I've added a comment on the portal, but for the benefit
> of qemu-devel:
> 
> Applying this patch on the latest qemu (5.0.90),

Did you also apply
https://gitlab.freedesktop.org/slirp/libslirp/-/merge_requests/48 ?

Samuel



Re: Is traceroute supposed to work in user mode networking (slirp) ?

2020-07-18 Thread Samuel Thibault
Hello,

Ottavio Caruso, le mar. 14 juil. 2020 12:15:48 +0100, a ecrit:
> I cannot get traceroute to work with standard udp from any of my
> guests.
> 
> $ traceroute 8.8.8.8
> traceroute to 8.8.8.8 (8.8.8.8), 64 hops max, 40 byte packets
>  1  * * *

That was because

- libslirp was not forwarding the ttl value, thus always set to 64 hops
- libslirp was not reporting icmp errors.

I had a try at both, and that indeed seems to be fixing the issue:
https://gitlab.freedesktop.org/slirp/libslirp/-/merge_requests/48
https://gitlab.freedesktop.org/slirp/libslirp/-/merge_requests/49

> Any clues? Is this intended behaviour? Any workarounds that don't
> involve tap/tun/bridges?

Not without updating libslirp with the abovementioned patches.

Samuel



Re: [PATCH v2] slirp: update submodule to v4.2.0 + mingw-fix

2020-03-18 Thread Samuel Thibault
Marc-André Lureau, le mar. 17 mars 2020 19:13:36 +0100, a ecrit:
> git shortlog
> 126c04acbabd7ad32c2b018fe10dfac2a3bc1210..7012a2c62e5b54eab88c119383022ec7ce86e9b2
> 
> 5eraph (1):
>   Use specific outbound IP address
> 
> Akihiro Suda (8):
>   remove confusing comment that exists from ancient slirp
>   add slirp_new(SlirpConfig *, SlirpCb *, void *)
>   allow custom MTU
>   add disable_host_loopback (prohibit connections to 127.0.0.1)
>   add SlirpConfig version
>   emu: remove dead code
>   emu: disable by default
>   fix a typo in a comment
> 
> Anders Waldenborg (1):
>   state: fix loading of guestfwd state
> 
> Giuseppe Scrivano (1):
>   socket: avoid getpeername after shutdown(SHUT_WR)
> 
> Jindrich Novy (1):
>   Don't leak memory when reallocation fails.
> 
> Jordi Pujol Palomer (1):
>   fork_exec: correctly parse command lines that contain spaces
> 
> Marc-André Lureau (54):
>   Merge branch 'AkihiroSuda/libslirp-slirp4netns'
>   Merge branch 'fix-typo' into 'master'
>   meson: make it subproject friendly
>   Merge branch 'meson' into 'master'
>   misc: fix compilation warnings
>   Merge branch 'fix-shutdown-wr' into 'master'
>   sbuf: remove unused and undefined sbcopy() path
>   sbuf: check more strictly sbcopy() bounds with offset
>   sbuf: replace a comment with a runtime warning
>   Replace remaining malloc/free user with glib
>   tcp_attach() can no longer fail
>   state: can't ENOMEM
>   sbuf: use unsigned types
>   sbuf: simplify sbreserve()
>   dnssearch: use g_strv_length()
>   vmstate: silence scan-build warning
>   gitlab-ci: run scan-build
>   Merge branch 'mem-cleanups' into 'master'
>   libslirp.map: bind slirp_new to SLIRP_4.1 version
>   meson: fix libtool versioning
>   Release v4.1.0
>   Merge branch '4.1.0' into 'master'
>   CHANGELOG: start unreleased section
>   Merge branch 'add-unix' into 'master'
>   util: add G_SIZEOF_MEMBER() macro
>   Check bootp_filename is not going to be truncated
>   bootp: remove extra cast
>   bootp: replace simple snprintf() with strcpy()
>   tftp: clarify what is actually OACK m_len
>   tcp_emu: add more fixme/warnings comments
>   util: add slirp_fmt() helpers
>   dhcpv6: use slirp_fmt()
>   misc: use slirp_fmt0()
>   tftp: use slirp_fmt0()
>   tcp_ctl: use slirp_fmt()
>   tcp_emu: fix unsafe snprintf() usages
>   misc: improve error report
>   Use g_snprintf()
>   util: add gnuc format function attribute to slirp_fmt*
>   Merge branch 'aw-guestfwd-state' into 'master'
>   Merge branch 'slirp-fmt' into 'master'
>   socket: remove extra label and variable
>   socket: factor out sotranslate ipv4/ipv6 handling
>   socket: remove need for extra scope_id variable
>   socket: do not fallback on host loopback if get_dns_addr() failed
>   socket: do not fallback on loopback addr for addresses in our 
> mask/prefix
>   Prepare for v4.2.0 release
>   Merge branch 'translate-fix' into 'master'
>   Merge branch 'release-v4.2.0' into 'master'
>   changelog: post-release
>   changelog: fix link
>   .gitlab-ci: add --werror, treat CI build warnings as errors
>   Revert "socket: remove need for extra scope_id variable"
>   Merge branch 'mingw-fix' into 'master'
> 
> PanNengyuan (1):
>   libslirp: fix NULL pointer dereference in tcp_sockclosed
> 
> Philippe Mathieu-Daudé (1):
>   Add a git-publish configuration file
> 
> Prasad J Pandit (4):
>   slirp: ncsi: compute checksum for valid data length
>   slirp: use correct size while emulating IRC commands
>   slirp: use correct size while emulating commands
>   slirp: tftp: restrict relative path access
> 
> Renzo Davoli (2):
>   Add slirp_remove_guestfwd()
>   Add slirp_add_unix()
> 
> Samuel Thibault (14):
>   ip_reass: explain why we should not always update the q pointer
>   Merge branch 'comment' into 'master'
>   Merge branch 'no-emu' into 'master'
>   Fix bogus indent, no source change
>   ip_reass: Fix use after free
>   Merge branch 'reass2' into 'master'
>   Make host receive broadcast packets
>   arp: Allow 0.0.0.0 destination address
>   Merge branch 'warnings' into 'master'
>   Merge branch 'arp_0' into 'master'
>   Merge branch 'broadcast' into 'master'
>   tcp_emu: Fix oob access
>   Merge branch 'oob' into 'master'
>   Merge branch 'master' into 'master'
> 
> Cc: Samuel Thibault 
> Signe

Re: [PATCH 4/4] usb-serial: Fix timeout closing the device

2020-03-14 Thread Samuel Thibault
Jason Andryuk, le jeu. 12 mars 2020 08:55:23 -0400, a ecrit:
> Linux guests wait ~30 seconds when closing the emulated /dev/ttyUSB0.
> During that time, the kernel driver is sending many control URBs
> requesting GetModemStat (5).  Real hardware returns a status with
> FTDI_THRE (Transmitter Holding Register) and FTDI_TEMT (Transmitter
> Empty) set.  QEMU leaves them clear, and it seems Linux is waiting for
> FTDI_TEMT to be set to indicate the tx queue is empty before closing.
> 
> Set the bits when responding to a GetModemStat query and avoid the
> shutdown delay.
> 
> Signed-off-by: Jason Andryuk 

Reviewed-by: Samuel Thibault 

> ---
> Looking at a USB dump for a real FTDI USB adapter, I see these bits set
> in all the bulk URBs where QEMU currently has them clear.
> ---
>  hw/usb/dev-serial.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/usb/dev-serial.c b/hw/usb/dev-serial.c
> index ef33bcd127..5389235f17 100644
> --- a/hw/usb/dev-serial.c
> +++ b/hw/usb/dev-serial.c
> @@ -332,7 +332,7 @@ static void usb_serial_handle_control(USBDevice *dev, 
> USBPacket *p,
>  break;
>  case DeviceInVendor | FTDI_GET_MDM_ST:
>  data[0] = usb_get_modem_lines(s) | 1;
> -data[1] = 0;
> +data[1] = FTDI_THRE | FTDI_TEMT;
>  p->actual_length = 2;
>  break;
>  case DeviceOutVendor | FTDI_SET_EVENT_CHR:
> -- 
> 2.24.1
> 

-- 
Samuel
c> ah (on trouve fluide glacial sur le net, ou il faut aller dans le monde reel 
?)
s> dans le monde reel
c> zut



Re: [PATCH 3/4] usb-serial: Increase receive buffer to 496

2020-03-14 Thread Samuel Thibault
Jason Andryuk, le jeu. 12 mars 2020 08:55:22 -0400, a ecrit:
> A FTDI USB adapter on an xHCI controller can send 512 byte USB packets.
> These are 8 * ( 2 bytes header + 62 bytes data).  A 384 byte receive
> buffer is insufficient to fill a 512 byte packet, so bump the receive
> size to 496 ( 512 - 2 * 8 ).
> 
> Signed-off-by: Jason Andryuk 

Reviewed-by: Samuel Thibault 

> ---
>  hw/usb/dev-serial.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/usb/dev-serial.c b/hw/usb/dev-serial.c
> index 96b6c34202..ef33bcd127 100644
> --- a/hw/usb/dev-serial.c
> +++ b/hw/usb/dev-serial.c
> @@ -29,7 +29,7 @@ do { printf("usb-serial: " fmt , ## __VA_ARGS__); } while 
> (0)
>  #define DPRINTF(fmt, ...) do {} while(0)
>  #endif
>  
> -#define RECV_BUF 384
> +#define RECV_BUF (512 - (2 * 8))
>  
>  /* Commands */
>  #define FTDI_RESET   0
> -- 
> 2.24.1
> 

-- 
Samuel
After watching my newly-retired dad spend two weeks learning how to make a new
folder, it became obvious that "intuitive" mostly means "what the writer or
speaker of intuitive likes".
(Bruce Ediger, bedi...@teal.csn.org, in comp.os.linux.misc, on X the
intuitiveness of a Mac interface.)



Re: [PATCH 2/4] usb-serial: chunk data to wMaxPacketSize

2020-03-13 Thread Samuel Thibault
Jason Andryuk, le jeu. 12 mars 2020 08:55:21 -0400, a ecrit:
> usb-serial has issues with xHCI controllers where data is lost in the
> VM.  Inspecting the URBs in the guest, EHCI starts every 64 byte boundary
> (wMaxPacketSize) with a header.  EHCI hands packets into
> usb_serial_token_in() with size 64, so these cannot cross the 64 byte
> boundary.  The xHCI controller has packets of 512 bytes and the usb-serial
> will just write through the 64 byte boundary.  In the guest, this means
> data bytes are interpreted as header, so data bytes don't make it out
> the serial interface.
> 
> Re-work usb_serial_token_in to chunk data into 64 byte units - 2 byte
> header and 62 bytes data.  The Linux driver reads wMaxPacketSize to find
> the chunk size, so we match that.
> 
> Real hardware was observed to pass in 512 byte URBs (496 bytes data +
> 8 * 2 byte headers).  Since usb-serial only buffers 384 bytes of data,
> usb-serial will pass in 6 64 byte blocks and 1 12 byte partial block for
> 462 bytes max.
> 
> Signed-off-by: Jason Andryuk 

Reviewed-by: Samuel Thibault 

> ---
>  hw/usb/dev-serial.c | 43 +++
>  1 file changed, 27 insertions(+), 16 deletions(-)
> 
> diff --git a/hw/usb/dev-serial.c b/hw/usb/dev-serial.c
> index 71fa786bd8..96b6c34202 100644
> --- a/hw/usb/dev-serial.c
> +++ b/hw/usb/dev-serial.c
> @@ -360,15 +360,16 @@ static void usb_serial_handle_control(USBDevice *dev, 
> USBPacket *p,
>  
>  static void usb_serial_token_in(USBSerialState *s, USBPacket *p)
>  {
> -int first_len, len;
> +const int max_packet_size = desc_iface0.eps[0].wMaxPacketSize;
> +int packet_len;
>  uint8_t header[2];
>  
> -first_len = RECV_BUF - s->recv_ptr;
> -len = p->iov.size;
> -if (len <= 2) {
> +packet_len = p->iov.size;
> +if (packet_len <= 2) {
>  p->status = USB_RET_NAK;
>  return;
>  }
> +
>  header[0] = usb_get_modem_lines(s) | 1;
>  /* We do not have the uart details */
>  /* handle serial break */
> @@ -380,21 +381,31 @@ static void usb_serial_token_in(USBSerialState *s, 
> USBPacket *p)
>  } else {
>  header[1] = 0;
>  }
> -len -= 2;
> -if (len > s->recv_used)
> -len = s->recv_used;
> -if (!len) {
> +
> +if (!s->recv_used) {
>  p->status = USB_RET_NAK;
>  return;
>  }
> -if (first_len > len)
> -first_len = len;
> -usb_packet_copy(p, header, 2);
> -usb_packet_copy(p, s->recv_buf + s->recv_ptr, first_len);
> -if (len > first_len)
> -usb_packet_copy(p, s->recv_buf, len - first_len);
> -s->recv_used -= len;
> -s->recv_ptr = (s->recv_ptr + len) % RECV_BUF;
> +
> +while (s->recv_used && packet_len > 2) {
> +int first_len, len;
> +
> +len = MIN(packet_len, max_packet_size);
> +len -= 2;
> +if (len > s->recv_used)
> +len = s->recv_used;
> +
> +first_len = RECV_BUF - s->recv_ptr;
> +if (first_len > len)
> +first_len = len;
> +usb_packet_copy(p, header, 2);
> +usb_packet_copy(p, s->recv_buf + s->recv_ptr, first_len);
> +if (len > first_len)
> +usb_packet_copy(p, s->recv_buf, len - first_len);
> +s->recv_used -= len;
> +s->recv_ptr = (s->recv_ptr + len) % RECV_BUF;
> +packet_len -= len + 2;
> +}
>  
>  return;
>  }
> -- 
> 2.24.1
> 

-- 
Samuel
We are Pentium of Borg. Division is futile. You will be approximated.
(seen in someone's .signature)



Re: [PATCH 1/4] usb-serial: Move USB_TOKEN_IN into a helper function

2020-03-13 Thread Samuel Thibault
Jason Andryuk, le jeu. 12 mars 2020 08:55:20 -0400, a ecrit:
> We'll be adding a loop, so move the code into a helper function.  breaks
> are replaced with returns.
> 
> Signed-off-by: Jason Andryuk 

Reviewed-by: Samuel Thibault 

> ---
>  hw/usb/dev-serial.c | 77 +
>  1 file changed, 43 insertions(+), 34 deletions(-)
> 
> diff --git a/hw/usb/dev-serial.c b/hw/usb/dev-serial.c
> index daac75b7ae..71fa786bd8 100644
> --- a/hw/usb/dev-serial.c
> +++ b/hw/usb/dev-serial.c
> @@ -358,13 +358,53 @@ static void usb_serial_handle_control(USBDevice *dev, 
> USBPacket *p,
>  }
>  }
>  
> +static void usb_serial_token_in(USBSerialState *s, USBPacket *p)
> +{
> +int first_len, len;
> +uint8_t header[2];
> +
> +first_len = RECV_BUF - s->recv_ptr;
> +len = p->iov.size;
> +if (len <= 2) {
> +p->status = USB_RET_NAK;
> +return;
> +}
> +header[0] = usb_get_modem_lines(s) | 1;
> +/* We do not have the uart details */
> +/* handle serial break */
> +if (s->event_trigger && s->event_trigger & FTDI_BI) {
> +s->event_trigger &= ~FTDI_BI;
> +header[1] = FTDI_BI;
> +usb_packet_copy(p, header, 2);
> +return;
> +} else {
> +header[1] = 0;
> +}
> +len -= 2;
> +if (len > s->recv_used)
> +len = s->recv_used;
> +if (!len) {
> +p->status = USB_RET_NAK;
> +return;
> +}
> +if (first_len > len)
> +first_len = len;
> +usb_packet_copy(p, header, 2);
> +usb_packet_copy(p, s->recv_buf + s->recv_ptr, first_len);
> +if (len > first_len)
> +usb_packet_copy(p, s->recv_buf, len - first_len);
> +s->recv_used -= len;
> +s->recv_ptr = (s->recv_ptr + len) % RECV_BUF;
> +
> +return;
> +}
> +
>  static void usb_serial_handle_data(USBDevice *dev, USBPacket *p)
>  {
>  USBSerialState *s = (USBSerialState *)dev;
>  uint8_t devep = p->ep->nr;
>  struct iovec *iov;
> -uint8_t header[2];
> -int i, first_len, len;
> +int i;
>  
>  switch (p->pid) {
>  case USB_TOKEN_OUT:
> @@ -382,38 +422,7 @@ static void usb_serial_handle_data(USBDevice *dev, 
> USBPacket *p)
>  case USB_TOKEN_IN:
>  if (devep != 1)
>  goto fail;
> -first_len = RECV_BUF - s->recv_ptr;
> -len = p->iov.size;
> -if (len <= 2) {
> -p->status = USB_RET_NAK;
> -break;
> -}
> -header[0] = usb_get_modem_lines(s) | 1;
> -/* We do not have the uart details */
> -/* handle serial break */
> -if (s->event_trigger && s->event_trigger & FTDI_BI) {
> -s->event_trigger &= ~FTDI_BI;
> -header[1] = FTDI_BI;
> -usb_packet_copy(p, header, 2);
> -break;
> -} else {
> -header[1] = 0;
> -}
> -len -= 2;
> -if (len > s->recv_used)
> -len = s->recv_used;
> -if (!len) {
> -p->status = USB_RET_NAK;
> -break;
> -}
> -if (first_len > len)
> -first_len = len;
> -usb_packet_copy(p, header, 2);
> -usb_packet_copy(p, s->recv_buf + s->recv_ptr, first_len);
> -if (len > first_len)
> -usb_packet_copy(p, s->recv_buf, len - first_len);
> -s->recv_used -= len;
> -s->recv_ptr = (s->recv_ptr + len) % RECV_BUF;
> +usb_serial_token_in(s, p);
>  break;
>  
>  default:
> -- 
> 2.24.1
> 

-- 
Samuel
Tu as lu les docs. Tu es devenu un informaticien. Que tu le veuilles
ou non. Lire la doc, c'est le Premier et Unique Commandement de
l'informaticien.
-+- TP in: Guide du Linuxien pervers - "L'évangile selon St Thomas"



Re: [PATCH v2 07/30] qapi/block-core.json: Use literal block for ascii art

2020-02-16 Thread Samuel Thibault
Hello,

Philippe Mathieu-Daudé, le lun. 17 févr. 2020 01:44:35 +0100, a ecrit:
> On Sat, Feb 15, 2020 at 10:01 PM Aleksandar Markovic
>  wrote:
> > 9:56 PM Sub, 15.02.2020. Philippe Mathieu-Daudé 
> >  је написао/ла:
> > > On Fri, Feb 14, 2020 at 12:04 AM Aleksandar Markovic
> > >  wrote:
> > > >
> > > > 6:59 PM Čet, 13.02.2020. Peter Maydell  је 
> > > > написао/ла:
> > > > >
> > > > > The ascii-art graph
> > > >
> > > > Just out of couriousity, are unicode characters allowed in rst files?
> > >
> > > I remember 2 years ago a blind developer thanked the QEMU community to
> > > still restrict commits to 80 characters, because while 4K display are
> > > available, he and other visually impaired developers cloud still
> > > browse the QEMU codebase with their refreshable Braille display (which
> > > was 80 cels). I don't know how many visually impaired developers are
> > > following this project. A quick google returns " There is no concept
> > > of Unicode in Braille. In that sense Braille is similar to old 8-bit
> > > code pages which represented different symbols in different languages
> > > for the same symbol code."
> > > (https://superuser.com/questions/629443/represent-unicode-characters-in-braille).
> > >
> > > (I'm Cc'ing Samuel who cares about Braille displays.)

Nowadays' screen reader do provide some translations for some unicode
fancies. But the analogy with codepage remains true: since there are
only 256 braille pattern, there will be ambiguity between representation
of plain text and representation of unicode fancies. Using plain ascii
avoids this issue, i.e. blind developers will know that '|' and '-' are
commonly used for drawing, and their Braille representations are not
ambiguous. So that's indeed better to keep them ascii.

More generally, ascii art is however hard to catch with a Braille device
anyway...

Samuel



[Bug 1812451] Re: In windows host, tftp arbitrary file read vulnerability

2020-01-20 Thread Samuel thibault
This is fixed upstream by
https://gitlab.freedesktop.org/slirp/libslirp/commit/14ec36e107a8c9af7d0a80c3571fe39b291ff1d4

-- 
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1812451

Title:
  In windows host, tftp arbitrary file read vulnerability

Status in QEMU:
  Fix Committed

Bug description:
  https://github.com/qemu/qemu/blob/master/slirp/tftp.c#L343

    if (!strncmp(req_fname, "../", 3) ||
    req_fname[strlen(req_fname) - 1] == '/' ||
    strstr(req_fname, "/../")) {
    tftp_send_error(spt, 2, "Access violation", tp);
    return;
    }

  There is file path check for not allowing escape tftp directory.
  But, in windows, file path is separated by "\" backslash.
  So, guest can read arbitrary file in Windows host.

  This bug is variant of CVE-2019-2553 - Directory traversal
  vulnerability.

To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/1812451/+subscriptions



Re: [PATCH] build-sys: do not include Windows SLIRP dependencies in $LIBS

2019-12-11 Thread Samuel Thibault
Paolo Bonzini, le mer. 11 déc. 2019 15:23:23 +0100, a ecrit:
> When including the internal SLIRP library, we should add all the libraries 
> that
> it needs for the build.  Right now they are all included by QEMU, but 
> -liphlpapi
> is not needed without slirp.  Move it from LIBS to slirp_libs.
> 
> Based on a patch by Marc-André Lureau.
> 
> Signed-off-by: Paolo Bonzini 

Acked-by: Samuel Thibault 

(I don't have a win environment to test this)

> ---
>  configure | 5 -
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/configure b/configure
> index 6099be1..d16dad2 100755
> --- a/configure
> +++ b/configure
> @@ -926,7 +926,7 @@ if test "$mingw32" = "yes" ; then
>DSOSUF=".dll"
># MinGW needs -mthreads for TLS and macro _MT.
>QEMU_CFLAGS="-mthreads $QEMU_CFLAGS"
> -  LIBS="-lwinmm -lws2_32 -liphlpapi $LIBS"
> +  LIBS="-lwinmm -lws2_32 $LIBS"
>write_c_skeleton;
>if compile_prog "" "-liberty" ; then
>  LIBS="-liberty $LIBS"
> @@ -6069,6 +6069,9 @@ case "$slirp" in
>  mkdir -p slirp
>  slirp_cflags="-I\$(SRC_PATH)/slirp/src -I\$(BUILD_DIR)/slirp/src"
>  slirp_libs="-L\$(BUILD_DIR)/slirp -lslirp"
> +if test "$mingw32" = "yes" ; then
> +  slirp_libs="$slirp_libs -lws2_32 -liphlpapi"
> +fi
>  ;;
>  
>system)
> -- 
> 1.8.3.1
> 
> 

-- 
Samuel
 > Quelqu'un aurait-il une solution pour réinitialiser un MBR
 Si tu veux qu'il soit complètement blanc (pas souhaitable, à mon avis) :
 dd if=/dev/zero of=/dev/hda bs=512 count=1 (sous Linux)
 -+- OT in Guide du linuxien (très) pervers - "Pour les K difficiles" -+-



Re: Braille device (chardev/baum.c) is unable to detect the TTY correctly and does not act on graphic console connect/disconnect

2019-11-14 Thread Samuel Thibault
Samuel Thibault, le jeu. 14 nov. 2019 14:27:12 +0100, a ecrit:
> Samuel Thibault, le jeu. 14 nov. 2019 14:08:41 +0100, a ecrit:
> > The way to properly fix it is to add a brlapi channel to spice:
> 
> And that would be workable through a spice agent as well, so that
> braille management from orca

Or event NVDA or Narrator, actually.

> running inside the guest could talk
> directly through to brltty running on the host instead of having to go
> through the emulation layers.
> 
> Samuel



Re: Braille device (chardev/baum.c) is unable to detect the TTY correctly and does not act on graphic console connect/disconnect

2019-11-14 Thread Samuel Thibault
Samuel Thibault, le jeu. 14 nov. 2019 14:08:41 +0100, a ecrit:
> The way to properly fix it is to add a brlapi channel to spice:

And that would be workable through a spice agent as well, so that
braille management from orca running inside the guest could talk
directly through to brltty running on the host instead of having to go
through the emulation layers.

Samuel



Re: Braille device (chardev/baum.c) is unable to detect the TTY correctly and does not act on graphic console connect/disconnect

2019-11-14 Thread Samuel Thibault
Hello,

Teemu Kuusisto, le jeu. 14 nov. 2019 14:09:15 +0200, a ecrit:
> As a blind developer I would be very happy to use QEMU's baum chardev for a 
> braille display. Unfortunately, this device fails to detect the tty in which 
> the spice client is running.

Ah indeed that case was never looked at.

> The current code calls qemu_console_get_window_id() to get the tty.

> The code does not currently consider the fact that the lifetime of the
> connected graphical consoles is not the same as the lifetime of the
> VM.

Indeed, with spice the situation is different.

> This function returns zero, which causes the code to skip even the
> default behavior of brlapi's brlapi__enterTtyMode() (including
> checcking some env variables such as CONTROLVT)

Mmm, indeed that should be fixed into letting brlapi try to find it, so
that CONTROLVT can work.

> window id sounds like something different than a tty number, maybe a
> number of X display?

Yes, under X you need to provide the X window id.

> Is it possible to get callbacks for connect and disconnect of a   
> graphical console (like spice and vnc)? How?
[...]
> Such events should lead to calls of brlapi__EnterTtyMode() and
> brlapi__leaveTtyMode().

That would seem to be the way to go indeed, but see below.

> Is it further possible to get any information of the connected
> client to determine the tty, and possibly sub-windows too (see
> brlapi__enterTtyModeWithPath), in which the client is running?

More precisely you would need to know the X window ID on the front-end
side. The VNC and spice protocols don't currently provide this.  Also
when the VM and the frontend are running on different machines this
would not make any sense anyway so I don't think it will get added to
spice & vnc anyway.

One ugly way to get it to work is to run the spice client and keep it
open, get its X window id through xwininfo or equivalent, and only then
start qemu with CONTROLVT set to that id. But of course you can't close
the client and reopen it later.

The way to properly fix it is to add a brlapi channel to spice: in
addition to main, display, inputs, cursor, playback, and record
channels, we would have a brlapi channel. The brlapi protocol is already
packet-based, so that would work nicely. The server in qemu would list
the brlapi channel in addition to others, and brlapi packets would flow
over. The spice client would just forward brlapi packets over without
modification, except for the enterttymode packet, in which it would just
prepend its own windowpath and window id. The forwarding implementation
could be implemented in libbrlapi so that spice clients don't have
to maintain the support. Similarly, we could modify libbrlapi to not
necessarily connect to the usual brlapi socket, but let the application
provide send/recv functions to exchange the packets.

Samuel



Re: [PATCH] MAINTAINERS: slirp: Remove myself as maintainer

2019-11-11 Thread Samuel Thibault
Thomas Huth, le lun. 11 nov. 2019 14:15:36 +0100, a ecrit:
> On 11/11/2019 08.57, Jan Kiszka wrote:
> > May I point out that this one was never merged?
> > 
> > Sorry, I really can't help in this area anymore.
> 
> I'm planning to send a "qtest + misc" PULL request tomorrow ... I can
> add the patch there if Samuel does not have anything else pending
> related to SLIRP right now...

I don't have anything, please add it.

Sorry I didn't realize that I should do more than just ack-ing the
change.

Samuel



Re: [PATCH v4] smb daemon get additional command line parameters from env variable

2019-11-02 Thread Samuel Thibault
Jordi Pujol, le sam. 02 nov. 2019 08:41:52 +0100, a ecrit:
> @@ -909,6 +910,12 @@ static int slirp_smb(SlirpState* s, cons
>   CONFIG_SMBD_COMMAND, s->smb_dir, smb_conf);
>  g_free(smb_conf);
> 
> +options = g_getenv("SMBDOPTIONS");
> +if (options) {
> +smb_cmdline = g_strdup_printf("%s %s", smb_cmdline, options);
> +g_free(options);
> +}

Again, what g_getenv mustn't be freed. I believe you even get a warning
about it: g_getenv returns a const gchar *.

The old value of smb_cmdline, however, has to be freed, otherwise that's
a memory leak.

Samuel



Re: [PATCH v3] smb daemon get additional command line parameters from env variable

2019-11-01 Thread Samuel Thibault
Jordi Pujol, le ven. 01 nov. 2019 15:38:19 +0100, a ecrit:
> +options = g_getenv("SMBDOPTIONS");
> +if (options) {
> +smb_cmdline = g_strdup_printf("%s %s", smb_cmdline, options);
> +}
> +g_free(options);

But then do not free it :)

Samuel



Re: [PATCH v2] smb daemon get additional command line parameters from env variable

2019-10-31 Thread Samuel Thibault
Hello,

Jordi Pujol, le jeu. 31 oct. 2019 14:33:00 +0100, a ecrit:
> The smbd daemon takes additional command line options
> from environment variable SMBDOPTIONS.
> Set the environment variable SMBDOPTIONS before executing qemu.
> 
> Example:
> 
> export SMBDOPTIONS="--option='server min protocol=CORE' -d 4"
> 
> Signed-off-by: Jordi Pujol Palomer 

> ---
> --- qemu-4.1-a/net/slirp.c
> +++ qemu_4.1-b/net/slirp.c
> @@ -909,6 +909,12 @@ static int slirp_smb(SlirpState* s, cons
>   CONFIG_SMBD_COMMAND, s->smb_dir, smb_conf);
>  g_free(smb_conf);
> 
> +char *options = g_strdup(g_getenv("SMBDOPTIONS"));

Why strduping it? you can just use g_getenv.

> +if (options) {
> +smb_cmdline = g_strdup_printf("%s %s", smb_cmdline, options);
> +}
> +g_free(options);
> +
>  if (slirp_add_exec(s->slirp, smb_cmdline, _addr, 139) < 0 ||
>  slirp_add_exec(s->slirp, smb_cmdline, _addr, 445) < 0) {
>  slirp_smb_cleanup(s);
> 

> --- qemu-4.1-a/slirp/src/misc.c 2019-10-29 14:40:15.043120941 +0100
> +++ qemu-4.1-b/slirp/src/misc.c 2019-10-29 14:41:04.440235684 +0100

Please submit this part to https://gitlab.freedesktop.org/slirp/libslirp/

Make sure to note in the changelog that g_shell_parse_argv does only
tokenization, and no replacement.

Samuel

> @@ -168,7 +168,9 @@ g_spawn_async_with_fds_slirp(const gchar
>  int fork_exec(struct socket *so, const char *ex)
>  {
>  GError *err = NULL;
> -char **argv;
> +gint argc = 0;
> +gchar **argv = NULL;
> +gboolean ret;
>  int opt, sp[2];
> 
>  DEBUG_CALL("fork_exec");
> @@ -179,7 +181,13 @@ int fork_exec(struct socket *so, const c
>  return 0;
>  }
> 
> -argv = g_strsplit(ex, " ", -1);
> +ret = g_shell_parse_argv(ex, , , );
> +if (err) {
> +g_critical("fork_exec invalid command: %s", err->message);
> +g_error_free(err);
> +return 0;
> +}
> +
>  g_spawn_async_with_fds(NULL /* cwd */, argv, NULL /* env */,
> G_SPAWN_SEARCH_PATH, fork_exec_child_setup,
> NULL /* data */, NULL /* child_pid */, sp[1], 
> sp[1],
> **
> 
> Thanks,
> 
> Jordi Pujol
> 



Re: Python 2 and test/vm/netbsd

2019-10-22 Thread Samuel Thibault
Eduardo Habkost, le ven. 18 oct. 2019 13:41:43 -0300, a ecrit:
> On Fri, Oct 18, 2019 at 06:00:19PM +0200, Samuel Thibault wrote:
> > It was implemented at the time of introduction of IPv6 in SLIRP. Perhaps
> > NetBSD has a slightly different behavior which makes the implementation
> > fail to notice the error.
> 
> If anybody is interested in investigating it, a network traffic
> dump generated by `-object filter-dump` is attached.

The dump does show the Destination Unreachable icmp message, so it seems
it's the guest which does not notice it for some reason.

Samuel



Re: Python 2 and test/vm/netbsd

2019-10-22 Thread Samuel Thibault
Samuel Thibault, le ven. 18 oct. 2019 18:00:19 +0200, a ecrit:
> Philippe Mathieu-Daudé, le ven. 18 oct. 2019 16:58:00 +0200, a ecrit:
> > On 10/18/19 4:29 PM, Eduardo Habkost wrote:
> > > In addition to that, the connect() error should be generating a
> > > ICMP6_UNREACH message, and I'd expect the NetBSD guest to notice
> > > it instead of waiting for timeout.
> > 
> > Is this missing in SLiRP?
> 
> It was implemented at the time of introduction of IPv6 in SLIRP. Perhaps
> NetBSD has a slightly different behavior which makes the implementation
> fail to notice the error.

It definitely is there in tcp_input(): on tcp_fconnect() error an
ICMP6_UNREACH message is sent.  I can confirm that this works with a
Linux host and Linux guest.

Samuel



Re: Python 2 and test/vm/netbsd

2019-10-18 Thread Samuel Thibault
Philippe Mathieu-Daudé, le ven. 18 oct. 2019 16:58:00 +0200, a ecrit:
> On 10/18/19 4:29 PM, Eduardo Habkost wrote:
> > On Fri, Oct 18, 2019 at 12:44:39PM +0200, Gerd Hoffmann wrote:
> > >Hi,
> > > 
> > > > > Running with V=1, I see packages being downloaded at reasonable 
> > > > > speeds, but
> > > > > there's a huge interval (of various minutes) between each package 
> > > > > download.
> > > > 
> > > > I've found the cause for the slowness I'm seeing: for each file
> > > > being downloaded, the guest spents at least 75 seconds trying to
> > > > connect to the IPv6 address of ftp.NetBSD.org, before trying
> > > > IPv4.
> > > 
> > > Ah, that nicely explains why it worked just fine for me.  First, I have
> > > a local proxy configured so the installer isn't going to connect to
> > > ftp.NetBSD.org directly.  Second I have IPv6 connectivity.
> > > 
> > > > I don't know if this is a NetBSD bug, or a slirp bug.
> > > 
> > > Both I'd say ...
> > > 
> > > First, by default slirp should not send IPv6 router announcements
> > > to the user network if the host has no IPv6 connectivity.
> > > 
> > > Second, the recommended way to connect is to try ipv4 and ipv6 in
> > > parallel, then use whatever connects first.  Web browsers typically
> > > do it that way.  wget and curl don't do that though, they try one
> > > address after the other, and I guess this is where the delay comes
> > > from ...
> > 
> > In addition to that, the connect() error should be generating a
> > ICMP6_UNREACH message, and I'd expect the NetBSD guest to notice
> > it instead of waiting for timeout.
> 
> Is this missing in SLiRP?

It was implemented at the time of introduction of IPv6 in SLIRP. Perhaps
NetBSD has a slightly different behavior which makes the implementation
fail to notice the error.

Samuel



Re: [PATCH v2 2/2] curses: correctly pass the color pair to setcchar()

2019-10-13 Thread Samuel Thibault
Matthew Kilgore, le jeu. 03 oct. 2019 23:53:38 -0400, a ecrit:
> The current code does not correctly pass the color pair information to
> setcchar(), it instead always passes zero. This results in the curses
> output always being in white on black.
> 
> This patch fixes this by using PAIR_NUMBER() to retrieve the color pair
> number from the chtype value, and then passes that value as an argument
> to setcchar().
> 
> Signed-off-by: Matthew Kilgore 

Reviewed-by: Samuel Thibault 
Tested-by: Samuel Thibault 

Thanks!

> ---
>  ui/curses.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/ui/curses.c b/ui/curses.c
> index 84003f56a323..3a1b71451c93 100644
> --- a/ui/curses.c
> +++ b/ui/curses.c
> @@ -77,12 +77,14 @@ static void curses_update(DisplayChangeListener *dcl,
>  for (x = 0; x < width; x++) {
>  chtype ch = line[x] & A_CHARTEXT;
>  chtype at = line[x] & A_ATTRIBUTES;
> +short color_pair = PAIR_NUMBER(line[x]);
> +
>  ret = getcchar(_to_curses[ch], wch, , , NULL);
>  if (ret == ERR || wch[0] == 0) {
>  wch[0] = ch;
>  wch[1] = 0;
>  }
> -setcchar(_line[x], wch, at, 0, NULL);
> +setcchar(_line[x], wch, at, color_pair, NULL);
>  }
>  mvwadd_wchnstr(screenpad, y, 0, curses_line, width);
>  }
> -- 
> 2.23.0
> 

-- 
Samuel
 bash: ls: Computer bought the farm
 THAT frightens ppl! :P
 id rather see: "bash: ls: Initialization of googol(AWAX)
disengaged in HYPER32/64 mode due to faulty page request at
AX:12A34F84B"
 at least that would give me the feeling that the
*programmers* knows what is going on :P
(lovely Hurd...)



Re: [PATCH v2 1/2] curses: use the bit mask constants provided by curses

2019-10-13 Thread Samuel Thibault
Matthew Kilgore, le jeu. 03 oct. 2019 23:53:37 -0400, a ecrit:
> The curses API provides the A_ATTRIBUTES and A_CHARTEXT bit masks for
> getting the attributes and character parts of a chtype, respectively. We
> should use provided constants instead of using 0xff.
> 
> Signed-off-by: Matthew Kilgore 

Reviewed-by: Samuel Thibault 
Tested-by: Samuel Thibault 

> ---
>  ui/curses.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/ui/curses.c b/ui/curses.c
> index ec281125acbd..84003f56a323 100644
> --- a/ui/curses.c
> +++ b/ui/curses.c
> @@ -75,8 +75,8 @@ static void curses_update(DisplayChangeListener *dcl,
>  line = screen + y * width;
>  for (h += y; y < h; y ++, line += width) {
>  for (x = 0; x < width; x++) {
> -chtype ch = line[x] & 0xff;
> -chtype at = line[x] & ~0xff;
> +chtype ch = line[x] & A_CHARTEXT;
> +chtype at = line[x] & A_ATTRIBUTES;
>  ret = getcchar(_to_curses[ch], wch, , , NULL);
>  if (ret == ERR || wch[0] == 0) {
>  wch[0] = ch;
> -- 
> 2.23.0
> 

-- 
Samuel
The nice thing about Windows is - It does not just crash, it displays a
dialog box and lets you press 'OK' first.
(Arno Schaefer's .sig)



[PULL 0/1] slirp update

2019-10-06 Thread Samuel Thibault
The following changes since commit 860d9048c78ce59c5903c3d5209df56f38400986:

  Merge remote-tracking branch 
'remotes/kraxel/tags/audio-20190924-pull-request' into staging (2019-09-24 
13:51:51 +0100)

are available in the Git repository at:

  https://people.debian.org/~sthibault/qemu.git tags/samuel-thibault

for you to fetch changes up to 120b721f5b590393971673a315f02969ec89cccb:

  slirp: Allow non-local DNS address when restrict is off (2019-10-01 19:03:08 
+0200)


slirp: Allow non-local DNS address when restrict is off


Samuel Thibault (1):
  slirp: Allow non-local DNS address when restrict is off

 net/slirp.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)



[PULL 1/1] slirp: Allow non-local DNS address when restrict is off

2019-10-06 Thread Samuel Thibault
This can be used to set a DNS server to be used by the guest which is
different from the one configured on the host.

Buglink: https://bugs.launchpad.net/qemu/+bug/1010484
Signed-off-by: Samuel Thibault 
Reviewed-by: Philippe Mathieu-Daudé 
Reviewed-by: Thomas Huth 
---
 net/slirp.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/slirp.c b/net/slirp.c
index f42f496641..c4334ee876 100644
--- a/net/slirp.c
+++ b/net/slirp.c
@@ -456,7 +456,7 @@ static int net_slirp_init(NetClientState *peer, const char 
*model,
 error_setg(errp, "Failed to parse DNS");
 return -1;
 }
-if ((dns.s_addr & mask.s_addr) != net.s_addr) {
+if (restricted && (dns.s_addr & mask.s_addr) != net.s_addr) {
 error_setg(errp, "DNS doesn't belong to network");
 return -1;
 }
@@ -522,7 +522,7 @@ static int net_slirp_init(NetClientState *peer, const char 
*model,
 error_setg(errp, "Failed to parse IPv6 DNS");
 return -1;
 }
-if (!in6_equal_net(_prefix, _dns, vprefix6_len)) {
+if (restricted && !in6_equal_net(_prefix, _dns, vprefix6_len)) 
{
 error_setg(errp, "IPv6 DNS doesn't belong to network");
 return -1;
 }
-- 
2.23.0




[PATCHv2] slirp: Allow non-local DNS address when restrict is off

2019-10-01 Thread Samuel Thibault
This can be used to set a DNS server to be used by the guest which is
different from the one configured on the host.

Buglink: https://bugs.launchpad.net/qemu/+bug/1010484
Signed-off-by: Samuel Thibault 
---
Difference from first version:
- handle DNS IPv6 as well
- reference bug with Buglink

 net/slirp.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/slirp.c b/net/slirp.c
index f42f496641..c4334ee876 100644
--- a/net/slirp.c
+++ b/net/slirp.c
@@ -456,7 +456,7 @@ static int net_slirp_init(NetClientState *peer, const char 
*model,
 error_setg(errp, "Failed to parse DNS");
 return -1;
 }
-if ((dns.s_addr & mask.s_addr) != net.s_addr) {
+if (restricted && (dns.s_addr & mask.s_addr) != net.s_addr) {
 error_setg(errp, "DNS doesn't belong to network");
 return -1;
 }
@@ -522,7 +522,7 @@ static int net_slirp_init(NetClientState *peer, const char 
*model,
 error_setg(errp, "Failed to parse IPv6 DNS");
 return -1;
 }
-if (!in6_equal_net(_prefix, _dns, vprefix6_len)) {
+if (restricted && !in6_equal_net(_prefix, _dns, vprefix6_len)) 
{
 error_setg(errp, "IPv6 DNS doesn't belong to network");
 return -1;
 }
-- 
2.23.0




[PATCH] slirp: Allow non-local DNS address when restrict is off

2019-10-01 Thread Samuel Thibault
This can be used to set a DNS server to be used by the guest which is
different from the one configured on the host.

Buglink: https://bugs.launchpad.net/qemu/+bug/1010484
Signed-off-by: Samuel Thibault 
---
 net/slirp.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/slirp.c b/net/slirp.c
index f42f496641..c4334ee876 100644
--- a/net/slirp.c
+++ b/net/slirp.c
@@ -456,7 +456,7 @@ static int net_slirp_init(NetClientState *peer, const char 
*model,
 error_setg(errp, "Failed to parse DNS");
 return -1;
 }
-if ((dns.s_addr & mask.s_addr) != net.s_addr) {
+if (restricted && (dns.s_addr & mask.s_addr) != net.s_addr) {
 error_setg(errp, "DNS doesn't belong to network");
 return -1;
 }
@@ -522,7 +522,7 @@ static int net_slirp_init(NetClientState *peer, const char 
*model,
 error_setg(errp, "Failed to parse IPv6 DNS");
 return -1;
 }
-if (!in6_equal_net(_prefix, _dns, vprefix6_len)) {
+if (restricted && !in6_equal_net(_prefix, _dns, vprefix6_len)) 
{
 error_setg(errp, "IPv6 DNS doesn't belong to network");
 return -1;
 }
-- 
2.23.0




[PATCH] slirp: Allow non-local DNS address when restrict is off

2019-09-29 Thread Samuel Thibault
This can be used to set a DNS server to be used by the guest which is
different from the one configured on the host.

This fixes LP 1010484.

Signed-off-by: Samuel Thibault 
---
 net/slirp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/slirp.c b/net/slirp.c
index f42f496641..4d158b0542 100644
--- a/net/slirp.c
+++ b/net/slirp.c
@@ -456,7 +456,7 @@ static int net_slirp_init(NetClientState *peer, const char 
*model,
 error_setg(errp, "Failed to parse DNS");
 return -1;
 }
-if ((dns.s_addr & mask.s_addr) != net.s_addr) {
+if (restricted && (dns.s_addr & mask.s_addr) != net.s_addr) {
 error_setg(errp, "DNS doesn't belong to network");
 return -1;
 }
-- 
2.23.0




Re: [Qemu-devel] slirp, incoming packets get truncated

2019-09-09 Thread Samuel Thibault
Hello,

Philippe Mathieu-Daudé, le lun. 09 sept. 2019 12:51:23 +0200, a ecrit:
> Anyhow, if you plan to properly (with your S-o-b) commit your patch to
> the libslirp repository,

Actually the libslirp repository already has something there. Since
IPv4/6 have a maximum of 64KB packets, it just allocates that size
on the stack, whîch is simpler in the end. Also it makes mru/mtu
configurable, we will be able to plug that into qemu once released.

Samuel



Re: [Qemu-devel] slirp, incoming packets get truncated

2019-09-07 Thread Samuel Thibault
Hello,

As usual, several things here.

Chris Heinze, le mar. 03 sept. 2019 17:02:15 +0200, a ecrit:
> root@guest:~# tcpdump -ni eth0 port 19003
> tcpdump: verbose output suppressed, use -v or -vv for full protocol decode
> listening on eth0, link-type EN10MB (Ethernet), capture size 262144 bytes
> 16:49:39.430959 IP 10.0.2.2.33294 > 10.0.2.15.19003: UDP, bad length 9000 > 
> 1472

tcpdump seems to be showing dumb output here. The packet is fragmented
by slirp, which makes tcpdump confused and show only the first
fragment. If you let tcpdump print everything, you will see the other
fragments. In reality, everything is going fine here.

> i tried to change slirp/src/if.h to: 
> 
> #define IF_MTU 9000
> #define IF_MRU 9000
> 
> but the resulting qemu-system-x86_64 binary did not behave differently.

Did you explicitly remove the qemu-system-x86_64 binary? As mentioned
previously on the list, there seems to be a missing Makefile dependency.

Now, with MTU set to 9000, the packets just don't go at all. Could you
try the attached patch? The lowest layer of slirp was indeed limited to
1600-byte frames for no good reason. With this and the virtio driver, I
could exchange 9000-byte packets.

Samuel
diff --git a/src/slirp.c b/src/slirp.c
index b0194cb..3fd6f68 100644
--- a/src/slirp.c
+++ b/src/slirp.c
@@ -890,20 +890,22 @@ static int if_encap6(Slirp *slirp, struct mbuf *ifm, 
struct ethhdr *eh,
  */
 int if_encap(Slirp *slirp, struct mbuf *ifm)
 {
-uint8_t buf[1600];
-struct ethhdr *eh = (struct ethhdr *)buf;
+uint8_t *buf;
+struct ethhdr *eh;
 uint8_t ethaddr[ETH_ALEN];
 const struct ip *iph = (const struct ip *)ifm->m_data;
 int ret;
 
-if (ifm->m_len + ETH_HLEN > sizeof(buf)) {
-return 1;
-}
+buf = g_malloc(ifm->m_len + ETH_HLEN);
+if (!buf)
+return 0;
+eh = (struct ethhdr *)buf;
 
 switch (iph->ip_v) {
 case IPVERSION:
 ret = if_encap4(slirp, ifm, eh, ethaddr);
 if (ret < 2) {
+g_free(buf);
 return ret;
 }
 break;
@@ -911,6 +913,7 @@ int if_encap(Slirp *slirp, struct mbuf *ifm)
 case IP6VERSION:
 ret = if_encap6(slirp, ifm, eh, ethaddr);
 if (ret < 2) {
+g_free(buf);
 return ret;
 }
 break;
@@ -929,6 +932,7 @@ int if_encap(Slirp *slirp, struct mbuf *ifm)
   eh->h_dest[5]);
 memcpy(buf + sizeof(struct ethhdr), ifm->m_data, ifm->m_len);
 slirp_send_packet_all(slirp, buf, ifm->m_len + ETH_HLEN);
+g_free(buf);
 return 1;
 }
 


Re: [Qemu-devel] slirp, incoming packets get truncated

2019-09-06 Thread Samuel Thibault
Chris Heinze, le ven. 06 sept. 2019 12:54:24 +0200, a ecrit:
> i'm not aware of any (canonical) parameters to set a max size for received 
> packets

Ah, right.

> i found no way to configure slirp (in qemu) itself except for the DEFINEs in 
> the if.h.

Yes, that's where it is to be configured atm. IF_MRU is however only
used for setting the TCP mss, there is no code that is supposed to
truncate packets etc., that's why I'm really surprised that there is
such truncation happening.

Samuel



Re: [Qemu-devel] slirp, incoming packets get truncated

2019-09-06 Thread Samuel Thibault
Hello,

Chris Heinze, le mar. 03 sept. 2019 17:02:15 +0200, a ecrit:
> on the guest:
> root@guest:~# tcpdump -ni eth0 port 19003
> tcpdump: verbose output suppressed, use -v or -vv for full protocol decode
> listening on eth0, link-type EN10MB (Ethernet), capture size 262144 bytes
> 16:49:39.430959 IP 10.0.2.2.33294 > 10.0.2.15.19003: UDP, bad length 9000 > 
> 1472

Just to be sure: did you configure your guest mru to 9000 ? Does the
emulated network hardware card support jumbo frames?

Samuel



[Qemu-devel] [Bug 1837094] Re: UndefinedBehaviorSanitizer crash around slirp::ip_reass()

2019-08-29 Thread Samuel thibault
And

https://gitlab.freedesktop.org/slirp/libslirp/commit/d203c81b

-- 
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1837094

Title:
  UndefinedBehaviorSanitizer crash around slirp::ip_reass()

Status in QEMU:
  New

Bug description:
  tag: v4.1.0-rc1

  ./configure --enable-sanitizers --extra-cflags=-O1

  ==26130==ERROR: UndefinedBehaviorSanitizer: SEGV on unknown address 
0x0008 (pc 0x561ad346d588 bp 0x7fff6ee9f940 sp 0x7fff6ee9f8e8 
T26130)
  ==26130==The signal is caused by a WRITE memory access.
  ==26130==Hint: address points to the zero page.
  #0 0x561ad346d587 in ip_deq() at slirp/src/ip_input.c:411:55
  #1 0x561ad346cffb in ip_reass() at slirp/src/ip_input.c:304:9
  #2 0x561ad346cb6f in ip_input() at slirp/src/ip_input.c:184:18

  I only had access to the last packet which isn't the culprit, I'm now
  seeing how to log the network traffic of the guest to provide more
  useful information.

To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/1837094/+subscriptions



Re: [Qemu-devel] [Slirp] [PATCH 1/2] Do not reassemble fragments pointing outside of the original payload

2019-08-25 Thread Samuel Thibault
Hello,

Philippe Mathieu-Daudé, le ven. 23 août 2019 17:15:32 +0200, a ecrit:
> > Did you make your test with commit 126c04acbabd ("Fix heap overflow in
> > ip_reass on big packet input") applied?
> 
> Yes, unfortunately it doesn't fix the issue.

Ok.

Could you try the attached patch?  There was a use-after-free.  Without
it, I can indeed crash qemu with the given exploit.  With it I don't
seem to be able to crash it (trying in a loop for several minutes).

Samuel
diff --git a/src/ip_input.c b/src/ip_input.c
index 7364ce0..aa514ae 100644
--- a/src/ip_input.c
+++ b/src/ip_input.c
@@ -292,6 +292,7 @@ static struct ip *ip_reass(Slirp *slirp, struct ip *ip, 
struct ipq *fp)
  */
 while (q != (struct ipasfrag *)>frag_link &&
ip->ip_off + ip->ip_len > q->ipf_off) {
+struct ipasfrag *prev;
 i = (ip->ip_off + ip->ip_len) - q->ipf_off;
 if (i < q->ipf_len) {
 q->ipf_len -= i;
@@ -299,9 +300,10 @@ static struct ip *ip_reass(Slirp *slirp, struct ip *ip, 
struct ipq *fp)
 m_adj(dtom(slirp, q), i);
 break;
 }
+prev = q;
 q = q->ipf_next;
-m_free(dtom(slirp, q->ipf_prev));
-ip_deq(q->ipf_prev);
+ip_deq(prev);
+m_free(dtom(slirp, prev));
 }
 
 insert:


Re: [Qemu-devel] [Slirp] [PATCH 1/2] Do not reassemble fragments pointing outside of the original payload

2019-08-22 Thread Samuel Thibault
Hello,

Philippe Mathieu-Daudé, le jeu. 22 août 2019 16:41:33 +0200, a ecrit:
>   Later the newly calculated pointer q is converted into ip structure
>   and values are modified, Due to the wrong calculation of the delta,
>   ip will be pointing to incorrect location and ip_src and ip_dst can
>   be used to write controlled data onto the calculated location. This
>   may also crash qemu if the calculated ip is located in unmaped area.

That does not seem to be related to this:

> Do not queue fragments pointing out of the original payload to avoid
> to calculate the variable delta.

I don't understand the relation with having to calculate delta.

> diff --git a/src/ip_input.c b/src/ip_input.c
> index 7364ce0..ee52085 100644
> --- a/src/ip_input.c
> +++ b/src/ip_input.c
> @@ -304,6 +304,19 @@ static struct ip *ip_reass(Slirp *slirp, struct ip *ip, 
> struct ipq *fp)
>  ip_deq(q->ipf_prev);
>  }
>  
> +/*
> + * If we received the first fragment, we know the original
> + * payload size.

? We only know the total payload size when receiving the last fragment
(payload = offset*8 + size).

> Verify fragments are within our payload.

By construction of the protocol, fragments can only be within the
payload, since it's the last fragment which provides the payload size.

> +for (q = fp->frag_link.next; q != (struct ipasfrag*)>frag_link;
> +q = q->ipf_next) {
> +if (!q->ipf_off && q->ipf_len) {
> +if (ip->ip_off + ip->ip_len >= q->ipf_len) {
> +goto dropfrag;
> +}
> +}
> +}

Fragments are kept in order, there is no need to go around the list to
find the fragment with offset zero, if it is there it is the first one.

Did you make your test with commit 126c04acbabd ("Fix heap overflow in
ip_reass on big packet input") applied?

Samuel



Re: [Qemu-devel] [ANNOUNCE] QEMU 4.1.0-rc3 is now available

2019-08-02 Thread Samuel Thibault
Marc-André Lureau, le ven. 02 août 2019 15:07:46 +0400, a ecrit:
> And Samuel probably thought the same, since he didn't update the submodule.

I'm rather mostly buried under piles of things to do...

> According to MAINTAINERS, this is for Samuel to take care of. But I'll
> do it if he ask me.

Please do.

Samuel



Re: [Qemu-devel] [PATCH] ui/curses: Fix build with -m32

2019-05-27 Thread Samuel Thibault
Max Reitz, le lun. 27 mai 2019 16:25:40 +0200, a ecrit:
> wchar_t may resolve to be an unsigned long on 32-bit architectures.
> Using the %x conversion specifier will then give a compiler warning:
> 
> ui/curses.c: In function ‘get_ucs’:
> ui/curses.c:492:49: error: format ‘%x’ expects argument of type ‘unsigned 
> int’, but argument 3 has type ‘wchar_t’ {aka ‘long int’} [-Werror=format=]
>   492 | fprintf(stderr, "Could not convert 0x%04x "
>   |  ~~~^
>   | |
>   | unsigned int
>   |  %04lx
>   493 | "from wchar_t to a multibyte character: %s\n",
>   494 | wch, strerror(errno));
>   | ~~~
>   | |
>   | wchar_t {aka long int}
> ui/curses.c:504:49: error: format ‘%x’ expects argument of type ‘unsigned 
> int’, but argument 3 has type ‘wchar_t’ {aka ‘long int’} [-Werror=format=]
>   504 | fprintf(stderr, "Could not convert 0x%04x "
>   |  ~~~^
>   | |
>   | unsigned int
>   |  %04lx
>   505 | "from a multibyte character to UCS-2 : %s\n",
>   506 | wch, strerror(errno));
>   | ~~~
>   | |
>   | wchar_t {aka long int}
> 
> Fix this by casting the wchar_t value to an unsigned long and using %lx
> as the conversion specifier.
> 
> Fixes: b7b664a4fe9a955338f2e11a0f7433b29c8cbad0
> Signed-off-by: Max Reitz 

Reviewed-by: Samuel Thibault 

> ---
>  ui/curses.c | 8 
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/ui/curses.c b/ui/curses.c
> index 1f3fcabb00..e9319eb8ae 100644
> --- a/ui/curses.c
> +++ b/ui/curses.c
> @@ -489,9 +489,9 @@ static uint16_t get_ucs(wchar_t wch, iconv_t conv)
>  memset(, 0, sizeof(ps));
>  ret = wcrtomb(mbch, wch, );
>  if (ret == -1) {
> -fprintf(stderr, "Could not convert 0x%04x "
> +fprintf(stderr, "Could not convert 0x%04lx "
>  "from wchar_t to a multibyte character: %s\n",
> -wch, strerror(errno));
> +(unsigned long)wch, strerror(errno));
>  return 0xFFFD;
>  }
>  
> @@ -501,9 +501,9 @@ static uint16_t get_ucs(wchar_t wch, iconv_t conv)
>  such = sizeof(uch);
>  
>  if (iconv(conv, , , , ) == (size_t) -1) {
> -fprintf(stderr, "Could not convert 0x%04x "
> +fprintf(stderr, "Could not convert 0x%04lx "
>  "from a multibyte character to UCS-2 : %s\n",
> -wch, strerror(errno));
> +(unsigned long)wch, strerror(errno));
>  return 0xFFFD;
>  }
>  
> -- 
> 2.21.0
> 
> 

-- 
Samuel
/*
 * [...] Note that 120 sec is defined in the protocol as the maximum
 * possible RTT.  I guess we'll have to use something other than TCP
 * to talk to the University of Mars.
 * PAWS allows us longer timeouts and large windows, so once implemented
 * ftp to mars will work nicely.
 */
(from /usr/src/linux/net/inet/tcp.c, concerning RTT [retransmission timeout])



[Qemu-devel] [PULL 0/1] Update upstream slirp

2019-05-09 Thread Samuel Thibault
The following changes since commit 30302acee710881cb248ec3391adcd54dcf52396:

  Update upstream slirp (2019-05-09 09:58:57 +0200)

are available in the Git repository at:

  https://people.debian.org/~sthibault/qemu.git tags/samuel-thibault

for you to fetch changes up to 30302acee710881cb248ec3391adcd54dcf52396:

  Update upstream slirp (2019-05-09 09:58:57 +0200)


Update slirp submodule

Samuel Thibault (1):
  Update upstream slirp
  Adds gitignore, README file, and fixes ident protocol parsing.





[Qemu-devel] [PULL 1/1] Update upstream slirp

2019-05-09 Thread Samuel Thibault
Adds gitignore, README file, and fixes ident protocol parsing.

Signed-off-by: Samuel Thibault 
---
 slirp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/slirp b/slirp
index 0e79ba4856..f0da672620 16
--- a/slirp
+++ b/slirp
@@ -1 +1 @@
-Subproject commit 0e79ba48567ccfb3cc2cf2e98cce8811eee7e455
+Subproject commit f0da6726207b740f6101028b2992f918477a4b08
-- 
2.20.1




[Qemu-devel] [PULL 1/1] Update slirp submodule

2019-05-04 Thread Samuel Thibault
To fix Windows on ARM.
---
 slirp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/slirp b/slirp
index 59a1b1f165..0e79ba4856 16
--- a/slirp
+++ b/slirp
@@ -1 +1 @@
-Subproject commit 59a1b1f165458c2acb7ff0525b543945f7416225
+Subproject commit 0e79ba48567ccfb3cc2cf2e98cce8811eee7e455
-- 
2.20.1




[Qemu-devel] [PULL 0/1] Update slirp submodule

2019-05-04 Thread Samuel Thibault
The following changes since commit 52ec9dcc1ed5609674e7b52198c18207bb193548:

  Update slirp submodule (2019-05-04 14:38:05 +0200)

are available in the Git repository at:

  https://people.debian.org/~sthibault/qemu.git tags/samuel-thibault

for you to fetch changes up to 52ec9dcc1ed5609674e7b52198c18207bb193548:

  Update slirp submodule (2019-05-04 14:38:05 +0200)


Update slirp submodule

To fix Windows on ARM.





Re: [Qemu-devel] [PATCH v2 1/4] Initial Windows on ARM (AArch64 64-Bit) host support

2019-05-04 Thread Samuel Thibault
Hello,

Cao Jiaxi, le mer. 01 mai 2019 02:10:09 +0800, a ecrit:
> This series of patches is for initial support of Windows 10 on ARM as a QEMU 
> host.
> Currently only TCG intepreter is working correctly, it crashes when TCG JIT 
> is enabled.
> For now we assume it is built using the clang aarch64-w64-mingw32 toolchain, 
> you can get a prebuilt toolchain at https://github.com/mstorsjo/llvm-mingw.
> 
> QEMU_PACKED: Remove gcc_struct attribute in Windows non x86 targets
> This attribute is for x86 only, and it generates an warning on ARM64 
> Clang/MinGW targets.

I have applied it to the libslirp repo, thanks!

I'll submit a pull request to get it updated on qemu.

Samuel



[Qemu-devel] [PULL 0/2] slirp: move slirp as git submodule project

2019-05-02 Thread Samuel Thibault
The following changes since commit 8482ff2eb3bb95020eb2f370a9b3ea26511e41df:

  Merge remote-tracking branch 'remotes/jnsnow/tags/bitmaps-pull-request' into 
staging (2019-05-02 12:04:51 +0100)

are available in the Git repository at:

  https://people.debian.org/~sthibault/qemu.git tags/samuel-thibault

for you to fetch changes up to 7c57bdd82026ba03f3158bbcd841afde7c2dc43a:

  build-sys: move slirp as git submodule project (2019-05-03 00:15:37 +0200)


slirp: move slirp as git submodule project

Marc-André Lureau (2):
  build-sys: pass CFLAGS & LDFLAGS to subdir-slirp
  build-sys: move slirp as git submodule project


Marc-André Lureau (2):
  build-sys: pass CFLAGS & LDFLAGS to subdir-slirp
  build-sys: move slirp as git submodule project

 .gitmodules   |3 +
 Makefile  |2 +-
 configure |   11 +-
 scripts/archive-source.sh |2 +-
 slirp |1 +
 slirp/COPYRIGHT   |   62 --
 slirp/Makefile|   47 --
 slirp/src/arp_table.c |   92 ---
 slirp/src/bootp.c |  371 ---
 slirp/src/bootp.h |  129 
 slirp/src/cksum.c |  161 -
 slirp/src/debug.h |   46 --
 slirp/src/dhcpv6.c|  224 ---
 slirp/src/dhcpv6.h|   53 --
 slirp/src/dnssearch.c |  311 -
 slirp/src/if.c|  218 ---
 slirp/src/if.h|   21 -
 slirp/src/ip.h|  242 ---
 slirp/src/ip6.h   |  160 -
 slirp/src/ip6_icmp.c  |  438 -
 slirp/src/ip6_icmp.h  |  232 ---
 slirp/src/ip6_input.c |   78 ---
 slirp/src/ip6_output.c|   39 --
 slirp/src/ip_icmp.c   |  470 --
 slirp/src/ip_icmp.h   |  166 -
 slirp/src/ip_input.c  |  469 --
 slirp/src/ip_output.c |  170 -
 slirp/src/libslirp.h  |  118 
 slirp/src/main.h  |   16 -
 slirp/src/mbuf.c  |  235 ---
 slirp/src/mbuf.h  |  127 
 slirp/src/misc.c  |  321 --
 slirp/src/misc.h  |   66 --
 slirp/src/ncsi-pkt.h  |  445 -
 slirp/src/ncsi.c  |  194 --
 slirp/src/ndp_table.c |   87 ---
 slirp/src/qtailq.h|  194 --
 slirp/src/sbuf.c  |  186 --
 slirp/src/sbuf.h  |   27 -
 slirp/src/slirp.c | 1118 
 slirp/src/slirp.h |  275 
 slirp/src/socket.c|  945 ---
 slirp/src/socket.h|  160 -
 slirp/src/state.c |  388 ---
 slirp/src/stream.c|  120 
 slirp/src/stream.h|   35 -
 slirp/src/tcp.h   |  181 --
 slirp/src/tcp_input.c | 1554 -
 slirp/src/tcp_output.c|  522 ---
 slirp/src/tcp_subr.c  |  987 
 slirp/src/tcp_timer.c |  294 -
 slirp/src/tcp_timer.h |  128 
 slirp/src/tcp_var.h   |  162 -
 slirp/src/tcpip.h |  102 ---
 slirp/src/tftp.c  |  463 --
 slirp/src/tftp.h  |   52 --
 slirp/src/udp.c   |  363 ---
 slirp/src/udp.h   |   92 ---
 slirp/src/udp6.c  |  173 -
 slirp/src/util.c  |  368 ---
 slirp/src/util.h  |  175 -
 slirp/src/vmstate.c   |  441 -
 slirp/src/vmstate.h   |  409 
 63 files changed, 15 insertions(+), 15726 deletions(-)
 create mode 16 slirp
 delete mode 100644 slirp/COPYRIGHT
 delete mode 100644 slirp/Makefile
 delete mode 100644 slirp/src/arp_table.c
 delete mode 100644 slirp/src/bootp.c
 delete mode 100644 slirp/src/bootp.h
 delete mode 100644 slirp/src/cksum.c
 delete mode 100644 slirp/src/debug.h
 delete mode 100644 slirp/src/dhcpv6.c
 delete mode 100644 slirp/src/dhcpv6.h
 delete mode 100644 slirp/src/dnssearch.c
 delete mode 100644 slirp/src/if.c
 delete mode 100644 slirp/src/if.h
 delete mode 100644 slirp/src/ip.h
 delete mode 100644 slirp/src/ip6.h
 delete mode 100644 slirp/src/ip6_icmp.c
 delete mode 100644 slirp/src/ip6_icmp.h
 delete mode 100644 slirp/src/ip6_input.c
 delete mode 100644 slirp/src/ip6_output.c
 delete mode 100644 slirp/src/ip_icmp.c
 delete mode 100644 slirp/src/ip_icmp.h
 delete mode 100644 slirp/src/ip_input.c
 delete mode 100644 slirp/src/ip_output.c
 delete mode 100644 slirp/src/libslirp.h
 delete mode 100644 slirp/src/main.h
 delete mode 100644 slirp/src/mbuf.c
 delete mode 100644 slirp/src/mbuf.h
 delete mode 100644 slirp/src/misc.c
 delete mode 100644 slirp/src/misc.h
 delete mode 100644 slirp/src/ncsi-pkt.h
 delete mode 100644 slirp/src/ncsi.c
 delete mode 100644 slirp/src/ndp_table.c
 delete mode 100644 slirp/src/qtailq.h
 delete mode 100644 slirp/src/sbuf.c
 delete mode 100644 slirp/src/sbuf.h
 delete mode 1006

  1   2   3   4   5   6   7   8   9   10   >