Hi, I'm *sorry* that I am not good at English because I'm Japanese. I found memory leak(?) of 1 byte when PEAP authentication, by valgrind. I tried fllowing patch for rlm_eap. it look like work well. is it corret way? -------------------------------- diff -urN ../freeradius-1.1.7/src/modules/rlm_eap/eap.c ./src/modules/rlm_eap/eap.c --- ../freeradius-1.1.7/src/modules/rlm_eap/eap.c 2007-04-08 07:18:34.000000000 +0900 +++ ./src/modules/rlm_eap/eap.c 2007-10-10 02:44:20.000000000 +0900 @@ -421,6 +421,11 @@ */ if (reply->type.data && reply->type.length > 0) { memcpy(&hdr->data[1], reply->type.data, reply->type.length); + + // 2007/10/09 workaround for memory leak. + DEBUG2(" rlm_eap: in %s() in %s Freeing reply->type.data 0x%x", __func__, __FILE__, (unsigned int)reply->type.data); + reply->type.bNeedFreeData = 0; + free(reply->type.data); reply->type.data = reply->packet + EAP_HEADER_LEN + 1/*EAPtype*/; } diff -urN ../freeradius-1.1.7/src/modules/rlm_eap/libeap/eap_tls.c ./src/modules/rlm_eap/libeap/eap_tls.c --- ../freeradius-1.1.7/src/modules/rlm_eap/libeap/eap_tls.c 2007-04-08 06:27:19.000000000 +0900 +++ ./src/modules/rlm_eap/libeap/eap_tls.c 2007-10-09 22:39:14.000000000 +0900 @@ -808,6 +808,10 @@ return 0; } + // 2007/10/09 workaround for memory leak. + DEBUG2(" rlm_eap: in %s() Malloc(%d) for eap_ds->request->type.data 0x%x", __func__, (reply->length - TLS_HEADER_LEN + 1), (unsigned int)eap_ds->request->type.data); + eap_ds->request->type.bNeedFreeData = 1; + /* EAPTLS Header length is excluded while computing EAP typelen */ eap_ds->request->type.length = reply->length - TLS_HEADER_LEN; diff -urN ../freeradius-1.1.7/src/modules/rlm_eap/libeap/eap_types.h ./src/modules/rlm_eap/libeap/eap_types.h --- ../freeradius-1.1.7/src/modules/rlm_eap/libeap/eap_types.h 2006-05-19 23:19:15.000000000 +0900 +++ ./src/modules/rlm_eap/libeap/eap_types.h 2007-10-09 20:51:14.000000000 +0900 @@ -88,6 +88,9 @@ unsigned char type; unsigned int length; unsigned char *data; + + // 2007/10/09 workaround for memory leak. + unsigned int bNeedFreeData; } eaptype_t; /* diff -urN ../freeradius-1.1.7/src/modules/rlm_eap/libeap/eapcommon.c ./src/modules/rlm_eap/libeap/eapcommon.c --- ../freeradius-1.1.7/src/modules/rlm_eap/libeap/eapcommon.c 2007-03-05 23:34:55.000000000 +0900 +++ ./src/modules/rlm_eap/libeap/eapcommon.c 2007-10-10 02:43:30.000000000 +0900 @@ -216,6 +216,11 @@ */ if (reply->type.data && reply->type.length > 0) { memcpy(&hdr->data[1], reply->type.data, reply->type.length); + + // 2007/10/09 workaround for memory leak. + DEBUG2(" rlm_eap: in %s() in %s Freeing reply->type.data 0x%x", __func__, __FILE__, (unsigned int)reply->type.data); + reply->type.bNeedFreeData = 0; + free(reply->type.data); reply->type.data = reply->packet + EAP_HEADER_LEN + 1/*EAPtype*/; } @@ -386,6 +391,11 @@ ep.type.type = eap_type; ep.type.length = vp->length; ep.type.data = malloc(vp->length); + + // 2007/10/09 workaround for memory leak. + DEBUG2(" rlm_eap: in %s() Malloc(%d) for ep.type.data 0x%x", __func__, vp->length, (unsigned int)ep.type.data); + ep.type.bNeedFreeData = 1; + memcpy(ep.type.data,vp->strvalue, vp->length); eap_basic_compose(req, &ep); } diff -urN ../freeradius-1.1.7/src/modules/rlm_eap/mem.c ./src/modules/rlm_eap/mem.c --- ../freeradius-1.1.7/src/modules/rlm_eap/mem.c 2007-02-10 00:42:03.000000000 +0900 +++ ./src/modules/rlm_eap/mem.c 2007-10-09 22:40:28.000000000 +0900 @@ -54,6 +54,14 @@ * so we do not free it but we NULL it free(eap_packet->type.data); */ + + // 2007/10/09 workaround for memory leak. + if(eap_packet->type.bNeedFreeData) { + DEBUG2(" rlm_eap: in %s() Freeing eap-packet->type.data 0x%x", __func__, (unsigned int)eap_packet->type.data); + eap_packet->type.bNeedFreeData = 0; + free(eap_packet->type.data); + } + eap_packet->type.data = NULL; } -------------------------------- -------------------------------------- New Design Yahoo! JAPAN 2008/01/01 http://pr.mail.yahoo.co.jp/newdesign/
blue_11j@yahoo.co.jp wrote:
I found memory leak(?) of 1 byte when PEAP authentication, by valgrind.
Can you provide a test case, hopefully with wpa_supplicant as the client?
I tried fllowing patch for rlm_eap. it look like work well. is it corret way?
I'm not sure. The "type.data" field *is* malloc'd. It is NOT free'd in eap_packet_free(). However, it *is* free'd in eap_wireformat(). The only time I can see there being a memory leak is when the EAP packet is allocated, *but* it is never encoded into the wire-format. If you go to the rlm_eap directory, you can do: $ grep -r alloc . | grep type.data | grep c: That will give you around 10 locations where type.data = malloc(...). If this is really a memory leak, then *all* of those locations will likely have to be fixed. The following change inside of eap_packet_free() *should* catch all corner cases. Alan DeKok. if (eap_packet->type.data) { /* * There IS a packet, AND the data portion points * to the EAP data: do nothing. Otherwise, free it. */ if (!(eap_packet->packet && (eap_packet->type.data == (eap_packet->packet + 5)))) { free(eap_packet->type.data); } eap_packet->type.data = NULL; }
participants (2)
-
Alan DeKok -
blue_11j@yahoo.co.jp