Bug 4982 - [PATCH] please use offsetof for manipulating struct sockaddr_un
[PATCH] please use offsetof for manipulating struct sockaddr_un
Status: RESOLVED FIXED
Product: xorg
Classification: Unclassified
Component: Lib/xtrans
git
x86 (IA32) other
: high normal
Assigned To: Alan Coopersmith
: patch
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2005-11-07 13:26 UTC by petr.salinger
Modified: 2006-08-24 17:48 UTC (History)
1 user (show)

See Also:
i915 platform:
i915 features:


Attachments
updated patch for CVS HEAD (1.05 KB, patch)
2006-04-22 00:39 UTC, Robert Millan
no flags Details | Splinter Review
updated patch for CVS HEAD (1.05 KB, patch)
2006-04-23 01:31 UTC, petr.salinger
no flags Details | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description petr.salinger 2005-11-07 13:26:49 UTC
On debian/kFreeBSD is not defined HasBSD44Sockets.
It should't be problem because
FreeBSD kernel 5.4 and 6.0 tries to be compatible with older (BSD43) source code
and at least in connect(), bind() ignores value in the field sun_len.
It uses only the value addrlen from function in system call.

Unfortunately, the we are hitted by computing the whole name length as
namelen = strlen(sockname.sun_path) + sizeof(sockname.sun_family);
instead of portable
namelen = strlen(sockname.sun_path) + offsetof(struct sockaddr_un, sun_path);

Therefore, it would be sufficient to correct calculation with sun_path in 
whole xorg-x11. Moreover, it is mainly correct, only 2 files need fix.

ok      programs/xdm/prngc.c
ok      programs/xdm/xdmcp.c
ok      programs/xdm/netaddr.c
patch   lib/dps/csconndi.c
patch   lib/xtrans/Xtranssock.c
ok      lib/xtrans/Xtranstli.c
ok      lib/xtrans/Xtranslcl.c
ok      lib/xtrans/Xtransutil.c
ok      extras/Mesa/src/glx/mini/miniglx_events.c       

The  patch should be straightforward:
Please, could you apply it.
Thanks in advance

Petr



--- lib/dps/csconndi.c~ 2005-11-07 20:59:23.000000000 +0000
+++ lib/dps/csconndi.c  2005-11-07 20:59:23.000000000 +0000
@@ -506,7 +506,7 @@
     sprintf (unaddr.sun_path, "%s_%d", CSDPS_UNIX_PATH, port);
 
     addr = (struct sockaddr *) &unaddr;
-    addrlen = strlen(unaddr.sun_path) + sizeof(unaddr.sun_family);
+    addrlen = strlen(unaddr.sun_path) + offsetof(struct sockaddr_un, sun_path));
 
     /*
      * Open the network connection.


--- lib/xtrans/Xtranssock.c~    2005-11-07 21:02:32.000000000 +0000
+++ lib/xtrans/Xtranssock.c     2005-11-07 21:02:32.000000000 +0000
@@ -1092,7 +1092,7 @@
     sockname.sun_len = strlen(sockname.sun_path);
     namelen = SUN_LEN(&sockname);
 #else
-    namelen = strlen(sockname.sun_path) + sizeof(sockname.sun_family);
+    namelen = strlen(sockname.sun_path) + offsetof(struct sockaddr_un, sun_path);
 #endif
 
     unlink (sockname.sun_path);
@@ -1975,7 +1975,7 @@
     sockname.sun_len = strlen (sockname.sun_path);
     namelen = SUN_LEN (&sockname);
 #else
-    namelen = strlen (sockname.sun_path) + sizeof (sockname.sun_family);
+    namelen = strlen (sockname.sun_path) + offsetof(struct sockaddr_un, sun_path);
 #endif
 
 
@@ -1989,7 +1989,7 @@
        return TRANS_CONNECT_FAILED;
     }
     old_namelen = strlen (old_sockname.sun_path) +
-       sizeof (old_sockname.sun_family);
+       offsetof(struct sockaddr_un, sun_path);
 #endif
Comment 1 Aurelien Jarno 2006-01-02 00:49:54 UTC
Actually according to POSIX1.2004, you don't even't need to use strlen and 
offsetof, you can use sizeof(struct sockaddr_un) instead. Tested with a 
FreeBSD kernel and a Linux kernel. 
  
POSIX1.2004, page 263 says:    
"address_len   Specifies the length of the sockaddr structure pointed to by    
the address argument."    
     
Comment 2 Robert Millan 2006-04-22 00:38:09 UTC
Reassigning to modular xlibs.
Comment 3 Robert Millan 2006-04-22 00:39:11 UTC
Created attachment 5404 [details] [review]
updated patch for CVS HEAD
Comment 4 Daniel Stone 2006-04-22 00:57:43 UTC
... and back to xorg.  xlibs is dead.
Comment 5 petr.salinger 2006-04-23 01:31:40 UTC
Created attachment 5424 [details] [review]
updated patch for CVS HEAD

there have been a typo in last chunk
Comment 6 Alan Coopersmith 2006-08-24 17:48:10 UTC
Patch committed to git head:

commit 2495789d6c290e2037b2836f28b027786ea5b605
Author: Petr Salinger <petr.salinger@t-systems.cz>
Date:   Sun Apr 23 01:31:00 2006 -0700

    Bug 4982: use offsetof for manipulating struct sockaddr_un
    
    X.Org Bugzilla #4982 <https://bugs.freedesktop.org/show_bug.cgi?id=4982>
    Patch #5424 <https://bugs.freedesktop.org/attachment.cgi?id=5424>

Thanks for the patch!

(I've also committed a followon that uses SUN_LEN if it's defined, even when
 BSD44SOCKETS is not.)