[PATCH v3 0/2] Missing endianness conversions in user mode

[PATCH v3 0/2] Missing endianness conversions in user mode

[Mathis Marion]

From: Mathis Marion <mathis.marion@silabs.com>

For a bit of context, I was trying to test a network border router [1]
daemon using the MIPS architecture (see [2]). I didn't have access to
real MIPS hardware so I figured I would emulate it using QEMU user mode.
I ran into a couple of problems all related to endianness conversion for
syscalls between host and target as MIPS is big endian and my x86 host
is little.

[1]: https://github.com/SiliconLabs/wisun-br-linux
[2]: https://github.com/SiliconLabs/wisun-br-linux/issues/5

v3:
- remove patches "linux-user: fix timerfd read endianness conversion"
  and "linux-user: add target to host netlink conversions" as they were
  integrated.
- remove sin6_flowinfo endianness conversion
- use NLA_TYPE_MASK instead of ~NLA_F_NESTED

v2:
- remove context from target_to_host_for_each_nlattr()

Mathis Marion (2):
  linux-user: fix sockaddr_in6 endianness
  linux-user: handle netlink flag NLA_F_NESTED

 linux-user/fd-trans.c | 4 ++--
 linux-user/syscall.c  | 5 +++++
 2 files changed, 7 insertions(+), 2 deletions(-)

-- 
2.39.2

[PATCH v3 1/2] linux-user: fix sockaddr_in6 endianness

[Mathis Marion]

From: Mathis Marion <mathis.marion@silabs.com>

The sin6_scope_id field uses the host byte order, so there is a
conversion to be made when host and target endianness differ.

Signed-off-by: Mathis Marion <mathis.marion@silabs.com>
---
 linux-user/syscall.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 247e20572d..bafa77d353 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -1713,6 +1713,11 @@ static inline abi_long target_to_host_sockaddr(int fd, struct sockaddr *addr,
 	lladdr = (struct target_sockaddr_ll *)addr;
 	lladdr->sll_ifindex = tswap32(lladdr->sll_ifindex);
 	lladdr->sll_hatype = tswap16(lladdr->sll_hatype);
+    } else if (sa_family == AF_INET6) {
+        struct sockaddr_in6 *in6addr;
+
+        in6addr = (struct sockaddr_in6 *)addr;
+        in6addr->sin6_scope_id = tswap32(in6addr->sin6_scope_id);
     }
     unlock_user(target_saddr, target_addr, 0);
 
-- 
2.39.2

Re: [PATCH v3 1/2] linux-user: fix sockaddr_in6 endianness

[Laurent Vivier]

Le 07/03/2023 à 16:42, Mathis Marion a écrit :
> From: Mathis Marion <mathis.marion@silabs.com>
> 
> The sin6_scope_id field uses the host byte order, so there is a
> conversion to be made when host and target endianness differ.
> 
> Signed-off-by: Mathis Marion <mathis.marion@silabs.com>
> ---
>   linux-user/syscall.c | 5 +++++
>   1 file changed, 5 insertions(+)
> 
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index 247e20572d..bafa77d353 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -1713,6 +1713,11 @@ static inline abi_long target_to_host_sockaddr(int fd, struct sockaddr *addr,
>   	lladdr = (struct target_sockaddr_ll *)addr;
>   	lladdr->sll_ifindex = tswap32(lladdr->sll_ifindex);
>   	lladdr->sll_hatype = tswap16(lladdr->sll_hatype);
> +    } else if (sa_family == AF_INET6) {
> +        struct sockaddr_in6 *in6addr;
> +
> +        in6addr = (struct sockaddr_in6 *)addr;
> +        in6addr->sin6_scope_id = tswap32(in6addr->sin6_scope_id);
>       }
>       unlock_user(target_saddr, target_addr, 0);
>   

Reviewed-by: Laurent Vivier <laurent@vivier.eu>

Re: [PATCH v3 1/2] linux-user: fix sockaddr_in6 endianness

[Laurent Vivier]

Le 07/03/2023 à 16:42, Mathis Marion a écrit :
> From: Mathis Marion <mathis.marion@silabs.com>
> 
> The sin6_scope_id field uses the host byte order, so there is a
> conversion to be made when host and target endianness differ.
> 
> Signed-off-by: Mathis Marion <mathis.marion@silabs.com>
> ---
>   linux-user/syscall.c | 5 +++++
>   1 file changed, 5 insertions(+)
> 
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index 247e20572d..bafa77d353 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -1713,6 +1713,11 @@ static inline abi_long target_to_host_sockaddr(int fd, struct sockaddr *addr,
>   	lladdr = (struct target_sockaddr_ll *)addr;
>   	lladdr->sll_ifindex = tswap32(lladdr->sll_ifindex);
>   	lladdr->sll_hatype = tswap16(lladdr->sll_hatype);
> +    } else if (sa_family == AF_INET6) {
> +        struct sockaddr_in6 *in6addr;
> +
> +        in6addr = (struct sockaddr_in6 *)addr;
> +        in6addr->sin6_scope_id = tswap32(in6addr->sin6_scope_id);
>       }
>       unlock_user(target_saddr, target_addr, 0);
>   

