getpeername returns size bigger than passed in?

[tqh]

Currently investigating Firefox (gcc4) crashes on SSL. One of the things I'm seeing is at least one case where calling getpeername with a size of 28 returns one with size 32. Unfortunatly it's quite messy on Firefox side so I'll just show how it's called:

It fails this check, it has other problems as well though: ​http://lxr.mozilla.org/mozilla1.8.0/source/nsprpub/pr/src/io/prsocket.c#60

Called from here: ​http://lxr.mozilla.org/mozilla1.8.0/source/nsprpub/pr/src/io/prsocket.c#1086

Here is BeOS definition of _PR_MD_GETPEERNAME (Haiku has _PR_HAVE_SOCKADDR_LEN defined): ​http://lxr.mozilla.org/mozilla1.8.0/source/nsprpub/pr/src/md/beos/bnet.c#772

And here is the wonderful definition of PrNetAddr: ​http://lxr.mozilla.org/mozilla1.8.0/source/nsprpub/pr/include/prio.h#174

Simple huh :)

[axeld]

Good catch! Indeed, return_address() in kernel_stack.cpp did not take the original buffer size into account when copying the address. Fixed in hrev24310.

[axeld]

BTW getpeername() (and friends) are actually supposed to return the length of the new address returned, even if it's longer than the original buffer (it's just not supposed to overwrite bytes in the target buffer that don't fit the address).

See ​http://www.opengroup.org/onlinepubs/009695399/functions/getpeername.html

[tqh]

Yes, I added a question mark because I wasn't really sure. Although you are only confusing me, I expect that getting a size bigger than I pass in shouldn't happen.

I'll probably find some more stuff as I go along. I'll report them and let you guys sort them out. I think most bugs are on my side though..

[tqh]

I think there are still issues, these are just printfs with what I get from Haiku's getpeername: First call: _MD_getpeername:addrlen = 28

[called]

_MD_getpeername:addrlen after = 28 _MD_getpeername:sockaddr.sa_len after = 82

Second call: _MD_getpeername:addrlen = 28

[called]

_MD_getpeername:addrlen after = 32 _MD_getpeername:sockaddr.sa_len after = 32

First call has very strange things going on with the sa_len field. Mozilla treats sa_len and sa_family and that is set to 8786 in decimal. That's 0x2252 which I can't make any sense of at all. Second at least seems to be ok in general as family is AF_INET at least but there the it returns a bigger size.

[axeld]

As I said, it is supposed to return the *actual* size, not the one of the buffer. The first call looks like some missing initialization, so it does not have anything to do with the original bug.

Please file a new bug report for this problem, and also mention the circumstances, ie. what kind of socket, is it already connected, bound, in use, whatever.

[tqh]

That is not how I understand it, also it is not how it is used on ANY other platform in Mozilla.

Here is from FreeBSD ​http://www.freebsd.org/cgi/man.cgi?query=getpeername&apropos=0&sektion=0&manpath=FreeBSD+6.3-RELEASE&format=html: "The namelen argument should be initialized to indicate the

amount of space pointed to by name. On return it contains the actual size of the name returned (in bytes)."

[axeld]

I agree that ​http://www.opengroup.org/onlinepubs/009695399/functions/getpeername.html is not as clear as one could wish for. However, quoting from the POSIX standard 1003.1g: "[...] fromlen shall refer to the value before truncation [...]"

I've also written a small test application, and at least Linux behaves exactly like that, too. It might be different on other platforms, but I prefer following the standard.

[tqh]

Ok, thanks for clearing that up. Following the spec is fine by me. Do you know how BeOS behaves in this case, I guess I'll have to do some adaption at my end and it would be nice to know.

[axeld]

I've added the getpeername.cpp test program to the repository, and tested it under Dano (ie. BONE), too. There, getpeername() behaves exactly as you suggested (and obviously as FreeBSD), ie. it returns the buffer size if that is smaller than the actual length of the address.

Could you please open a ticket for the case with the strange values? Or did getpeername() just fail in this case?

[tqh]

I'm not sure what's going on with those values. I want to investigate on my end first. I just posted them to see if it was something known. I'll post if I think it's Haiku's fault.

[tqh]

I think I'll change that check to see if is same as sockaddr (32) bytes, although I'll need to do that at runtime to support BeOS as well. I'll also need to increase PRNetAddr to 32 bytes to be as large as sockaddr in Haiku. Is there any other things I should be aware of in regards of sizeof sockaddr. Linux and others seems to have different sizes depending on family if I understand the check correctly, but I assume we only use 32 bytes.

Anyway working fine with hardcoded check for 32 bytes. Posting with it now.

[axeld]

I'm not sure why the size is that important. We actually have different sizes per family, too, it's just that we stayed compatible with BeOS with regards to sockaddr and sockaddr_in. For sockaddr_in6 and others, there is no need to stay compatible, though. We also have a sockaddr_storage which is 256 bytes in size.

The last bytes in sockaddr_in are always padding, though, and can be neglected.

[tqh]

Mozilla does sanitychecks on the size, as we don't have anything to go on we should either not sanitycheck or check that the addrlen is the same as a sockaddr. Other platforms check that the returned value is the same as a socket for that family, but afaict Haiku returns 32 for inet at least, while other platforms thinks that it should be 16.

[tqh]

I think hrev24948 is/was the fix to the problem from my point of view. And yes, Firefox does pass in a smaller buffer than Haiku wants (BeOS seems to have had a smaller one).

[bonefish]

Replying to tqh:

I think hrev24948 is/was the fix to the problem from my point of view. And yes, Firefox does pass in a smaller buffer than Haiku wants (BeOS seems to have had a smaller one).

I really only fixed a bug I introduced a few days ago. Haiku's sockaddr_in is BONE compatible, which is 32 bytes big.