Le 26/10/2015 15:40, Peter Maydell a écrit :
> On 6 October 2015 at 18:11, Laurent Vivier <laur...@vivier.eu> wrote:
>> This is obsolete, but if we want to use dhcp with some distros (like debian
>> ppc 8.2 jessie), we need it.
>>
>> At the bind level, we are not able to know the socket type so we try to
>> guess it by analyzing the name. We manage only the case "ethX",
>> "ethX" in spk_device is similar to set htons(0x6574) in sll_protocol in the
>> normal case, and as this protocol does not exist, it's ok.
>>
>> SOCK_PACKET uses network endian to encode protocol in socket()
>>
>> in PACKET(7) :
>> protocol is the IEEE 802.3 protocol
>> number in network order. See the <linux/if_ether.h> include file for a
>> list of allowed protocols. When protocol is set to htons(ETH_P_ALL)
>> then all protocols are received. All incoming packets of that protocol
>> type will be passed to the packet socket before they are passed to the
>> protocols implemented in the kernel.
>>
>> Signed-off-by: Laurent Vivier <laur...@vivier.eu>
>> ---
>> This patch is a remix of an old patch sent in 2012:
>> https://patchwork.ozlabs.org/patch/208892/
>>
>> linux-user/syscall.c | 33 +++++++++++++++++++++++++++------
>> 1 file changed, 27 insertions(+), 6 deletions(-)
>>
>> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
>> index 64be431..71cc1e2 100644
>> --- a/linux-user/syscall.c
>> +++ b/linux-user/syscall.c
>> @@ -111,6 +111,7 @@ int __clone2(int (*fn)(void *), void *child_stack_base,
>> #include <linux/route.h>
>> #include <linux/filter.h>
>> #include <linux/blkpg.h>
>> +#include <linux/if_packet.h>
>> #include "linux_loop.h"
>> #include "uname.h"
>>
>> @@ -1198,11 +1199,20 @@ static inline abi_long
>> target_to_host_sockaddr(struct sockaddr *addr,
>> memcpy(addr, target_saddr, len);
>> addr->sa_family = sa_family;
>> if (sa_family == AF_PACKET) {
>> - struct target_sockaddr_ll *lladdr;
>> + /* Manage an obsolete case :
>> + * if socket type is SOCK_PACKET, bind by name otherwise by index
>> + * but we are not able to know socket type, so check if the name
>> + * is usable...
>> + * see linux/net/packet/af_packet.c: packet_bind_spkt()
>> + */
>> + if (strncmp((char *)((struct sockaddr_pkt *)addr)->spkt_device,
>> + "eth", 3) != 0) {
>> + struct target_sockaddr_ll *lladdr;
>
> This confuses me. The packet(7) manpage suggests there are two flavours
> of packet socket:
> (1) legacy AF_INET + SOCK_PACKET
> (2) new style AF_PACKET + SOCK_RAW / SOCK_DGRAM
>
> but this comment suggests it's trying to handle AF_PACKET + SOCK_PACKET ?
In fact, I've started not from the man page, but from a non working dhcp
client, originally with a m68k target and etch-m68k distro, and I've met
again this problem on a ppc target and jessie distro.
>
> If AF_PACKET was introduced as the new way of doing things, it's not
> clear why it would be the family type used in the legacy approach's
> sockaddr_pkt (though there seems to be code around that does this).
> I suspect that 2.0 kernels just didn't check af_family here at all.
by digging into the code, I have found:
$ apt-get source isc-dhcp-client
$ vi isc-dhcp-4.3.1/common/lpf.c
...
72 int if_register_lpf (info)
73 struct interface_info *info;
74 {
...
79 if ((sock = socket(PF_PACKET, SOCK_PACKET,
80 htons((short)ETH_P_ALL))) < 0) {
...
So we can see socket() is used with domain PF_PACKET and type SOCK_PACKET.
Next, the interface name is put into the sa_data of sockaddr, and bind()
is used with AF_PACKET:
94 /* Bind to the interface name */
95 memset (&sa, 0, sizeof sa);
96 sa.sa_family = AF_PACKET;
97 strncpy (sa.sa_data, (const char *)info -> ifp, sizeof
sa.sa_data);
98 if (bind (sock, &sa, sizeof sa)) {
ifp is initialized from a list of all discovered interfaces in
common/discover.c:
...
238 int
239 begin_iface_scan(struct iface_conf_list *ifaces) {
...
283 if (ioctl(ifaces->sock, SIOCGLIFCONF, &ifaces->conf) < 0) {
...
918 void
919 discover_interfaces(int state) {
...
924 struct interface_info *tmp;
...
939 if (!begin_iface_scan(&ifaces)) {
...
955 while (next_iface(&info, &err, &ifaces)) {
956
957 /* See if we've seen an interface that matches this
one. */
958 for (tmp = interfaces; tmp; tmp = tmp->next) {
959 if (!strcmp(tmp->name, info.name))
960 break;
961 }
...
987 strcpy(tmp->name, info.name);
...
1050 }
...
1063 for (tmp = interfaces ; tmp != NULL ; tmp = tmp->next) {
1064 if (tmp->ifp == NULL) {
1065 struct ifreq *tif;
1066
1067 tif = (struct ifreq *)dmalloc(sizeof(struct
ifreq),
1068 MDL);
1069 if (tif == NULL)
1070 log_fatal("no space for ifp mockup.");
1071 strcpy(tif->ifr_name, tmp->name);
1072 tmp->ifp = tif;
1073 }
1074 }
>
> The code in the kernel's packet_recvmsg fills in the spkt_family
> field with the netdevice's type field, which is an ARPHRD_* constant
> as far as I can tell (I could well be wrong here).
kernel 4.3.0-rc3, net/packet/af_packet.c:
2961
2962 static int packet_bind_spkt(struct socket *sock, struct sockaddr
*uaddr,
2963 int addr_len)
2964 {
2965 struct sock *sk = sock->sk;
2966 char name[15];
2967 struct net_device *dev;
2968 int err = -ENODEV;
2969
2970 /*
2971 * Check legality
2972 */
2973
2974 if (addr_len != sizeof(struct sockaddr))
2975 return -EINVAL;
2976 strlcpy(name, uaddr->sa_data, sizeof(name));
2977
2978 dev = dev_get_by_name(sock_net(sk), name);
2979 if (dev)
2980 err = packet_do_bind(sk, dev, pkt_sk(sk)->num);
2981 return err;
2982 }
...
4246 static const struct proto_ops packet_ops_spkt = {
4247 .family = PF_PACKET,
...
4250 .bind = packet_bind_spkt,
...
3022
3023 static int packet_create(struct net *net, struct socket *sock,
int protocol,
3024 int kern)
...
3045 if (sock->type == SOCK_PACKET)
3046 sock->ops = &packet_ops_spkt;
...
> Odd to have code in target_to_host_sockaddr and not in
> host_to_target_sockaddr.
In the case of host_to_target_sockaddr(), there is no "if (sa_family ==
AF_PACKET) {" as it was in target_to_host_sockaddr(), it's why I didn't
add it (and I don't like to add code I don't test).
>
>> - lladdr = (struct target_sockaddr_ll *)addr;
>> - lladdr->sll_ifindex = tswap32(lladdr->sll_ifindex);
>> - lladdr->sll_hatype = tswap16(lladdr->sll_hatype);
>> + lladdr = (struct target_sockaddr_ll *)addr;
>> + lladdr->sll_ifindex = tswap32(lladdr->sll_ifindex);
>> + lladdr->sll_hatype = tswap16(lladdr->sll_hatype);
>> + }
>> }
>> unlock_user(target_saddr, target_addr, 0);
>>
>> @@ -2509,7 +2519,12 @@ static abi_long do_socketcall(int num, abi_ulong vptr)
>> /* now when we have the args, actually handle the call */
>> switch (num) {
>> case SOCKOP_socket: /* domain, type, protocol */
>> - return do_socket(a[0], a[1], a[2]);
>> + if (a[0] == AF_PACKET ||
>> + a[1] == TARGET_SOCK_PACKET) {
>> + return do_socket(a[0], a[1], tswap16(a[2]));
>> + } else {
>> + return do_socket(a[0], a[1], a[2]);
>> + }
>> case SOCKOP_bind: /* sockfd, addr, addrlen */
>> return do_bind(a[0], a[1], a[2]);
>> case SOCKOP_connect: /* sockfd, addr, addrlen */
>> @@ -7500,7 +7515,13 @@ abi_long do_syscall(void *cpu_env, int num, abi_long
>> arg1,
>> #endif
>> #ifdef TARGET_NR_socket
>> case TARGET_NR_socket:
>> - ret = do_socket(arg1, arg2, arg3);
>> + if (arg1 == AF_PACKET ||
>> + arg2 == TARGET_SOCK_PACKET) {
>> + /* in this case, socket() needs a network endian short */
>> + ret = do_socket(arg1, arg2, tswap16(arg3));
>> + } else {
>> + ret = do_socket(arg1, arg2, arg3);
>> + }
>
> This doesn't make sense to me. The argument to socket()
> is passed in via a register; so if the guest code correctly
> passes the protocol value as htons(whatever) then that will
> be the value in arg3 and we do not need to swap anything.
>
> I see we had this discussion about the previous version of the
> patch too... and my argument that we don't need the tswap16
> in the socketcall code path either still makes sense to me.
I agree with you, I think I have confused socketcall() that passes
parameters by memory, and socket() that passes parameters by registers.
I will remove this part and resend a patch.
>
>> fd_trans_unregister(ret);
>> break;
>> #endif
>> --
>> 2.4.3
>
> thanks
> -- PMM
>
Thank you for your comments,
Laurent