Applied to my linux-user-for-8.0 branch.

Thanks,
Laurent

Re: [PATCH v3 1/2] linux-user: fix sockaddr_in6 endianness

[Philippe Mathieu-Daudé]

On 7/3/23 16:42, Mathis Marion wrote:
> From: Mathis Marion <mathis.marion@silabs.com>
> 
> The sin6_scope_id field uses the host byte order, so there is a
> conversion to be made when host and target endianness differ.
> 
> Signed-off-by: Mathis Marion <mathis.marion@silabs.com>
> ---
>   linux-user/syscall.c | 5 +++++
>   1 file changed, 5 insertions(+)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

[PATCH v3 2/2] linux-user: handle netlink flag NLA_F_NESTED

[Mathis Marion]

From: Mathis Marion <mathis.marion@silabs.com>

Newer kernel versions require this flag to be present contrary to older
ones. Depending on the libnl version it is added or not.

Typically when using rtnl_link_inet6_set_addr_gen_mode, the netlink
packet generated may contain the following attribute:

with libnl 3.4

  {nla_len=16, nla_type=IFLA_AF_SPEC},
  [
    {nla_len=12, nla_type=AF_INET6},
    [{nla_len=5, nla_type=IFLA_INET6_ADDR_GEN_MODE}, IN6_ADDR_GEN_MODE_NONE]
  ]

with libnl 3.7

  {nla_len=16, nla_type=NLA_F_NESTED|IFLA_AF_SPEC},
  [
    {nla_len=12, nla_type=NLA_F_NESTED|AF_INET6},
    [{nla_len=5, nla_type=IFLA_INET6_ADDR_GEN_MODE}, IN6_ADDR_GEN_MODE_NONE]]
  ]

Masking the type is likely needed in other places. Only the above cases
are implemented in this patch.

Signed-off-by: Mathis Marion <mathis.marion@silabs.com>
---
 linux-user/fd-trans.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/linux-user/fd-trans.c b/linux-user/fd-trans.c
