[cc'ing xwin-discuss, since 6948630 was a change to X, not SFW]
While we could probably drop the "cast to (struct sockaddr *)" part of
6948630, I believe the other portion is still correct and better than
the previous code.
Previously the X code did:
struct sockaddr broad_addr;
/* check if it's the sort of address we want*/
broad_addr = *ifr->ifa_broadaddr;
XdmcpRegisterBroadcastAddress((struct sockaddr_in *) &broad_addr);
The previous code caused the compiler to refuse to overflow the stack
by doing a struct copy of a 246 byte struct sockaddr_storage to a
16-byte struct sockaddr allocation on the stack. Now that both sides
of the copy are struct sockaddr, the compiler would no longer bitch, and
just only copy the first 16 bytes - which is probably safe since this is
specifically looking for IPv4 broadcast addresses (multicast for both
IPv4 & v6 being handled elsewhere), but still 16 bytes of pointless copying.
The fix was simply to replace all three of the above lines with:
XdmcpRegisterBroadcastAddress(
(struct sockaddr_in *) ifr->ifa_broadaddr);
avoiding all the size mismatch issues and needless copy to stack.
X.Org upstream agreed and accepted this change into the next release
for all platforms.
-alan-
Darren Reed wrote:
> Now that this fix has been putback, consideration should be given
> to removing the changes applied with 6944247 and 6948630.
>
> Darren
>
> -------- Original Message --------
> *Synopsis*: ifa_addr and friends should be "sockaddr", not "sockaddr_storage"
>
> Bugster:
> http://monaco.sfbay.sun.com/jnlpargs/bugster.jnlp?jnlp_url=http://bugster.central.sun.com/bugster.jnlp&arg=6950944
> CrPrint: http://bt2ws.central.sun.com/CrPrint?id=6950944
> Monaco: http://monaco.sfbay.sun.com/detail.jsf?cr=6950944
>
> CR 6950944 changed on Jul 10 2010 by [email protected]
>
> === Field ============ === New Value ============= === Old Value =============
>
> Fixed in Build snv_145
> Status 8-Fix Available 7-Fix in Progress
> ====================== =========================== ===========================
>
>
> *Change Request ID*: 6950944
>
> *Synopsis*: ifa_addr and friends should be "sockaddr", not "sockaddr_storage"
>
> Product: solaris
> Category: library
> Subcategory: network-other
> Type: Defect
> Subtype: Interoperability
> Status: 8-Fix Available
> Substatus:
> Priority: 2-High
> Introduced In Release: solaris_nevada
> Introduced In Build: snv_137
> Responsible Manager: [email protected]
> Responsible Engineer: [email protected]
> Initial Evaluator: [email protected]
> Keywords:
>
> === *Description* ============================================================
> In the Solaris implementation of getifaddrs, the structure "ifaddrs" has been
> built using "struct sockaddr_storage *" for ifa_addr and ifa_netmask. This
> makes it, out of the box, incompatible with just about everything else in
> terms of source code.
>
> The goal of #6731945 was to provide the BSD API getifaddrs(). That means
> making it source code compatible so that open source compiles without any
> errors.
>
> It does not matter if we use "sockaddr_storage" internally, what matters is
> that the external structre, "struct ifaddrs", matches that found outside of
> OpenSolaris.
>
> *** (#1 of 1): 2010-05-09 05:50:22 GMT+00:00 [email protected]
>
>
> === *Public Comments* ========================================================
> The fix is simple: just change the "struct sockaddr_storage" pointers in
> <ifaddrs.h> to "struct sockaddr".
>
> All of the code that implements this library function allocates storage space
> using "sizeof (struct sockaddr_storage)" so there is no problem with the
> buffer allocated being over run.
>
> Additionally, because this is libc, free() doesn't care about the size.
>
> *** (#1 of 1): 2010-05-09 06:10:52 GMT+00:00 [email protected]
>
>
> === *Comments* ===============================================================
>
> === *Evaluation* =============================================================
> See Public Comments.
>
> *** (#1 of 1): 2010-05-09 06:10:52 GMT+00:00 [email protected]
>
>
> === *Suggested Fix* ==========================================================
> diff -r 6693c22bc83a -r 68c191027739 usr/src/head/ifaddrs.h
> --- a/usr/src/head/ifaddrs.h Sat May 08 10:40:19 2010 -0700
> +++ b/usr/src/head/ifaddrs.h Sat May 08 23:10:23 2010 -0700
> @@ -40,10 +40,8 @@
> struct ifaddrs *ifa_next; /* Pointer to the next structure. */
> char *ifa_name; /* Name of this network interface. */
> uint64_t ifa_flags; /* Flags as from SIOCGLIFFLAGS ioctl.
> */
> - struct sockaddr_storage *ifa_addr;
> - /* Network address of this interface.
> */
> - struct sockaddr_storage *ifa_netmask;
> - /* Netmask of this interface. */
> + struct sockaddr *ifa_addr; /* Network address of this interface.
> */
> + struct sockaddr *ifa_netmask; /* Netmask of this interface. */
> union {
> /*
> * At most one of the following two is valid. If the
> @@ -52,8 +50,8 @@
> * set, then `ifa_dstaddr' is valid. It is never the case that
> * both these bits are set at once.
> */
> - struct sockaddr_storage *ifu_broadaddr;
> - struct sockaddr_storage *ifu_dstaddr;
> + struct sockaddr *ifu_broadaddr;
> + struct sockaddr *ifu_dstaddr;
> } ifa_ifu;
> void *ifa_data; /* Address-specific data (may be unused).
> */
> /*
>
> *** (#1 of 1): 2010-05-09 06:10:52 GMT+00:00 [email protected]
--
-Alan Coopersmith- [email protected]
Oracle Solaris Platform Engineering: X Window System
_______________________________________________
xwin-discuss mailing list
[email protected]
List info: http://mail.opensolaris.org/mailman/listinfo/xwin-discuss
Unsubscribe: http://mail.opensolaris.org/mailman/options/xwin-discuss
