Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bugs in if_nametoindex/if_indextoname #1096

Closed
wkozaczuk opened this issue Aug 16, 2020 · 2 comments
Closed

Bugs in if_nametoindex/if_indextoname #1096

wkozaczuk opened this issue Aug 16, 2020 · 2 comments

Comments

@wkozaczuk
Copy link
Collaborator

wkozaczuk commented Aug 16, 2020

The implementation of if_nametoindex()/if_indextoname() copied from musl under libc/network/if_nametoindex.c and libc/network/if_indextoname.c has number of problems:

  1. Both use AF_UNIX domain which is unsupported on OSv (see fd = socket(AF_UNIX, SOCK_DGRAM, 0)); instead we neeed to use AF_INET like if_nameindex uses - see c4df104)
  2. Even after fixing the above, if_nametoindex returns strangely high number like 1082589186 for eth0 and 1082589185 for lo0; most likely it is caused by not zero-ing the index field (for example zeroing ifr.ifr_ifindex in if_nametoindex.c before calling ioctl() makes the code return correct 2 and 1 index; the correct fix should be somewhere in bsd/linuz compatibility layer code (bsd/sys/compat/linux/linux_ioctl.cc or bsd/sys/kern/sys_socket.cc or bsd/sys/net/if.cc).
  3. The if_indextoname needs support in ioctl layer (see missing handling of SIOCGIFNAME in bsd/sys/compat/linux/linux_ioctl.cc).
@wkozaczuk
Copy link
Collaborator Author

It looks like all these issues are fixed in the IPV6 branch (see https://github.com/cloudius-systems/osv/blob/ipv6/libc/network/if_nametoindex.c and https://github.com/cloudius-systems/osv/blob/ipv6/libc/network/if_indextoname.c). Also, partial netlink support has been added in IPV6 branch and it will be necessary for an upgrade to latest musl whose if_nameindex.c depends on. So it looks more so we will also need to merge IPV6 branch into master.

@wkozaczuk wkozaczuk added the bug label Jul 12, 2021
@wkozaczuk
Copy link
Collaborator Author

Instead of merging the entire ipv6 branch, we could partially merge some of commits or possibly only non-IPV6 part of it:

  • f1cd48e - netlink
  • d35c3dd - Fix if_indextoname(), if_nametoindex()
  • b687b7c - libc: Add IPv6 support to getifaddrs(), if_nameindex() using NETLINK socket
  • ea13cb1 - bsd: Fix SIOCSIFNAME when using linux compatiblity socket

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant