Skip to content

Commit 74fe0a0

Browse files
committed
qdl: fix resource leaks in programmer image decoding
decode_programmer_archive() allocates images[id].ptr (via malloc) and images[id].name (via strdup) while iterating CPIO entries. If a later entry fails validation, all previously allocated entries leak because the function returns -1 immediately. The same leak exists in decode_sahara_config() and its callers: decode_programmer() doesn't clean up partially-populated images on error, and qdl_flash() never frees the sahara_images array even on the normal exit path. Rather than restructuring the parser loop with break/cleanup logic, fix this at the ownership level: - Introduce sahara_images_free() as a reusable cleanup helper. free(NULL) is well-defined per C standard, so no NULL guards are needed before calling free(). - Drop 'const' from sahara_image.name: the struct always owns the allocation (via strdup), so const is misleading and forces casts when freeing. This follows the principle that the owner of a heap allocation should hold a non-const pointer to it. - Call sahara_images_free() in decode_programmer() error paths and in qdl_flash() cleanup, keeping decode_programmer_archive() itself simple with its existing early returns. Signed-off-by: Igor Opaniuk <igor.opaniuk@oss.qualcomm.com>
1 parent 38ff756 commit 74fe0a0

File tree

3 files changed

+19
-2
lines changed

3 files changed

+19
-2
lines changed

qdl.c

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -395,7 +395,11 @@ static int decode_programmer(char *s, struct sahara_image *images)
395395
return -1;
396396

397397
ret = decode_programmer_archive(&archive, images);
398-
if (ret < 0 || ret == 1)
398+
if (ret < 0) {
399+
sahara_images_free(images, MAPPING_SZ);
400+
return ret;
401+
}
402+
if (ret == 1)
399403
return ret;
400404

401405
ret = decode_sahara_config(&archive, images);
@@ -747,6 +751,9 @@ static int qdl_flash(int argc, char **argv)
747751
vip_gen_finalize(qdl);
748752

749753
qdl_close(qdl);
754+
755+
sahara_images_free(sahara_images, MAPPING_SZ);
756+
750757
free_programs();
751758
free_patches();
752759

qdl.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,7 @@ struct qdl_device {
7979
};
8080

8181
struct sahara_image {
82-
const char *name;
82+
char *name;
8383
void *ptr;
8484
size_t len;
8585
};
@@ -113,6 +113,7 @@ int sahara_run(struct qdl_device *qdl, const struct sahara_image *images,
113113
const char *ramdump_path,
114114
const char *ramdump_filter);
115115
int load_sahara_image(const char *filename, struct sahara_image *image);
116+
void sahara_images_free(struct sahara_image *images, size_t count);
116117
void print_hex_dump(const char *prefix, const void *buf, size_t len);
117118
unsigned int attr_as_unsigned(xmlNode *node, const char *attr, int *errors);
118119
const char *attr_as_string(xmlNode *node, const char *attr, int *errors);

util.c

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -266,3 +266,12 @@ int load_sahara_image(const char *filename, struct sahara_image *image)
266266
close(fd);
267267
return -1;
268268
}
269+
270+
void sahara_images_free(struct sahara_image *images, size_t count)
271+
{
272+
for (size_t i = 0; i < count; i++) {
273+
free(images[i].name);
274+
free(images[i].ptr);
275+
images[i] = (struct sahara_image){};
276+
}
277+
}

0 commit comments

Comments
 (0)