Opened 12 years ago

Closed 12 years ago

Last modified 12 years ago

#1898 closed bug (fixed)

getpeername returns size bigger than passed in?

Reported by: tqh Owned by: axeld
Priority: normal Milestone: R1
Component: Network & Internet/Stack Version: R1/pre-alpha1
Keywords: Cc:
Blocked By: Blocking:
Has a Patch: no Platform: All


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:

Called from here:

Here is BeOS definition of _PR_MD_GETPEERNAME (Haiku has _PR_HAVE_SOCKADDR_LEN defined):

And here is the wonderful definition of PrNetAddr:

Simple huh :)

Change History (15)

comment:1 by axeld, 12 years ago

Resolution: fixed
Status: newclosed

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.

comment:2 by axeld, 12 years ago

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).


comment:3 by tqh, 12 years ago

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..

comment:4 by tqh, 12 years ago

Resolution: fixed
Status: closedreopened

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


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

Second call: _MD_getpeername:addrlen = 28


_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.

comment:5 by axeld, 12 years ago

Resolution: fixed
Status: reopenedclosed

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.

comment:6 by tqh, 12 years ago

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 "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)."

comment:7 by axeld, 12 years ago

I agree that 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.

comment:8 by tqh, 12 years ago

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.

comment:9 by axeld, 12 years ago

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?

comment:10 by tqh, 12 years ago

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.

comment:11 by tqh, 12 years ago

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.

comment:12 by axeld, 12 years ago

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.

comment:13 by tqh, 12 years ago

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.

comment:14 by tqh, 12 years ago

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).

in reply to:  14 comment:15 by bonefish, 12 years ago

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.

Note: See TracTickets for help on using tickets.