al_shutdown_video_addon invalid read & free

al_shutdown_video_addon invalid read & free

huguesdpdn

Member #16,616

January 2017

Hello,

I am really enjoying this new graphic library, but still detect a problem with al_shutdown_video_addon().

This is what I get when I use or not functions relatives to al_shutdown_video_addon .
This is valgrind with my program (an empty main calling all init addons and calling all shutdown addons).
Program compiled under gcc with flags -g3, also for the lib allegro library (mode debug).

==6982== Invalid read of size 8
==6982== at 0x433B93: al_shutdown_video_addon (video.c:282)
==6982== by 0x45282C: _al_run_exit_funcs (exitfunc.c:92)
==6982== by 0x414264: al_uninstall_system (system.c:312)
==6982== by 0xB413138: __run_exit_handlers (in /lib64/libc-2.22.so)
==6982== by 0xB413184: exit (in /lib64/libc-2.22.so)
==6982== by 0xB3FC6EB: (below main) (in /lib64/libc-2.22.so)
==6982== Address 0x12d9cd70 is 0 bytes inside a block of size 24 free'd
==6982== at 0x4C2A3CC: free (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==6982== by 0x433BA9: al_shutdown_video_addon (video.c:283)
==6982== by 0x40CDF5: HUD_graphics_shutdown (HUD_graphics_init_shutdown.c:36)
==6982== by 0x40CD1C: HUD_global_destroy (HUD_global.c:37)
==6982== by 0x40CC91: HUD_global_create (HUD_global.c:22)
==6982== by 0x4105E2: main (main.c:11)
==6982== Block was alloc'd at
==6982== at 0x4C2B250: calloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==6982== by 0x433F29: add_handler (video.c:77)
==6982== by 0x433F29: al_init_video_addon (video.c:262)
==6982== by 0x40CDBE: HUD_graphics_init (HUD_graphics_init_shutdown.c:18)
==6982== by 0x40CBBD: HUD_global_create (HUD_global.c:7)
==6982== by 0x4105E2: main (main.c:11)
==6982==
==6982== Invalid free() / delete / delete[] / realloc()
==6982== at 0x4C2A3CC: free (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==6982== by 0x433BA9: al_shutdown_video_addon (video.c:283)
==6982== by 0x45282C: _al_run_exit_funcs (exitfunc.c:92)
==6982== by 0x414264: al_uninstall_system (system.c:312)
==6982== by 0xB413138: __run_exit_handlers (in /lib64/libc-2.22.so)
==6982== by 0xB413184: exit (in /lib64/libc-2.22.so)
==6982== by 0xB3FC6EB: (below main) (in /lib64/libc-2.22.so)
==6982== Address 0x12d9cd70 is 0 bytes inside a block of size 24 free'd
==6982== at 0x4C2A3CC: free (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==6982== by 0x433BA9: al_shutdown_video_addon (video.c:283)
==6982== by 0x40CDF5: HUD_graphics_shutdown (HUD_graphics_init_shutdown.c:36)
==6982== by 0x40CD1C: HUD_global_destroy (HUD_global.c:37)
==6982== by 0x40CC91: HUD_global_create (HUD_global.c:22)
==6982== by 0x4105E2: main (main.c:11)
==6982== Block was alloc'd at
==6982== at 0x4C2B250: calloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==6982== by 0x433F29: add_handler (video.c:77)
==6982== by 0x433F29: al_init_video_addon (video.c:262)
==6982== by 0x40CDBE: HUD_graphics_init (HUD_graphics_init_shutdown.c:18)
==6982== by 0x40CBBD: HUD_global_create (HUD_global.c:7)
==6982== by 0x4105E2: main (main.c:11)

So I check al_init_video_addon function source code to try to understand why this is happening, and maybe this can help :

In add_handler (video.c, line 73) called by al_init_video_addon,
v->next is never set to NULL or initialised, normal ?
I think this cause the invalid read and free, but not sure, could be a memset behind or something like this.

Thanks to Allegro Team

SiegeLord

Member #7,827

October 2006

Nah, that struct is allocated via al_calloc which zeroes the memory out. The issue is that al_shutdown_video_addon is called twice, once by you explicitly (which works fine) but then a second time by Allegro's destructor system which gets invoked when the main function exits (see your backtrace). In principle you could remove your explicit call to al_shutdown_video_addon and things will work fine. Either way, this is now fixed on the master branch: https://github.com/liballeg/allegro5/commit/6471441. Thanks for reporting this!

"For in much wisdom is much grief: and he that increases knowledge increases sorrow."-Ecclesiastes 1:18
[SiegeLord's Abode][Codes]:[DAllegro5]:[RustAllegro]

Neil Roy

Member #2,229

April 2002

Not sure why you would call al_shutdown_video_addon() manually when the documentation clearly states it is shut down automatically.

---
“I love you too.” - last words of Wanda Roy

huguesdpdn

Member #16,616

January 2017

@SiegeLord : I was thought there was something like this with a bzero or memset behind, but I didn't check the al_calloc function.

Thank you for the new release! Hope coming soon 8-)

@Neil Roy:
You are right, but I prefer call it myself to be sure this is done. It's psychological
Note: Sometimes, it could be usefull if you destroy everything and relaunch everything without exiting your program, especially in embedded (and in fact I am working on an embedded system ).

Thanks both of you.