[hipl-users] [PATCH] weird behavior in send_hip_addr fixed
- From: tkilappa@xxxxxxxxx
- To: hipl-users@xxxxxxxxxxxxx
- Date: Thu, 21 Jan 2010 15:24:02 +0200
Hello. Once again, thanks for the updates.
I found two more issues:
1)
The day before, I complained about the weird behavior when connecting
to a remote host with IPs in /etc/hosts and HITs in /etc/hip/hosts:
daemon info(getaddrinfo.c:586@gethosts_hit): Searching for a HIT value
for host 'my_test_host' from file '/etc/hip/hosts'.
info(getaddrinfo.c:622@gethosts_hit): Found a HIT/LSI value for host
'my_test_host' on line 6 of file '/etc/hip/hosts'.
die(builder.c:1728@hip_build_user_hdr): assertion failed
It took quite some debugging, but I found the error, it was caused by
send_hipd_addr() in libinet6/getaddrinfo.c. The function initially
reserves the HIP message in the beginning, but before the header is
constructed, it does not otherwise modify it.
This usually works the first time it's called within a program, since
the memory block will be full of zeros. However, on subsequent calls
when the message is either not cleared or is freed and reserved anew,
it will be filled with garble and the assertion in
hip_build_user_hdr() will fail.
Clearing the struct before starting to build it eliminates the error
(the struct must be cleared in the innermost for loop, since it may be
reused several times within the function). 'msg' is not used within
the function prior to the memset I inserted, so this should not
introduce any regressions.
2)
What misled me for the better part of a day was what gethosts_hit()
does after finding a matching HIT from the hosts file. The suspicious
block begins near line 635, at the comment dated "18.01.2008 16:49"
The former part of the function already loops over the substrings
within the line read from /etc/hip/hosts and finds out whether or not
it's a HIT or an LSI. For some reason, the block then loops through
the substrings again when we have already stored HIT in 'hit' (or LSI
in 'lsi') and the name in 'fqdn_str' (or 'name').
I took the liberty of modifying the block to reflect on the earlier
part of the function so it'd be more readable and not do the same work
again.
----
These modifications fix the issue from the day before, the debug log
now looks like this:
info(getaddrinfo.c:586@gethosts_hit): Searching for a HIT value for
host 'my_test_host' from file '/etc/hip/hosts'.
info(getaddrinfo.c:621@gethosts_hit): Found a HIT/LSI value for host
'my_test_host' on line 6 of file '/etc/hip/hosts'.
debug(getaddrinfo.c:751@send_hipd_addr): HIT:
2001:0011:c8d8:fdb2:c031:6e98:bb24:4afd
debug(getaddrinfo.c:752@send_hipd_addr): IP: 10.2.0.1
info(getaddrinfo.c:765@send_hipd_addr): Mapped a HIT to an IPv4 address:
2001:11:c8d8:fdb2:c031:6e98:bb24:4afd -> 10.2.0.1.
debug(getaddrinfo.c:778@send_hipd_addr): Peer hostname my_test_host
debug(lib/core/message.c:196@hip_sendto_hipd): Sending user message 0
to HIPD on socket 3
debug(lib/core/message.c:200@hip_sendto_hipd): Sent 160 bytes
debug(lib/core/message.c:252@hip_send_recv_daemon_info_internal:
Waiting to receive daemon info.
debug(lib/core/message.c:267@hip_send_recv_daemon_info_internal: 160
bytes received from HIP daemon
error(lib/core/message.c:280@hip_send_recv_daemon_info_internal: HIP
message contained an error.
This is repeated for each IP associated with the host. However,
despite the error message replies, when all of the IPs have been dealt
with, the program successfully connects to
2001:11:c8d8:fdb2:c031:6e98:bb24:4afd and works.
Attached is a patch for libinet6/getaddrinfo.c, use it however you want.
I'm still curious about both the host alias behavior in gethosts_hit()
and the error reply from hipd, I think I'll try tro figure them out.
--
-Tatu Kilappa <tatu.kilappa@xxxxxx>
=== modified file 'libinet6/getaddrinfo.c'
--- libinet6/getaddrinfo.c 2010-01-19 13:10:46 +0000
+++ libinet6/getaddrinfo.c 2010-01-21 13:07:53 +0000
@@ -616,67 +616,58 @@
fqdn_str = getitem(&list,i);
}
/* Here we have the domain name in "fqdn" and the HIT in "hit" or the
LSI in "lsi". */
- if( (strlen(name) == strlen(fqdn_str)) &&
- strcmp(name, fqdn_str) == 0 ){
+ if(!strcmp(name, fqdn_str)) {
HIP_INFO("Found a HIT/LSI value for host '%s' on line "\
"%d of file '%s'.\n", name, lineno, HIPL_HOSTS_FILE);
if (is_lsi && (flags & AI_HIP))
- continue;
- else
- found_hits = 1;
-
- /* "add every HIT to linked list"
- What do you mean by "every"? We only have one HIT per
- line, don't we? Also, why do we loop through the list
- again when we already have the hit stored from the
- previous loop?
- 18.01.2008 16:49 -Lauri. */
- for(i = 0; i <length(&list); i++) {
- struct gaih_addrtuple *last_pat = NULL;
-
- aux = (struct gaih_addrtuple *)
- malloc(sizeof(struct gaih_addrtuple));
- if (aux == NULL){
- HIP_ERROR("Memory allocation error\n");
- return -EAI_MEMORY;
- }
- memset(aux, 0, sizeof(struct gaih_addrtuple));
-
- /* Get the last element in the list */
- if (**pat) {
- for (last_pat = **pat; last_pat->next
!= NULL;
- last_pat =
last_pat->next)
- ;
- }
-
- /* Place the HIT/LSI to the end of the list.*/
-
- if (inet_pton(AF_INET6, getitem(&list,i),
&hit)) {
- /* It's a HIT */
- aux->scopeid = 0;
- aux->family = AF_INET6;
- memcpy(aux->addr, &hit, sizeof(struct
in6_addr));
- if (**pat)
- last_pat->next = aux;
- else
- **pat = aux;
- }
- else if (inet_pton(AF_INET, getitem(&list,i),
&lsi)){
- /* IPv4 to IPV6 in order to be
supported by the daemon */
- aux->scopeid = 0;
- aux->family = AF_INET;
- HIP_DEBUG_LSI(" lsi to add", &lsi);
- memcpy(aux->addr, &lsi, sizeof(lsi));
- if (**pat)
- last_pat->next = aux;
- else
- **pat = aux;
- } else {
- free(aux);
- }
-
- }
- } // end of if
+ continue;
+
+ found_hits = 1;
+
+ /* At this point we have, as said above, the HIT in 'hit', the LSI in
'lsi'
+ * and the name in 'name'. These are now added to the gaih_addrtuple
list. */
+ aux = (struct gaih_addrtuple*)malloc(sizeof(struct gaih_addrtuple));
+ if(aux) {
+ struct gaih_addrtuple *last_pat = **pat;
+
+ /* last_pat needs to be the last element in the list. If **pat
+ * was null instead, we do nothing. */
+ if(last_pat) {
+ while(last_pat->next) {
+ last_pat = last_pat->next;
+ }
+ }
+
+ /* If is_lsi has been set, we had an LSI on the line. It should not
be
+ * possible to have both an LSI and a HIT on the same line. */
+ memset(aux, 0, sizeof(struct gaih_addrtuple));
+ if(is_lsi) {
+ aux->scopeid = 0;
+ aux->family = AF_INET;
+ HIP_DEBUG_LSI(" lsi to add", &lsi);
+ memcpy(aux->addr, &lsi, sizeof(lsi));
+ }
+ /* was HIT instead */
+ else
+ {
+ aux->scopeid = 0;
+ aux->family = AF_INET6;
+ memcpy(aux->addr, &hit, sizeof(hit));
+ }
+
+ /* Append the latest address tuple to the end of the list. */
+ if(last_pat) {
+ last_pat->next = aux;
+ }
+ else {
+ **pat = aux;
+ }
+ }
+ else {
+ HIP_ERROR("Memory allocation error\n");
+ return -EAI_MEMORY;
+ }
+ } /* end of matching fqdn */
destroy(&list);
} // end of while
@@ -732,6 +723,7 @@
}
for(at_ip = orig_at; at_ip != NULL; at_ip = at_ip->next) {
+ memset(msg, 0, HIP_MAX_PACKET);
hip_build_user_hdr(msg, SO_HIP_ADD_PEER_MAP_HIT_IP, 0);
if (at_ip->family == AF_INET6){
Other related posts:
