Date: Sat, 4 Mar 2023 09:12:48 GMT From: Dmitry Chagin <dchagin@FreeBSD.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org Subject: git: f9b0675b014d - main - linux(4): Refactor socket ioctl path to avoid referencing an unstable interfaces Message-ID: <202303040912.3249CmOt078912@gitrepo.freebsd.org>
next in thread | raw e-mail | index | archive | help
The branch main has been updated by dchagin: URL: https://cgit.FreeBSD.org/src/commit/?id=f9b0675b014d06995e5337b71129fc94bd1cedd0 commit f9b0675b014d06995e5337b71129fc94bd1cedd0 Author: Dmitry Chagin <dchagin@FreeBSD.org> AuthorDate: 2023-03-04 09:11:38 +0000 Commit: Dmitry Chagin <dchagin@FreeBSD.org> CommitDate: 2023-03-04 09:11:38 +0000 linux(4): Refactor socket ioctl path to avoid referencing an unstable interfaces Split the linux_ioctl_socket() function on two counterparts, where the linux_ioctl_socket_ifreq() intended to use in a code path which requires the struct ifreq manipulation, i.e., translating in/out values of the struct, while the linux_ioctl_socket() function is left as is, it calls sys_ioctl() without touching in/out values. Due to structures ifreq, sockaddr difference between FreeBSD and Linux the linux_ioctl_socket_ifreq() calls kern_ioctl() directly, converting in and out values to FreeBSD and to Linux accordingly. Finally, modify the ifname_linux_to_bsd() to return error code, not an unstable reference to the interface. Reviewed by: melifaro Differential Revision: https://reviews.freebsd.org/D38794 --- sys/compat/linux/linux.c | 4 +- sys/compat/linux/linux_common.h | 3 +- sys/compat/linux/linux_ioctl.c | 310 +++++++++++++++++----------------------- 3 files changed, 132 insertions(+), 185 deletions(-) diff --git a/sys/compat/linux/linux.c b/sys/compat/linux/linux.c index f31a4b5e4f5c..2e6e52f7490c 100644 --- a/sys/compat/linux/linux.c +++ b/sys/compat/linux/linux.c @@ -411,7 +411,7 @@ ifname_linux_to_ifp(struct thread *td, const char *lxname) return (arg.ifp); } -struct ifnet * +int ifname_linux_to_bsd(struct thread *td, const char *lxname, char *bsdname) { struct epoch_tracker et; @@ -424,7 +424,7 @@ ifname_linux_to_bsd(struct thread *td, const char *lxname, char *bsdname) strlcpy(bsdname, if_name(ifp), IFNAMSIZ); NET_EPOCH_EXIT(et); CURVNET_RESTORE(); - return (ifp); + return (ifp != NULL ? 0 : EINVAL); } unsigned short diff --git a/sys/compat/linux/linux_common.h b/sys/compat/linux/linux_common.h index 4b693ccaf868..3392f55672f3 100644 --- a/sys/compat/linux/linux_common.h +++ b/sys/compat/linux/linux_common.h @@ -34,9 +34,8 @@ int ifname_bsd_to_linux_ifp(struct ifnet *, char *, size_t); int ifname_bsd_to_linux_idx(u_int, char *, size_t); int ifname_bsd_to_linux_name(const char *, char *, size_t); struct ifnet *ifname_linux_to_ifp(struct thread *, const char *); +int ifname_linux_to_bsd(struct thread *, const char *, char *); -struct ifnet *ifname_linux_to_bsd(struct thread *td, - const char *lxname, char *bsdname); unsigned short linux_ifflags(struct ifnet *); int linux_ifhwaddr(struct ifnet *ifp, struct l_sockaddr *lsa); diff --git a/sys/compat/linux/linux_ioctl.c b/sys/compat/linux/linux_ioctl.c index 9f1fdd3a6671..4c95745e4307 100644 --- a/sys/compat/linux/linux_ioctl.c +++ b/sys/compat/linux/linux_ioctl.c @@ -49,6 +49,7 @@ __FBSDID("$FreeBSD$"); #include <sys/sbuf.h> #include <sys/sockio.h> #include <sys/soundcard.h> +#include <sys/syscallsubr.h> #include <sys/sysctl.h> #include <sys/sysproto.h> #include <sys/sx.h> @@ -2187,22 +2188,17 @@ linux_ifconf(struct thread *td, struct ifconf *uifc) full = 0; cbs.max_len = maxphys - 1; - CURVNET_SET(TD_TO_VNET(td)); /* handle the 'request buffer size' case */ if ((l_uintptr_t)ifc.ifc_buf == PTROUT(NULL)) { ifc.ifc_len = 0; NET_EPOCH_ENTER(et); if_foreach(linux_ifconf_ifnet_cb, &ifc); NET_EPOCH_EXIT(et); - error = copyout(&ifc, uifc, sizeof(ifc)); - CURVNET_RESTORE(); - return (error); + return (copyout(&ifc, uifc, sizeof(ifc))); } - if (ifc.ifc_len <= 0) { - CURVNET_RESTORE(); + if (ifc.ifc_len <= 0) return (EINVAL); - } again: if (ifc.ifc_len <= cbs.max_len) { @@ -2229,52 +2225,132 @@ again: if (error == 0) error = copyout(&ifc, uifc, sizeof(ifc)); sbuf_delete(sb); - CURVNET_RESTORE(); return (error); } static int -linux_gifflags(struct thread *td, struct ifnet *ifp, struct l_ifreq *ifr) +linux_ioctl_socket_ifreq(struct thread *td, int fd, u_int cmd, + struct l_ifreq *uifr) { - unsigned short flags; - - flags = linux_ifflags(ifp); - - return (copyout(&flags, &ifr->ifr_flags, sizeof(flags))); -} + struct l_ifreq lifr; + struct ifreq bifr; + size_t ifrusiz; + int error, temp_flags; -static int -linux_gifhwaddr(struct ifnet *ifp, struct l_ifreq *ifr) -{ - struct l_sockaddr lsa; - - if (linux_ifhwaddr(ifp, &lsa) != 0) - return (ENOENT); + switch (cmd) { + case LINUX_SIOCGIFINDEX: + cmd = SIOCGIFINDEX; + break; + case LINUX_SIOCGIFFLAGS: + cmd = SIOCGIFFLAGS; + break; + case LINUX_SIOCGIFADDR: + cmd = SIOCGIFADDR; + break; + case LINUX_SIOCSIFADDR: + cmd = SIOCSIFADDR; + break; + case LINUX_SIOCGIFDSTADDR: + cmd = SIOCGIFDSTADDR; + break; + case LINUX_SIOCGIFBRDADDR: + cmd = SIOCGIFBRDADDR; + break; + case LINUX_SIOCGIFNETMASK: + cmd = SIOCGIFNETMASK; + break; + case LINUX_SIOCSIFNETMASK: + cmd = SIOCSIFNETMASK; + break; + case LINUX_SIOCGIFMTU: + cmd = SIOCGIFMTU; + break; + case LINUX_SIOCSIFMTU: + cmd = SIOCSIFMTU; + break; + case LINUX_SIOCGIFHWADDR: + cmd = SIOCGHWADDR; + break; + /* + * XXX This is slightly bogus, but these ioctls are currently + * XXX only used by the aironet (if_an) network driver. + */ + case LINUX_SIOCDEVPRIVATE: + cmd = SIOCGPRIVATE_0; + break; + case LINUX_SIOCDEVPRIVATE+1: + cmd = SIOCGPRIVATE_1; + break; + default: + return (ENOIOCTL); + } - return (copyout(&lsa, &ifr->ifr_hwaddr, sizeof(lsa))); -} + error = copyin(uifr, &lifr, sizeof(lifr)); + if (error != 0) + return (error); + bzero(&bifr, sizeof(bifr)); - /* -* If we fault in bsd_to_linux_ifreq() then we will fault when we call -* the native ioctl(). Thus, we don't really need to check the return -* value of this function. -*/ -static int -bsd_to_linux_ifreq(struct ifreq *arg) -{ - struct ifreq ifr; - size_t ifr_len = sizeof(struct ifreq); - int error; + /* + * The size of Linux enum ifr_ifru is bigger than + * the FreeBSD size due to the struct ifmap. + */ + ifrusiz = (sizeof(lifr) > sizeof(bifr) ? sizeof(bifr) : + sizeof(lifr)) - offsetof(struct l_ifreq, ifr_ifru); + bcopy(&lifr.ifr_ifru, &bifr.ifr_ifru, ifrusiz); - if ((error = copyin(arg, &ifr, ifr_len))) + error = ifname_linux_to_bsd(td, lifr.ifr_name, bifr.ifr_name); + if (error != 0) return (error); - *(u_short *)&ifr.ifr_addr = ifr.ifr_addr.sa_family; + /* Translate in values. */ + switch (cmd) { + case SIOCGIFINDEX: + bifr.ifr_index = lifr.ifr_index; + break; + case SIOCSIFADDR: + case SIOCSIFNETMASK: + bifr.ifr_addr.sa_len = sizeof(struct sockaddr); + bifr.ifr_addr.sa_family = + linux_to_bsd_domain(lifr.ifr_addr.sa_family); + break; + default: + break; + } - error = copyout(&ifr, arg, ifr_len); + error = kern_ioctl(td, fd, cmd, (caddr_t)&bifr); + if (error != 0) + return (error); + bzero(&lifr.ifr_ifru, sizeof(lifr.ifr_ifru)); + + /* Translate out values. */ + switch (cmd) { + case SIOCGIFINDEX: + lifr.ifr_index = bifr.ifr_index; + break; + case SIOCGIFFLAGS: + temp_flags = bifr.ifr_flags | (bifr.ifr_flagshigh << 16); + lifr.ifr_flags = bsd_to_linux_ifflags(temp_flags); + break; + case SIOCGIFADDR: + case SIOCSIFADDR: + case SIOCGIFDSTADDR: + case SIOCGIFBRDADDR: + case SIOCGIFNETMASK: + bcopy(&bifr.ifr_addr, &lifr.ifr_addr, sizeof(bifr.ifr_addr)); + lifr.ifr_addr.sa_family = + bsd_to_linux_domain(bifr.ifr_addr.sa_family); + break; + case SIOCGHWADDR: + bcopy(&bifr.ifr_addr, &lifr.ifr_hwaddr, sizeof(bifr.ifr_addr)); + lifr.ifr_hwaddr.sa_family = LINUX_ARPHRD_ETHER; + break; + default: + bcopy(&bifr.ifr_ifru, &lifr.ifr_ifru, ifrusiz); + break; + } - return (error); + return (copyout(&lifr, uifr, sizeof(lifr))); } /* @@ -2284,84 +2360,34 @@ bsd_to_linux_ifreq(struct ifreq *arg) static int linux_ioctl_socket(struct thread *td, struct linux_ioctl_args *args) { - char lifname[LINUX_IFNAMSIZ], ifname[IFNAMSIZ]; - struct ifnet *ifp; struct file *fp; int error, type; - ifp = NULL; - error = 0; - error = fget(td, args->fd, &cap_ioctl_rights, &fp); if (error != 0) return (error); type = fp->f_type; fdrop(fp, td); + + CURVNET_SET(TD_TO_VNET(td)); + if (type != DTYPE_SOCKET) { /* not a socket - probably a tap / vmnet device */ switch (args->cmd) { case LINUX_SIOCGIFADDR: case LINUX_SIOCSIFADDR: case LINUX_SIOCGIFFLAGS: - return (linux_ioctl_special(td, args)); + error = linux_ioctl_special(td, args); + break; default: - return (ENOIOCTL); + error = ENOIOCTL; + break; } + CURVNET_RESTORE(); + return (error); } - switch (args->cmd & 0xffff) { - case LINUX_FIOGETOWN: - case LINUX_FIOSETOWN: - case LINUX_SIOCADDMULTI: - case LINUX_SIOCATMARK: - case LINUX_SIOCDELMULTI: - case LINUX_SIOCGIFNAME: - case LINUX_SIOCGIFCONF: - case LINUX_SIOCGPGRP: - case LINUX_SIOCSPGRP: - case LINUX_SIOCGIFCOUNT: - /* these ioctls don't take an interface name */ - break; - - case LINUX_SIOCGIFFLAGS: - case LINUX_SIOCGIFADDR: - case LINUX_SIOCSIFADDR: - case LINUX_SIOCGIFDSTADDR: - case LINUX_SIOCGIFBRDADDR: - case LINUX_SIOCGIFNETMASK: - case LINUX_SIOCSIFNETMASK: - case LINUX_SIOCGIFMTU: - case LINUX_SIOCSIFMTU: - case LINUX_SIOCSIFNAME: - case LINUX_SIOCGIFHWADDR: - case LINUX_SIOCSIFHWADDR: - case LINUX_SIOCDEVPRIVATE: - case LINUX_SIOCDEVPRIVATE+1: - case LINUX_SIOCGIFINDEX: - /* copy in the interface name and translate it. */ - error = copyin((void *)args->arg, lifname, LINUX_IFNAMSIZ); - if (error != 0) - return (error); - memset(ifname, 0, sizeof(ifname)); - ifp = ifname_linux_to_bsd(td, lifname, ifname); - if (ifp == NULL) - return (EINVAL); - /* - * We need to copy it back out in case we pass the - * request on to our native ioctl(), which will expect - * the ifreq to be in user space and have the correct - * interface name. - */ - error = copyout(ifname, (void *)args->arg, IFNAMSIZ); - if (error != 0) - return (error); - break; - - default: - return (ENOIOCTL); - } - - switch (args->cmd & 0xffff) { + switch (args->cmd) { case LINUX_FIOSETOWN: args->cmd = FIOSETOWN; error = sys_ioctl(td, (struct ioctl_args *)args); @@ -2397,67 +2423,6 @@ linux_ioctl_socket(struct thread *td, struct linux_ioctl_args *args) error = linux_ifconf(td, (struct ifconf *)args->arg); break; - case LINUX_SIOCGIFFLAGS: - args->cmd = SIOCGIFFLAGS; - error = linux_gifflags(td, ifp, (struct l_ifreq *)args->arg); - break; - - case LINUX_SIOCGIFADDR: - args->cmd = SIOCGIFADDR; - error = sys_ioctl(td, (struct ioctl_args *)args); - bsd_to_linux_ifreq((struct ifreq *)args->arg); - break; - - case LINUX_SIOCSIFADDR: - /* XXX probably doesn't work, included for completeness */ - args->cmd = SIOCSIFADDR; - error = sys_ioctl(td, (struct ioctl_args *)args); - break; - - case LINUX_SIOCGIFDSTADDR: - args->cmd = SIOCGIFDSTADDR; - error = sys_ioctl(td, (struct ioctl_args *)args); - bsd_to_linux_ifreq((struct ifreq *)args->arg); - break; - - case LINUX_SIOCGIFBRDADDR: - args->cmd = SIOCGIFBRDADDR; - error = sys_ioctl(td, (struct ioctl_args *)args); - bsd_to_linux_ifreq((struct ifreq *)args->arg); - break; - - case LINUX_SIOCGIFNETMASK: - args->cmd = SIOCGIFNETMASK; - error = sys_ioctl(td, (struct ioctl_args *)args); - bsd_to_linux_ifreq((struct ifreq *)args->arg); - break; - - case LINUX_SIOCSIFNETMASK: - error = ENOIOCTL; - break; - - case LINUX_SIOCGIFMTU: - args->cmd = SIOCGIFMTU; - error = sys_ioctl(td, (struct ioctl_args *)args); - break; - - case LINUX_SIOCSIFMTU: - args->cmd = SIOCSIFMTU; - error = sys_ioctl(td, (struct ioctl_args *)args); - break; - - case LINUX_SIOCSIFNAME: - error = ENOIOCTL; - break; - - case LINUX_SIOCGIFHWADDR: - error = linux_gifhwaddr(ifp, (struct l_ifreq *)args->arg); - break; - - case LINUX_SIOCSIFHWADDR: - error = ENOIOCTL; - break; - case LINUX_SIOCADDMULTI: args->cmd = SIOCADDMULTI; error = sys_ioctl(td, (struct ioctl_args *)args); @@ -2468,34 +2433,17 @@ linux_ioctl_socket(struct thread *td, struct linux_ioctl_args *args) error = sys_ioctl(td, (struct ioctl_args *)args); break; - case LINUX_SIOCGIFINDEX: - args->cmd = SIOCGIFINDEX; - error = sys_ioctl(td, (struct ioctl_args *)args); - break; - case LINUX_SIOCGIFCOUNT: error = 0; break; - /* - * XXX This is slightly bogus, but these ioctls are currently - * XXX only used by the aironet (if_an) network driver. - */ - case LINUX_SIOCDEVPRIVATE: - args->cmd = SIOCGPRIVATE_0; - error = sys_ioctl(td, (struct ioctl_args *)args); - break; - - case LINUX_SIOCDEVPRIVATE+1: - args->cmd = SIOCGPRIVATE_1; - error = sys_ioctl(td, (struct ioctl_args *)args); + default: + error = linux_ioctl_socket_ifreq(td, args->fd, args->cmd, + PTRIN(args->arg)); break; } - if (ifp != NULL) - /* restore the original interface name */ - copyout(lifname, (void *)args->arg, LINUX_IFNAMSIZ); - + CURVNET_RESTORE(); return (error); }
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202303040912.3249CmOt078912>