openhpi2 / Open-HPI

Open HPI is an open source implementation of the SA Forum's Hardware Platform Interface (HPI). HPI provides an abstracted interface to managing computer hardware, typically for chassis and rack based servers
Other
3 stars 1 forks source link

Create wrap_g_free to set the freed memory to NULL #2563

Closed openhpi2 closed 8 years ago

openhpi2 commented 9 years ago

g_free call does not set the pointer to NULL after freeing the memory. This is troublesome as some other thread could potentially try to free the pointer again leading to core dumps. Create a wrapper funtion to g_free which will free the memory and set the pointer to NULL.

Also g_malloc0 initializes the contents to 0 ater allocating it. There are places where g_malloc is used instead of g_malloc0. Modify those places also.

Reported by: dr_mohan

openhpi2 commented 9 years ago

I don't understand why we are permitting code to call g_free twice on the same piece of memory. It seems to me that this is a coding bug and should be addressed in the calling code.

The core dump is merely a symptom of a coding problem. If you mask the core dumps, you will no longer get notified of the root problem.

Original comment by: sutula

openhpi2 commented 9 years ago

To my knowledge we are not calling the g_free twice on the same place.

In some places we do set the pointer to NULL after freeing it.  But not in all the places.

In most of the places we have the following code if (a_ptr != NULL)    g_free(a_ptr); Since the g_free does not set a_ptr to NULL the first check itself becomes invalid. If the same pointer is allocated using g_malloc (instead of g_malloc0) we get the same contents that was there before g_free!. The following example demonstrates that. So it is safer to set the pointer to NULL once the memory is free'd.

Creating a wrapper takes care of it all in one place. Check for NULL, free it, set it to NULL after freeing it.

/* Use cc -o g_free_null g_free_null.c -lglib-2.0 */

include

include

main() {     char mem_ptrs = 0;     int len = 124;     printf("mem_ptrs pointer value at start is %p\n",mem_ptrs);     printf("The g_malloc is NOT supposed to initialize the allocated memory\n");     mem_ptrs = (char )g_malloc(sizeof(char) * len);     printf("The allocated string with g_malloc is %s and ptr is %p\n",mem_ptrs, mem_ptrs);     strcpy(mem_ptrs,"Mohan");     g_free(mem_ptrs);     printf("mem_ptrs pointer value after g_free is %p\n",mem_ptrs);     mem_ptrs = 0;     printf("mem_ptrs pointer value after mem_ptrs = 0 is %p\n",mem_ptrs);

    printf("The g_malloc is NOT supposed to initialize the allocated memory\n");     mem_ptrs = (char )g_malloc(sizeof(char) \ len);     printf("The allocated string with g_malloc is %s and ptr is %p\n",mem_ptrs, mem_ptrs);     g_free(mem_ptrs);     printf("mem_ptrs pointer value after g_free is %p\n",mem_ptrs);     mem_ptrs = 0;     printf("mem_ptrs pointer value after mem_ptrs = 0 is %p\n",mem_ptrs);

    printf("The g_malloc0 is supposed to initialize the allocated memory\n");     mem_ptrs = (char )g_malloc0(sizeof(char) \ len);     printf("The allocated string with g_malloc0 is %s and ptr is %p\n",mem_ptrs, mem_ptrs);     g_free(mem_ptrs);     printf("mem_ptrs pointer value after g_free is %p\n",mem_ptrs);     mem_ptrs = 0;     printf("mem_ptrs pointer value after mem_ptrs = 0 is %p\n",mem_ptrs);

}

Output is[root@dl380g7-1 bin]# ./g_free_null mem_ptrs pointer value at start is (nil) The g_malloc is NOT supposed to initialize the allocated memory The allocated string with g_malloc is  and ptr is 0xc5b010 mem_ptrs pointer value after g_free is 0xc5b010 mem_ptrs pointer value after mem_ptrs = 0 is (nil) The g_malloc is NOT supposed to initialize the allocated memory The allocated string with g_malloc is Mohan and ptr is 0xc5b010 mem_ptrs pointer value after g_free is 0xc5b010 mem_ptrs pointer value after mem_ptrs = 0 is (nil) The g_malloc0 is supposed to initialize the allocated memory The allocated string with g_malloc0 is  and ptr is 0xc5b010 mem_ptrs pointer value after g_free is 0xc5b010 mem_ptrs pointer value after mem_ptrs = 0 is (nil)

 On Tuesday, November 11, 2014 1:13 PM, Bryan Sutula <sutula@users.sf.net> wrote:

I don't understand why we are permitting code to call g_free twice on the same piece of memory.  It seems to me that this is a coding bug and should be addressed in the calling code.

The core dump is merely a symptom of a coding problem.  If you mask the core dumps, you will no longer get notified of the root problem.


\ [bugs:#1862] Create wrap_g_free to set the freed memory to NULL**

Status: open 3.6.0: 3.6.0 Labels: HP c-Class Plugin Created: Tue Nov 11, 2014 06:24 PM UTC by dr_mohan Last Updated: Tue Nov 11, 2014 06:24 PM UTC Owner: Hemantha Beecherla

g_free call does not set the pointer to NULL after freeing the memory. This is troublesome as some other thread could potentially try to free the pointer again  leading to core dumps. Create a wrapper funtion to g_free which will free the memory and set the pointer to NULL.

Also g_malloc0 initializes the contents to 0 ater allocating it. There are places where g_malloc is used instead of g_malloc0. Modify those places also.


Sent from sourceforge.net because you indicated interest in https://sourceforge.net/p/openhpi/bugs/1862/

To unsubscribe from further messages, please visit https://sourceforge.net/auth/subscriptions/

Alternate.txt.txt

Original comment by: dr_mohan

openhpi2 commented 9 years ago

Original comment by: dr_mohan

openhpi2 commented 9 years ago

Fixed with checkin #7613

Original comment by: dr_mohan