| Summary: | h->cm_map resource getting leak in read_xcf_header function | ||
|---|---|---|---|
| Product: | SDL_image | Reporter: | Nitz <nitin.j4> |
| Component: | misc | Assignee: | Sam Lantinga <slouken> |
| Status: | RESOLVED FIXED | QA Contact: | Sam Lantinga <slouken> |
| Severity: | critical | ||
| Priority: | P2 | CC: | sylvain.becker |
| Version: | unspecified | ||
| Hardware: | x86 | ||
| OS: | Linux | ||
| Attachments: | xcf image | ||
Created attachment 2927 [details]
xcf image
Adding an xcf image ...
valgrind reports this memory leak:
==22032== by 0x4EF0134: SDL_malloc_REAL (SDL_malloc.c:36)
==22032== by 0x4E7E413: SDL_malloc (SDL_dynapi_procs.h:406)
==22032== by 0x51EF463: load_xcf_tile_rle (IMG_xcf.c:474)
==22032== by 0x51EF99A: do_layer_surface (IMG_xcf.c:580)
==22032== by 0x51EEEED: IMG_LoadXCF_RW (IMG_xcf.c:761)
==22032== by 0x51DDA66: IMG_LoadTyped_RW (IMG.c:198)
==22032== by 0x51DD8F2: IMG_Load (IMG.c:139)
That seem to be fixed by doing:
--- a/IMG_xcf.c Sat Sep 09 18:57:05 2017 -0700
+++ b/IMG_xcf.c Mon Sep 11 21:43:09 2017 +0200
@@ -676,6 +676,7 @@
ty += 64;
}
if (ty >= level->height) {
+ free_xcf_tile (tile);
break;
}
That xcf loader is a mess, but I fixed the bugs you guys reported here, thanks! https://hg.libsdl.org/SDL_image/rev/d3e819a08733 |
h->cm_map resource getting leak in static xcf_header * read_xcf_header (SDL_RWops * src) function at the following location: do { xcf_read_property (src, &prop); if (prop.id == PROP_COMPRESSION) h->compr = (xcf_compr_type)prop.data.compression; else if (prop.id == PROP_COLORMAP) { // unused var: int i; h->cm_num = prop.data.colormap.num; h->cm_map = (unsigned char *) SDL_malloc (sizeof (unsigned char) * 3 * h->cm_num); SDL_memcpy (h->cm_map, prop.data.colormap.cmap, 3*sizeof (char)*h->cm_num); SDL_free (prop.data.colormap.cmap); } } while (prop.id != PROP_END); Here Overwriting "h->cm_map" in "h->cm_map = (unsigned char *)malloc(3U * h->cm_num)" leaks the storage that "h->cm_map" points to. Resource "h->cm_map" is not freed or pointed-to in function "memcpy(void * restrict, void const * restrict, size_t)". So there is need to check the logic or we can add a patch to avoid this kind of issue. Patch: if(!h->cm_map) h->cm_map = (unsigned char *) SDL_malloc (sizeof (unsigned char) * 3 * h- >cm_num); SDL_memcpy (h->cm_map, prop.data.colormap.cmap, 3*sizeof (char)*h->cm_num); (prop.data.colormap.cmap != NULL) { SDL_free(prop.data.colormap.cmap); prop.data.colormap.cmap = NULL ; } Thanks...