Hello folks,
I've been using a nice program on ppc32 with a ppc64
system+kernel. It uses netlink to determine some network interface
information. Recent kernels cause it to exit at the netlink stage. At
first, I thought it was a bug in the program. Now I'm not so sure.
The situation is specifically the use of SIOCGIFNAME in 32bit
binaries on 64 bit platforms+kernels. I've tested ppc32 on ppc64 and
ia32 on amd64. amd64 on amd64, ia32 on ia32,and ppc64 on ppc64
all work fine.
I've cooked up a test program to evince the problem. It just
asks netlink for an interface list, then uses SIOCGIFNAME to get the
interface name. On kernels before commit
32da477a5bfe96b6dfc8960e0d22d89ca09fd10a [NET]: Don't implement
dev_ifname32 inline, the test program runs fine. Kernels after, I get
EFAULT. This behavior is the same as the original program.
Instrumenting the kernel with printks, the EFAULT comes from
the first copy_in_user() at line 325 of fs/compat_ioctl.c (in
dev_ifname32()). I put some access_ok() checks in, and they do not
trigger (access is ok). The call never even gets into sys_ioctl().
I'm assuming at this point that copy_in_user() for ia32/amd64
and ppc32/ppc64 is what is having trouble. Test program follows.
Joel
#include <stdio.h>
#include <string.h>
#include <stdlib.h>
#include <stdarg.h>
#include <sys/types.h>
#include <sys/ioctl.h>
#include <sys/socket.h>
#include <arpa/inet.h>
#include <netinet/in.h>
#include <errno.h>
#include <unistd.h>
#include <net/if.h>
#include <asm/types.h>
#include <linux/rtnetlink.h>
#define NETLINK_BUFSIZE 16384
static void bail(int rc, char *fmt, ...)
{
va_list args;
if (fmt) {
va_start(args, fmt);
vfprintf(stderr, fmt, args);
va_end(args);
}
exit(rc);
}
int main(int argc, char *argv[])
{
int fd, ioctl_fd, rc, len;
struct {
struct nlmsghdr nlh;
struct rtgenmsg g;
} req;
struct sockaddr_nl nladdr;
static char rcvbuf[NETLINK_BUFSIZE];
struct nlmsghdr *h;
struct iovec iov = {rcvbuf, sizeof(rcvbuf) };
struct msghdr msg = {
(void *)&nladdr, sizeof(nladdr),
&iov, 1,
NULL, 0,
0
};
struct ifreq ifr;
struct ifaddrmsg *ifa;
fd = socket(AF_NETLINK, SOCK_RAW, NETLINK_ROUTE);
if (fd < 0)
bail(1, "Unable to open netlink socket: %s\n", strerror(errno));
if (setsockopt(fd, SOL_SOCKET, SO_RCVBUF, &rcvbuf, sizeof(rcvbuf)))
bail(1, "Unable to set socket options: %s\n", strerror(errno));
memset(&nladdr, 0, sizeof(nladdr));
nladdr.nl_family = AF_NETLINK;
req.nlh.nlmsg_len = sizeof(req);
req.nlh.nlmsg_type = RTM_GETADDR;
req.nlh.nlmsg_flags = NLM_F_ROOT|NLM_F_MATCH|NLM_F_REQUEST;
req.nlh.nlmsg_pid = 0;
req.nlh.nlmsg_seq = 1;
req.g.rtgen_family = AF_INET;
if (sendto(fd, (void *)&req, sizeof(req), 0,
(struct sockaddr *)&nladdr, sizeof(nladdr)) < 0)
bail(1, "Unable to send netlink request: %s\n", strerror(errno));
while (1)
{
rc = recvmsg(fd, &msg, 0);
if (!rc)
bail(1, "Netlink socket closed\n");
h = (struct nlmsghdr *)rcvbuf;
if (h->nlmsg_type == NLMSG_DONE)
break;
if (h->nlmsg_type == NLMSG_ERROR)
bail(1, "Error from netlink socket\n");
len = rc;
for (; NLMSG_OK(h, len); h = NLMSG_NEXT(h, len)) {
if (h->nlmsg_type != RTM_NEWADDR)
continue;
ioctl_fd = socket(AF_INET, SOCK_STREAM, 0);
if (ioctl_fd < 0)
bail(1, "Unable to create ioctl socket: %s\n",
strerror(errno));
ifa = NLMSG_DATA(h);
memset(&ifr, 0, sizeof(ifr));
ifr.ifr_ifindex = ifa->ifa_index;
rc = ioctl(ioctl_fd, SIOCGIFNAME, &ifr);
if (rc)
bail(1, "Unable to get name: %s\n", strerror(errno));
fprintf(stdout, "Interface %d, %s\n", ifr.ifr_ifindex,
ifr.ifr_ifrn.ifrn_name);
}
}
return 0;
}
--
"What no boss of a programmer can ever understand is that a programmer
is working when he's staring out of the window"
- With apologies to Burton Rascoe
Joel Becker
Principal Software Developer
Oracle
E-mail: [email protected]
Phone: (650) 506-8127
On Tue, 2007-10-30 at 17:38 -0700, Joel Becker wrote:
> Hello folks,
> I've been using a nice program on ppc32 with a ppc64
> system+kernel. It uses netlink to determine some network interface
> information. Recent kernels cause it to exit at the netlink stage. At
> first, I thought it was a bug in the program. Now I'm not so sure.
.../...
Reproduced here, I'll have a look, thanks.
Ben.
On Wednesday 31 October 2007, Joel Becker wrote:
>
> ????????Instrumenting the kernel with printks, the EFAULT comes from
> the first copy_in_user() at line 325 of fs/compat_ioctl.c (in
> dev_ifname32()). ?I put some access_ok() checks in, and they do not
> trigger (access is ok). ?The call never even gets into sys_ioctl().
Can you printk the pointers (arg and uifr) as well? Maybe the end up
unaligned or otherwise corrupted for some reason.
Does the same thing happen when you try the ioctl on something that
is not even a socket? E.g.
#include <whatever you need...>
int main(void)
{
struct ifreq ifr = {};
ioctl(0 /* standard input! */, SIOCGIFNAME, &ifr);
perror("ioctl");
}
Arnd <><
Bug is in the new dev_ifname32:
uifr = compat_alloc_user_space(sizeof(struct ifreq));
if (copy_in_user(uifr, compat_ptr(arg), sizeof(struct ifreq32)));
return -EFAULT;
There's a stray ";" after the if statement, that was obviously not
tested :-)
This fixes it here (tested):
----------------------------------------------------------------------------
[PATCH] Fix new dev_ifname32 returning -EFAULT
A stray semicolon slipped in the patch that updated dev_ifname32 to
not be inline, causing it to always return -EFAULT. This fixes it.
Signed-off-by: Benjamin Herrenschmidt <[email protected]>
---
Index: linux-work/fs/compat_ioctl.c
===================================================================
--- linux-work.orig/fs/compat_ioctl.c 2007-10-31 13:30:42.000000000 +1100
+++ linux-work/fs/compat_ioctl.c 2007-10-31 13:30:46.000000000 +1100
@@ -322,7 +322,7 @@ static int dev_ifname32(unsigned int fd,
int err;
uifr = compat_alloc_user_space(sizeof(struct ifreq));
- if (copy_in_user(uifr, compat_ptr(arg), sizeof(struct ifreq32)));
+ if (copy_in_user(uifr, compat_ptr(arg), sizeof(struct ifreq32)))
return -EFAULT;
err = sys_ioctl(fd, SIOCGIFNAME, (unsigned long)uifr);
From: Benjamin Herrenschmidt <[email protected]>
Date: Wed, 31 Oct 2007 13:35:24 +1100
> [PATCH] Fix new dev_ifname32 returning -EFAULT
>
> A stray semicolon slipped in the patch that updated dev_ifname32 to
> not be inline, causing it to always return -EFAULT. This fixes it.
>
> Signed-off-by: Benjamin Herrenschmidt <[email protected]>
Applied, thanks for the fix Ben!
Benjamin Herrenschmidt <[email protected]> writes:
> Bug is in the new dev_ifname32:
>
> uifr = compat_alloc_user_space(sizeof(struct ifreq));
> if (copy_in_user(uifr, compat_ptr(arg), sizeof(struct ifreq32)));
> return -EFAULT;
>
> There's a stray ";" after the if statement, that was obviously not
> tested :-)
Grr sorry about that, and thanks for catching this.
Eric
> This fixes it here (tested):
> ----------------------------------------------------------------------------
> [PATCH] Fix new dev_ifname32 returning -EFAULT
>
> A stray semicolon slipped in the patch that updated dev_ifname32 to
> not be inline, causing it to always return -EFAULT. This fixes it.
>
> Signed-off-by: Benjamin Herrenschmidt <[email protected]>
> ---
>
> Index: linux-work/fs/compat_ioctl.c
> ===================================================================
> --- linux-work.orig/fs/compat_ioctl.c 2007-10-31 13:30:42.000000000 +1100
> +++ linux-work/fs/compat_ioctl.c 2007-10-31 13:30:46.000000000 +1100
> @@ -322,7 +322,7 @@ static int dev_ifname32(unsigned int fd,
> int err;
>
> uifr = compat_alloc_user_space(sizeof(struct ifreq));
> - if (copy_in_user(uifr, compat_ptr(arg), sizeof(struct ifreq32)));
> + if (copy_in_user(uifr, compat_ptr(arg), sizeof(struct ifreq32)))
> return -EFAULT;
>
> err = sys_ioctl(fd, SIOCGIFNAME, (unsigned long)uifr);