bug in 3.2.0 nmblookup

freebsd 4.6.2 at least, the getnameinfo function expects the length
field inside the struct sockaddr to match the salen arg or it returns
an error. This causes nmblookup to not print the address of the found
name in a query like the following

nmblookup -R -U 172.17.132.16 nt4pdc#20

The following patch fixes this. I don't have IPV6 so could not actually
test the AF_INET6 portion of the fix.


/tmp/tmp.66554.0 Thu Jul 3 16:02:36 2008
source/lib/util_sock.c Thu Jul 3 16:02:00 2008
@@ -366,6 +366,7 @@
struct sockaddr_in *sa = (struct sockaddr_in *)ss;
memset(ss, '\0', sizeof(*ss));
ss->ss_family = AF_INET;
+ ss->ss_len = sizeof(struct sockaddr_in);
sa->sin_addr = ip;
}

@@ -380,6 +381,7 @@
struct sockaddr_in6 *sa = (struct sockaddr_in6 *)ss;
memset(ss, '\0', sizeof(*ss));
ss->ss_family = AF_INET6;
+ ss->ss_len = sizeof(struct sockaddr_in6);
sa->sin6_addr = ip;
}

Thu, Jul 03, 2008 at 04:18:29PM -0700, Herb Lewis wrote:
freebsd 4.6.2 at least, the getnameinfo function expects the length
field inside the struct sockaddr to match the salen arg or it returns
an error. This causes nmblookup to not print the address of the found
name in a query like the following
>
nmblookup -R -U 172.17.132.16 nt4pdc#20
>
The following patch fixes this. I don't have IPV6 so could not actually
test the AF_INET6 portion of the fix.
>
>
/tmp/tmp.66554.0 Thu Jul 3 16:02:36 2008
source/lib/util_sock.c Thu Jul 3 16:02:00 2008
@@ -366,6 +366,7 @@
struct sockaddr_in *sa = (struct sockaddr_in *)ss;
memset(ss, '\0', sizeof(*ss));
ss->ss_family = AF_INET;
+ ss->ss_len = sizeof(struct sockaddr_in);
sa->sin_addr = ip;
}
>
@@ -380,6 +381,7 @@
struct sockaddr_in6 *sa = (struct sockaddr_in6 *)ss;
memset(ss, '\0', sizeof(*ss));
ss->ss_family = AF_INET6;
+ ss->ss_len = sizeof(struct sockaddr_in6);
sa->sin6_addr = ip;
}

The problem is that many systems don't have ss_len
inside their sockaddr_storage struct so this fix
as posted will break everything *but* BSD.

The correct place to fix this is in lib/system.c,
inside sys_getnameinfo(). Look at the fix inside
there that was added for Solaris :

/*
* For Solaris we must make sure salen is the
* correct length for the incoming sa_family.
*/

something similar could be added for *BSD. But make
sure it's not an #ifdef BSD, but a configure check
for ss_len first. And be careful, I remember AIX
has a nasty definition of ss_len that messes things
up. This is a can of worms you're opening here :-).

Jeremy.

Alan DeKok wrote:
Jeremy Allison wrote:

/tmp/tmp.66554.0 Thu Jul 3 16:02:36 2008
source/lib/util_sock.c Thu Jul 3 16:02:00 2008
@@ -366,6 +366,7 @@
struct sockaddr_in *sa = (struct sockaddr_in *)ss;
memset(ss, '\0', sizeof(*ss));
ss->ss_family = AF_INET;
+ ss->ss_len = sizeof(struct sockaddr_in);
sa->sin_addr = ip;
}

>
That looks like a pointer aliasing bug to me.
>
Newer versions of GCC are more aggressive about optimizations. They
may notice that the assignment "sa->sin_addr = ip" is just before the
closing brace and that "sa" is a local variable, which isn't used
after that. So the assignment can safely be optimized away.

