Re: [PATCH v3 4/6] linux-user: netlink: Add emulation of IP_MULTICAST_IF

[Laurent Vivier]

Le 19/01/2025 à 05:41, deller@kernel.org a écrit :
> From: Helge Deller <deller@gmx.de>
>
> Add IP_MULTICAST_IF and share the code with IP_ADD_MEMBERSHIP / 
> IP_DROP_MEMBERSHIP.
> Sharing the code makes sense, because the manpage of ip(7)  says:
>
> IP_MULTICAST_IF (since Linux 1.2)
>        Set the local device for a multicast socket.  The argument
>        for setsockopt(2) is an ip_mreqn or (since Linux 3.5)
>        ip_mreq structure similar to IP_ADD_MEMBERSHIP, or an
>        in_addr structure.  (The kernel determines which structure
>        is being passed based on the size passed in optlen.)  For
>        getsockopt(2), the argument is an in_addr structure.
>
> Signed-off-by: Helge Deller <deller@gmx.de>
>
> v2: (based on feedback by Laurent Vivier)
> - refined commit message and restructure the copying of ip_mreqn fields
>
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index bbe2560927..4360543e20 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -2130,16 +2130,23 @@ static abi_long do_setsockopt(int sockfd, int level, 
> int optname,
>               }
>               ret = get_errno(setsockopt(sockfd, level, optname, &val, 
> sizeof(val)));
>               break;
> +        case IP_MULTICAST_IF:
>           case IP_ADD_MEMBERSHIP:
>           case IP_DROP_MEMBERSHIP:
>           {
>               struct ip_mreqn ip_mreq;
>               struct target_ip_mreqn *target_smreqn;
> +            int min_size;
>
>               QEMU_BUILD_BUG_ON(sizeof(struct ip_mreq) !=
>                                 sizeof(struct target_ip_mreq));
>
> -            if (optlen < sizeof (struct target_ip_mreq) ||
> +            if (optname == IP_MULTICAST_IF) {
> +                min_size = sizeof(struct in_addr);
> +            } else {
> +                min_size = sizeof(struct target_ip_mreq);
> +            }
> +            if (optlen < min_size ||
>                   optlen > sizeof (struct target_ip_mreqn)) {
>                   return -TARGET_EINVAL;
>               }
> @@ -2149,7 +2156,9 @@ static abi_long do_setsockopt(int sockfd, int level, int 
> optname,
>                   return -TARGET_EFAULT;
>               }
>               ip_mreq.imr_multiaddr.s_addr = 
> target_smreqn->imr_multiaddr.s_addr;
> -            ip_mreq.imr_address.s_addr = target_smreqn->imr_address.s_addr;
> +            if (optlen >= sizeof(struct target_ip_mreq)) {
> +                ip_mreq.imr_address.s_addr = target_smreqn->imr_address.s_addr;
> +            }
>               if (optlen == sizeof(struct target_ip_mreqn)) {
>                   ip_mreq.imr_ifindex = tswapal(target_smreqn->imr_ifindex);
>                   optlen = sizeof(struct ip_mreqn);

This part above seems to be in the commit message... and is the version 2, is 
this what you want?

> ---
>   linux-user/syscall.c | 20 ++++++++++++++------
>   1 file changed, 14 insertions(+), 6 deletions(-)
>
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index df8609b4d8..6ee02383da 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -2130,16 +2130,23 @@ static abi_long do_setsockopt(int sockfd, int level, 
> int optname,
>               }
>               ret = get_errno(setsockopt(sockfd, level, optname, &val, 
> sizeof(val)));
>               break;
> +        case IP_MULTICAST_IF:
>           case IP_ADD_MEMBERSHIP:
>           case IP_DROP_MEMBERSHIP:
>           {
>               struct ip_mreqn ip_mreq;
>               struct target_ip_mreqn *target_smreqn;
> +            int min_size;
>   
>               QEMU_BUILD_BUG_ON(sizeof(struct ip_mreq) !=
>                                 sizeof(struct target_ip_mreq));
>   
> -            if (optlen < sizeof (struct target_ip_mreq) ||
> +            if (optname == IP_MULTICAST_IF) {
> +                min_size = sizeof(struct in_addr);
> +            } else {
> +                min_size = sizeof(struct target_ip_mreq);
> +            }
> +            if (optlen < min_size ||
>                   optlen > sizeof (struct target_ip_mreqn)) {
>                   return -TARGET_EINVAL;
>               }
> @@ -2149,13 +2156,14 @@ static abi_long do_setsockopt(int sockfd, int level, 
> int optname,
>                   return -TARGET_EFAULT;
>               }
>               ip_mreq.imr_multiaddr.s_addr = 
> target_smreqn->imr_multiaddr.s_addr;
> -            ip_mreq.imr_address.s_addr = target_smreqn->imr_address.s_addr;
> -            if (optlen == sizeof(struct target_ip_mreqn)) {
> -                ip_mreq.imr_ifindex = tswapal(target_smreqn->imr_ifindex);
> -                optlen = sizeof(struct ip_mreqn);
> +            if (optlen >= sizeof(struct target_ip_mreq)) {
> +                ip_mreq.imr_address.s_addr = target_smreqn->imr_address.s_addr;
> +                if (optlen >= sizeof(struct target_ip_mreqn)) {
> +                    __put_user(target_smreqn->imr_ifindex, 
> &ip_mreq.imr_ifindex);
> +                    optlen = sizeof(struct ip_mreqn);
> +                }
>               }
>               unlock_user(target_smreqn, optval_addr, 0);
> -
>               ret = get_errno(setsockopt(sockfd, level, optname, &ip_mreq, 
> optlen));
>               break;
>           }

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