Re: [PATCH v1 3/4] linux-user: add target to host netlink conversions

[Philippe Mathieu-Daudé]

On 20/2/23 09:24, Mathis MARION wrote:
> On 20/02/2023 08:22, Philippe Mathieu-Daudé wrote:
> > On 17/2/23 17:35, Mathis Marion wrote:
> > > From: Mathis Marion <mathis.marion@silabs.com>
> > >
> > > Added conversions for:
> > > - IFLA_MTU
> > > - IFLA_TXQLEN
> > > - IFLA_AF_SPEC AF_INET6 IFLA_INET6_ADDR_GEN_MODE
> > > These relate to the libnl functions rtnl_link_set_mtu,
> > > rtnl_link_set_txqlen, and rtnl_link_inet6_set_addr_gen_mode.
> > >
> > > Signed-off-by: Mathis Marion <mathis.marion@silabs.com>
> > > ---
> > >   linux-user/fd-trans.c | 64 +++++++++++++++++++++++++++++++++++++++++++
> > >   1 file changed, 64 insertions(+)

> > > +                                               abi_long 
> > > (*target_to_host_nlattr)
> > > +                                                        (struct 
> > > nlattr *))
> > > +{
> > > +    unsigned short aligned_nla_len;
> > > +    abi_long ret;
> > > +
> > > +    while (len > sizeof(struct nlattr)) {
> > > +        if (tswap16(nlattr->nla_len) < sizeof(struct rtattr) ||
> > > +            tswap16(nlattr->nla_len) > len) {
> > > +            break;
> > > +        }
> > > +        nlattr->nla_len = tswap16(nlattr->nla_len);
> > > +        nlattr->nla_type = tswap16(nlattr->nla_type);
> > > +        ret = target_to_host_nlattr(nlattr);
> > > +        if (ret < 0) {
> >
> > If this fail, guest's nlattr is now inconsistent. Is this OK?
>
> The same check is done in target_to_host_for_each_rtattr(), and in all
> host_to_target_for_each* functions so I think this is OK.

Yeah this is pre-existing, so your patch is OK, but I still wonder
if this is safe. Laurent?

> > > +            return ret;
> > > +        }
> > > +
> > > +        aligned_nla_len = NLA_ALIGN(nlattr->nla_len);
> > > +        if (aligned_nla_len >= len) {
> > > +            break;
> > > +        }
> > > +        len -= aligned_nla_len;
> > > +        nlattr = (struct nlattr *)(((char *)nlattr) + aligned_nla_len);
> > > +    }
> > > +    return 0;
> > > +}