Thats sick!
sa may be a local variable but it is a pointer. Since when did gcc think
that local pointers only point locally?
What if a function scope pointer was initialized from an argument struct
(for convenience) would they optimize away that last use of that pointer
too?

function blah(struct suff* arg) {
struct secret *secret=stuff->secret;
// does this get opimized away as secret is going out of scope
secret->word="shh";
}

If I'm not wrong, this must be a gcc bug.

Sam
Yes I got hit by this recently. The solution (horrible as it
sounds) was to memcpy back and forth between "struct sockaddr_storage"
and "struct sockaddr_in*".
>
Just a heads up. If you suddenly see that the IP's are always zero,
this is what's going on.
>
Alan DeKok.
>

Nope. IS C99 forbids pointers of different types from pointing to
the same memory location. So the code in Samba (and in many other
programs) violates the spec. Hence the ''fno-strict-aliasing'
argument
to GCC, which allows your programs to continue working.

Samba has no trouble compiling that code under -Wall (which turns on
-fstrict-aliasing and -Wstrict-aliasing). The compiler is frequently
smart enough to know whether aliasing is a real issue inside a given
function. The case in point was not such a case.

Zach

>/tmp/tmp.66554.0 Thu Jul 3 16:02:36 2008
>source/lib/util_sock.c Thu Jul 3 16:02:00 2008
>@@ -366,6 +366,7 @@
>struct sockaddr_in *sa = (struct sockaddr_in *)ss;
>memset(ss, '\0', sizeof(*ss));
>ss->ss_family = AF_INET;
>+ ss->ss_len = sizeof(struct sockaddr_in);
>sa->sin_addr = ip;
>}

That looks like a pointer aliasing bug to me.

Newer versions of GCC are more aggressive about optimizations. They
may notice that the assignment "sa->sin_addr = ip" is just before the
closing brace and that "sa" is a local variable, which isn't used
after that. So the assignment can safely be optimized away.

In the case above, the aliasing occurs in the same function, the
original pointer ("ss") is passed into the function, and the compiler is
required to act on all of those assignments (because neither of the
pointers can be proven to reference local variables). Furthermore, the
assignments don't reference the same memory, so the actual order of the
assignments is irrelevant (one of the common aliasing optimization
issues is that the compiler should be allowed to reorder the assignments
for distinct pointers, but it's irrelevant here). There's no possible
aliasing bug here.

That said, I still would've coded it as sa->sin_family = AF_INET.
Flip-flopping types is fugly. :)

Zach

Zachary Loafman wrote:
In the case above, the aliasing occurs in the same function, the
original pointer ("ss") is passed into the function, and the compiler is
required to act on all of those assignments (because neither of the
pointers can be proven to reference local variables).

That may be true. All I know is I ran into pointer aliasing issues
with pretty much the same code. Casting 'struct sockaddr_storage *' to
'struct sockaddr_in *', assigning to 'sa', and discovering that *some*
assignments to 'struct sockaddr_in *' were optimized away.

Furthermore, the
assignments don't reference the same memory, so the actual order of the
assignments is irrelevant (one of the common aliasing optimization
issues is that the compiler should be allowed to reorder the assignments
for distinct pointers, but it's irrelevant here). There's no possible
aliasing bug here.

I don't claim to be a compiler expert. I'm just trying to give a
heads up for potential issues.

Alan DeKok.

8nike.com has been totally updated which has many new items in stock,such as Nike,Addidas,
Gucci,Chanel,LV,Lacoste,Boss,Evisu,Puma,Bape,Polo and so on .They are all with most fashion,top quality and nice
price.We offer the fastest shipping with EMS,UPS,DHL,TNT,FEDEX ,and the payment way as Western Union,
Money Booker,Bank Transfer and Paypal,PAYPAL accept large order now!!!
We will always offer you the best price and nice service,if you prefer some,please contact us!Thanks!
Let's do best for you!
msn: china8nike at live.cn
e-mail: sara at cn-fr.com