Skip site navigation (1)Skip section navigation (2)
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>