M. Warner Losh wrote:
> In message: <201008101313.o7addhye040...@haluter.fromme.com>
> Oliver Fromme <o...@fromme.com> writes:
> : M. Warner Losh wrote:
> : > The casting that syslogd does with struct sockaddrFOO is what triggers
> : > it. I'm not sure how to fix it, so there's about 6 or 8 programs in
> : > the tree that have WARNS lowered to 3 because of it.
> :
> : I've given it try, please see the patch below.
> : This is not really pretty, but it's a start.
> : It builds with WARNS=6 on all archs, including mips.
> :
> : What do you think?
Please ignore that patch. It compiles, but has some problems.
des pointed out that it makes sense to use a macro for type
casts. The new patch below does this for the cases where
simply using struct sockaddr_storage doesn't work.
> [...]
> : @@ -2409,9 +2414,9 @@
> : continue;
> : }
> : reject = 0;
> : - for (j = 0; j < 16; j += 4) {
> : - if ((*(u_int32_t
> *)&sin6->sin6_addr.s6_addr[j] & *(u_int32_t *)&m6p->sin6_addr.s6_addr[j])
> : - != *(u_int32_t
> *)&a6p->sin6_addr.s6_addr[j]) {
> : + for (j = 0; j < 16; j++) {
> : + if ((sin6->sin6_addr.s6_addr[j] &
> m6p->sin6_addr.s6_addr[j])
> : + != a6p->sin6_addr.s6_addr[j]) {
>
> This code looks better, but why have the casts been removed? I take
> it they aren't necessary?
Right. On the other hand, the loop iterations are increased
from 4 to 16, making the whole thing less efficient.
In the new patch below I introduced a typecast macro for
this case, too, so the iterations stay at 4.
Again, the new patch passes the universe test.
Best regards
Oliver
--- syslogd.c.orig 2010-08-05 21:59:11.000000000 +0200
+++ syslogd.c 2010-08-10 18:15:46.000000000 +0200
@@ -123,6 +123,15 @@
#define MAXUNAMES 20 /* maximum number of user names */
/*
+ * Macros to cast a struct sockaddr, or parts thereof.
+ * These are needed to silence the compiler on architectures
+ * with strict alignment requirements.
+ */
+
+#define SIN_CAST(sa) ((struct sockaddr_in *)(uintptr_t)(sa))
+#define UINT32_CAST(c) (*(u_int32_t *)(uintptr_t)&(c))
+
+/*
* Unix sockets.
* We have two default sockets, one with 666 permissions,
* and one for privileged programs.
@@ -330,8 +339,8 @@
static void readklog(void);
static void reapchild(int);
static void usage(void);
-static int validate(struct sockaddr *, const char *);
-static void unmapped(struct sockaddr *);
+static int validate(struct sockaddr_storage *, const char *);
+static void unmapped(struct sockaddr_storage *);
static void wallmsg(struct filed *, struct iovec *, const int iovlen);
static int waitdaemon(int, int, int);
static void timedout(int);
@@ -652,8 +661,8 @@
if (l > 0) {
line[l] = '\0';
hname = cvthname((struct
sockaddr *)&frominet);
- unmapped((struct sockaddr
*)&frominet);
- if (validate((struct sockaddr
*)&frominet, hname))
+ unmapped(&frominet);
+ if (validate(&frominet, hname))
printline(hname, line,
RemoteAddDate ? ADDDATE : 0);
} else if (l < 0 && errno != EINTR)
logerror("recvfrom inet");
@@ -678,17 +687,17 @@
}
static void
-unmapped(struct sockaddr *sa)
+unmapped(struct sockaddr_storage *ss)
{
struct sockaddr_in6 *sin6;
struct sockaddr_in sin4;
- if (sa->sa_family != AF_INET6)
+ if (ss->ss_family != AF_INET6)
return;
- if (sa->sa_len != sizeof(struct sockaddr_in6) ||
- sizeof(sin4) > sa->sa_len)
+ if (ss->ss_len != sizeof(struct sockaddr_in6) ||
+ sizeof(sin4) > ss->ss_len)
return;
- sin6 = (struct sockaddr_in6 *)sa;
+ sin6 = (struct sockaddr_in6 *)ss;
if (!IN6_IS_ADDR_V4MAPPED(&sin6->sin6_addr))
return;
@@ -699,7 +708,7 @@
sizeof(sin4.sin_addr));
sin4.sin_port = sin6->sin6_port;
- memcpy(sa, &sin4, sin4.sin_len);
+ memcpy(ss, &sin4, sin4.sin_len);
}
static void
@@ -1186,7 +1195,7 @@
break;
case F_FORW:
- port = (int)ntohs(((struct sockaddr_in *)
+ port = (int)ntohs((SIN_CAST
(f->f_un.f_forw.f_addr->ai_addr))->sin_port);
if (port != 514) {
dprintf(" %s:%d\n", f->f_un.f_forw.f_hname, port);
@@ -1711,7 +1720,7 @@
break;
case F_FORW:
- port = (int)ntohs(((struct sockaddr_in *)
+ port = (int)ntohs((SIN_CAST
(f->f_un.f_forw.f_addr->ai_addr))->sin_port);
if (port != 514) {
printf("%s:%d",
@@ -2340,7 +2349,7 @@
* Validate that the remote peer has permission to log to us.
*/
static int
-validate(struct sockaddr *sa, const char *hname)
+validate(struct sockaddr_storage *ss, const char *hname)
{
int i;
size_t l1, l2;
@@ -2369,8 +2378,8 @@
strlcat(name, ".", sizeof name);
strlcat(name, LocalDomain, sizeof name);
}
- if (getnameinfo(sa, sa->sa_len, ip, sizeof ip, port, sizeof port,
- NI_NUMERICHOST | NI_NUMERICSERV) != 0)
+ if (getnameinfo((struct sockaddr *)ss, ss->ss_len, ip, sizeof ip, port,
+ sizeof port, NI_NUMERICHOST | NI_NUMERICSERV) != 0)
return (0); /* for safety, should not occur */
dprintf("validate: dgram from IP %s, port %s, name %s;\n",
ip, port, name);
@@ -2384,12 +2393,12 @@
}
if (ap->isnumeric) {
- if (ap->a_addr.ss_family != sa->sa_family) {
+ if (ap->a_addr.ss_family != ss->ss_family) {
dprintf("rejected in rule %d due to address
family mismatch.\n", i);
continue;
}
if (ap->a_addr.ss_family == AF_INET) {
- sin4 = (struct sockaddr_in *)sa;
+ sin4 = (struct sockaddr_in *)ss;
a4p = (struct sockaddr_in *)&ap->a_addr;
m4p = (struct sockaddr_in *)&ap->a_mask;
if ((sin4->sin_addr.s_addr &
m4p->sin_addr.s_addr)
@@ -2400,7 +2409,7 @@
}
#ifdef INET6
else if (ap->a_addr.ss_family == AF_INET6) {
- sin6 = (struct sockaddr_in6 *)sa;
+ sin6 = (struct sockaddr_in6 *)ss;
a6p = (struct sockaddr_in6 *)&ap->a_addr;
m6p = (struct sockaddr_in6 *)&ap->a_mask;
if (a6p->sin6_scope_id != 0 &&
@@ -2410,8 +2419,8 @@
}
reject = 0;
for (j = 0; j < 16; j += 4) {
- if ((*(u_int32_t
*)&sin6->sin6_addr.s6_addr[j] & *(u_int32_t *)&m6p->sin6_addr.s6_addr[j])
- != *(u_int32_t
*)&a6p->sin6_addr.s6_addr[j]) {
+ if
((UINT32_CAST(sin6->sin6_addr.s6_addr[j]) &
UINT32_CAST(m6p->sin6_addr.s6_addr[j]))
+ !=
UINT32_CAST(a6p->sin6_addr.s6_addr[j])) {
++reject;
break;
}
_______________________________________________
svn-src-head@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
