Date: Fri, 15 Jul 2005 02:28:48 -0700 From: Sean McNeil <sean@mcneil.com> To: gnome@freebsd.org Subject: gamin broken on amd64 Message-ID: <1121419728.4388.31.camel@server.mcneil.com>
next in thread | raw e-mail | index | archive | help
--=-STBEm4VZIWKqr0fButKe Content-Type: text/plain Content-Transfer-Encoding: 7bit msg credentials are difficult to get right in a portable fashion. I'm not so certain that FreeBSD is doing things right here, but I have a patch to resolve the issue. Typically, one would create a structure that includes the cmsghdr + the actual message going across. server/gam_channel.c is a classic example. It defines a cmsg variable as struct { struct cmsghdr hdr; struct cmsgcred cred; } cmsg; Even though the documentation says that the sender is suppose to reserve the space in msg.msg_control, it appears that FreeBSD will allocate its own space and fill it in. When it does so, it uses the CMSG_DATA() to get the start of what should be struct cmsgcred cred. But it aligns based on the _ALIGN() macro. For AMD64, this is 8 bytes. But this means that when read on the other end, the credentials are all hosed up because they are off by 4 bytes. sys/socket.h says: /* * Header for ancillary data objects in msg_control buffer. * Used for additional information with/about a datagram * not expressible by flags. The format is a sequence * of message elements headed by cmsghdr structures. */ struct cmsghdr { socklen_t cmsg_len; /* data byte count, including hdr */ int cmsg_level; /* originating protocol */ int cmsg_type; /* protocol-specific type */ /* followed by u_char cmsg_data[]; */ }; technically, this is not true. cmsg_data[] is really adjusted to the alignment of the machine. Makes sense, I guess, since you probably want the data starting in the same place regardless of whether it has any long variables in it. Further, if there are any long values, then you would want to make sure that msg.msg_control is pointing to a properly aligned memory location. None of the examples I've seen take this into account. This means the safest way to do things would be to have something like struct cmsghdr *cmsg = malloc (CMSG_SPACE(sizeof(struct cmsgcred))); struct cmsgcred *cred = (struct cmsgcred)CMSG_DATA(cmsg); This, IMHO, is the most portable and "proper" way to accomplish things. But I just wanted a cheap and dirty solution. To solve this easily, I added an alignment attribute and changed a verification test. I created a new patch-server_gam_channel.c: --- server/gam_channel.c.orig Fri Jul 15 01:13:44 2005 +++ server/gam_channel.c Fri Jul 15 01:50:54 2005 @@ -29,10 +29,10 @@ { char data[2] = { 0, 0 }; int written; -#if defined(HAVE_CMSGCRED) && !defined(LOCAL_CREDS) +#if defined(HAVE_CMSGCRED) && (!defined(LOCAL_CREDS) || defined(__FreeBSD__)) struct { struct cmsghdr hdr; - struct cmsgcred cred; + struct cmsgcred cred __attribute__ ((aligned (_ALIGN(1)))); } cmsg; struct iovec iov; struct msghdr msg; @@ -53,7 +53,7 @@ #endif retry: -#if defined(HAVE_CMSGCRED) && !defined(LOCAL_CREDS) +#if defined(HAVE_CMSGCRED) && (!defined(LOCAL_CREDS) || defined(__FreeBSD__)) written = sendmsg(fd, &msg, 0); #else written = write(fd, &data[0], 1); @@ -94,13 +94,13 @@ #ifdef HAVE_CMSGCRED struct { struct cmsghdr hdr; - struct cmsgcred cred; + struct cmsgcred cred __attribute__ ((aligned (_ALIGN(1)))); } cmsg; #endif s_uid = getuid(); -#if defined(LOCAL_CREDS) && defined(HAVE_CMSGCRED) +#if defined(LOCAL_CREDS) && defined(HAVE_CMSGCRED) && !defined(__FreeBSD__) /* Set the socket to receive credentials on the next message */ { int on = 1; @@ -139,9 +139,9 @@ goto failed; } #ifdef HAVE_CMSGCRED - if (cmsg.hdr.cmsg_len < sizeof(cmsg) || cmsg.hdr.cmsg_type != SCM_CREDS) { + if (cmsg.hdr.cmsg_len > sizeof(cmsg) || cmsg.hdr.cmsg_type != SCM_CREDS) { GAM_DEBUG(DEBUG_INFO, - "Message from recvmsg() was not SCM_CREDS\n"); + "Message from recvmsg() was not SCM_CREDS.\n"); goto failed; } #endif This got me working again on AMD64. I've also attached it to assure proper spacing and such. If I ever get enough time, I can whip up a "correct" patch for this. By the way, the gnome keyring manager has this same issue, but the fortunate thing is that looking at the effective uid gives you the uid, which is usually the same. gamin croaks because the pid ends up being 0 from the 4-byte offset. Cheers, Sean --=-STBEm4VZIWKqr0fButKe--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?1121419728.4388.31.camel>