index 4852a75d9d..c04a97c73a 100644
--- a/linux-user/fd-trans.c
+++ b/linux-user/fd-trans.c
@@ -1359,7 +1359,7 @@ static abi_long target_to_host_for_each_rtattr(struct rtattr *rtattr,
 
 static abi_long target_to_host_data_spec_nlattr(struct nlattr *nlattr)
 {
-    switch (nlattr->nla_type) {
+    switch (nlattr->nla_type & NLA_TYPE_MASK) {
     case AF_INET6:
         return target_to_host_for_each_nlattr(NLA_DATA(nlattr), nlattr->nla_len,
                                               target_to_host_data_inet6_nlattr);
@@ -1375,7 +1375,7 @@ static abi_long target_to_host_data_link_rtattr(struct rtattr *rtattr)
 {
     uint32_t *u32;
 
-    switch (rtattr->rta_type) {
+    switch (rtattr->rta_type & NLA_TYPE_MASK) {
     /* uint32_t */
     case QEMU_IFLA_MTU:
     case QEMU_IFLA_TXQLEN:
-- 
2.39.2

Re: [PATCH v3 2/2] linux-user: handle netlink flag NLA_F_NESTED

[Laurent Vivier]

Le 07/03/2023 à 16:42, Mathis Marion a écrit :
> From: Mathis Marion <mathis.marion@silabs.com>
> 
> Newer kernel versions require this flag to be present contrary to older
> ones. Depending on the libnl version it is added or not.
> 
> Typically when using rtnl_link_inet6_set_addr_gen_mode, the netlink
> packet generated may contain the following attribute:
> 
> with libnl 3.4
> 
>    {nla_len=16, nla_type=IFLA_AF_SPEC},
>    [
>      {nla_len=12, nla_type=AF_INET6},
>      [{nla_len=5, nla_type=IFLA_INET6_ADDR_GEN_MODE}, IN6_ADDR_GEN_MODE_NONE]
>    ]
> 
> with libnl 3.7
> 
>    {nla_len=16, nla_type=NLA_F_NESTED|IFLA_AF_SPEC},
>    [
>      {nla_len=12, nla_type=NLA_F_NESTED|AF_INET6},
>      [{nla_len=5, nla_type=IFLA_INET6_ADDR_GEN_MODE}, IN6_ADDR_GEN_MODE_NONE]]
>    ]
> 
> Masking the type is likely needed in other places. Only the above cases
> are implemented in this patch.
> 
> Signed-off-by: Mathis Marion <mathis.marion@silabs.com>
> ---
>   linux-user/fd-trans.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/linux-user/fd-trans.c b/linux-user/fd-trans.c
> index 4852a75d9d..c04a97c73a 100644
> --- a/linux-user/fd-trans.c
> +++ b/linux-user/fd-trans.c
> @@ -1359,7 +1359,7 @@ static abi_long target_to_host_for_each_rtattr(struct rtattr *rtattr,
>   
>   static abi_long target_to_host_data_spec_nlattr(struct nlattr *nlattr)
>   {
> -    switch (nlattr->nla_type) {
> +    switch (nlattr->nla_type & NLA_TYPE_MASK) {
>       case AF_INET6:
>           return target_to_host_for_each_nlattr(NLA_DATA(nlattr), nlattr->nla_len,
>                                                 target_to_host_data_inet6_nlattr);
> @@ -1375,7 +1375,7 @@ static abi_long target_to_host_data_link_rtattr(struct rtattr *rtattr)
>   {
>       uint32_t *u32;
>   
> -    switch (rtattr->rta_type) {
> +    switch (rtattr->rta_type & NLA_TYPE_MASK) {
>       /* uint32_t */
>       case QEMU_IFLA_MTU:
>       case QEMU_IFLA_TXQLEN:

Reviewed-by: Laurent Vivier <laurent@vivier.eu>

Re: [PATCH v3 2/2] linux-user: handle netlink flag NLA_F_NESTED

[Laurent Vivier]

Le 07/03/2023 à 16:42, Mathis Marion a écrit :
> From: Mathis Marion <mathis.marion@silabs.com>
> 
> Newer kernel versions require this flag to be present contrary to older
> ones. Depending on the libnl version it is added or not.
> 
> Typically when using rtnl_link_inet6_set_addr_gen_mode, the netlink
> packet generated may contain the following attribute:
> 
> with libnl 3.4
> 
>    {nla_len=16, nla_type=IFLA_AF_SPEC},
>    [
>      {nla_len=12, nla_type=AF_INET6},
>      [{nla_len=5, nla_type=IFLA_INET6_ADDR_GEN_MODE}, IN6_ADDR_GEN_MODE_NONE]
>    ]
> 
> with libnl 3.7
> 
>    {nla_len=16, nla_type=NLA_F_NESTED|IFLA_AF_SPEC},
>    [
>      {nla_len=12, nla_type=NLA_F_NESTED|AF_INET6},
>      [{nla_len=5, nla_type=IFLA_INET6_ADDR_GEN_MODE}, IN6_ADDR_GEN_MODE_NONE]]
>    ]
> 
> Masking the type is likely needed in other places. Only the above cases
> are implemented in this patch.
> 
> Signed-off-by: Mathis Marion <mathis.marion@silabs.com>
> ---
>   linux-user/fd-trans.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/linux-user/fd-trans.c b/linux-user/fd-trans.c
> index 4852a75d9d..c04a97c73a 100644
> --- a/linux-user/fd-trans.c
> +++ b/linux-user/fd-trans.c
> @@ -1359,7 +1359,7 @@ static abi_long target_to_host_for_each_rtattr(struct rtattr *rtattr,
>   
>   static abi_long target_to_host_data_spec_nlattr(struct nlattr *nlattr)
>   {
> -    switch (nlattr->nla_type) {
> +    switch (nlattr->nla_type & NLA_TYPE_MASK) {
>       case AF_INET6:
>           return target_to_host_for_each_nlattr(NLA_DATA(nlattr), nlattr->nla_len,
>                                                 target_to_host_data_inet6_nlattr);
> @@ -1375,7 +1375,7 @@ static abi_long target_to_host_data_link_rtattr(struct rtattr *rtattr)
>   {
>       uint32_t *u32;
>   
> -    switch (rtattr->rta_type) {
> +    switch (rtattr->rta_type & NLA_TYPE_MASK) {
>       /* uint32_t */
>       case QEMU_IFLA_MTU:
>       case QEMU_IFLA_TXQLEN:

Applied to my linux-user-for-8.0 branch.

Thanks,
Laurent