1330490 - memset 0 of the entire PRNetAddr in PR_InitializeNetAddr

Status: RESOLVED FIXED

Description

[Shih-Chiang Chien [:schien] (UTC+8) (use ni? plz)]

PR_InitializeNetAddr only memset 0 for ipv4 struct. We should clear the entire PRNetAddr for supporting ipv6 as well.

See bug 1326483 comment #24 for the detail.

Comment 1

[Shih-Chiang Chien [:schien] (UTC+8) (use ni? plz)]

Created attachment 8826072 [details]
Bug 1330490 - clear entire PRNetAddr struct in PR_InitializeNetAddr for IPv6.

Review commit: https://reviewboard.mozilla.org/r/104122/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/104122/

Comment 2

[Nils Ohlmeier [:drno]]

I confirm that this fixes the problem I found in bug 1333185.

BTW I think that PR_InitializeNetAddr() needs the IP address family as an input, because even though this properly memset's the struct now it still then does everything based on the AF_INET (even when that is the wrong address family). But that is probably for another ticket to fix.

Comment 3

[Ted Mielczarek [:ted] [:ted.mielczarek]]

Comment on attachment 8826072 [details]
Bug 1330490 - clear entire PRNetAddr struct in PR_InitializeNetAddr for IPv6.

https://reviewboard.mozilla.org/r/104122/#review110708

::: nsprpub/pr/src/misc/prnetdb.c:1408
(Diff revision 1)
>      PRNetAddrValue val, PRUint16 port, PRNetAddr *addr)
>  {
>      PRStatus rv = PR_SUCCESS;
>      if (!_pr_initialized) _PR_ImplicitInitialization();
>  
> -	if (val != PR_IpAddrNull) memset(addr, 0, sizeof(addr->inet));
> +	if (val != PR_IpAddrNull) memset(addr, 0, sizeof(*addr));

I don't *think* this should cause any problems, but I was looking at the history here, and there are some comments on bug 54796 that are a little concerning. I'm not sure they're still relevant nowadays, though.

Comment 4

[Shih-Chiang Chien [:schien] (UTC+8) (use ni? plz)]

I double checked the current mozilla-central and doesn't found any code that passes sockaddr_in to PR_InitializedNetAddr.

I'm not sure about the proper way to move this bug forward since NSPR is a shared library to many project. Do I simply land this bug via mozreview?

Comment 5

[Ted Mielczarek [:ted] [:ted.mielczarek]]

No, this needs to land in the NSPR repository.

Comment 6

[Shih-Chiang Chien [:schien] (UTC+8) (use ni? plz)]

Created attachment 8836592 [details] [diff] [review]
[patch for nspr repo] bug1330490.patch

Need someone to land this patch on nspr repo. @ted can you help?

Comment 7

[Shih-Chiang Chien [:schien] (UTC+8) (use ni? plz)]

@kaie can you help merge this patch to NSPR repo?

Comment 8

[Kai Engert (:kaie:)]

https://hg.mozilla.org/projects/nspr/rev/2f1bfbcc79baac751f8bd83a9f799f33366b8d3a