[Libosinfo] [PATCH 8/8] db: Fix valgrind issues with language detection

Christophe Fergeau cfergeau at redhat.com
Thu Nov 5 16:20:46 UTC 2015


match_languages() returns a list containing a single string, but given
the way language_code_from_raw() works, this string may actually point
to already freed memory by the time match_languages() ends.
Subsequent uses of this list will cause accesses to free'd memory.

This commit changes match_languages() so that it uses the string itself
(before it's freed) rather than letting the caller do this.

This fixes:
==28623== Invalid read of size 1
==28623==    at 0x4C2BC22: strlen (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==28623==    by 0x6A6B1F2: g_strdup (gstrfuncs.c:362)
==28623==    by 0x553FCA2: osinfo_entity_add_param (osinfo_entity.c:253)
==28623==    by 0x5554BD8: osinfo_media_set_languages (osinfo_media.c:1256)
==28623==    by 0x555A78A: fill_media (osinfo_db.c:641)
==28623==    by 0x555ABED: osinfo_db_identify_media (osinfo_db.c:709)
==28623==    by 0x4030AA: test_one (test-isodetect.c:346)
==28623==    by 0x40339D: test_windows (test-isodetect.c:395)
==28623==    by 0x532B78A: tcase_run_tfun_nofork.isra.9 (check_run.c:390)
==28623==    by 0x532BB7C: srunner_iterate_tcase_tfuns (check_run.c:231)
==28623==    by 0x532BB7C: srunner_run_tcase (check_run.c:373)
==28623==    by 0x532BB7C: srunner_iterate_suites (check_run.c:195)
==28623==    by 0x532BB7C: srunner_run (check_run.c:782)
==28623==    by 0x403A1B: main (test-isodetect.c:500)
==28623==  Address 0x89914b0 is 0 bytes inside a block of size 4 free'd
==28623==    at 0x4C29D6A: free (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==28623==    by 0x6A525ED: g_free (gmem.c:189)
==28623==    by 0x55586A6: match_languages (osinfo_db.c:117)
==28623==    by 0x555A76C: fill_media (osinfo_db.c:639)
==28623==    by 0x555ABED: osinfo_db_identify_media (osinfo_db.c:709)
==28623==    by 0x4030AA: test_one (test-isodetect.c:346)
==28623==    by 0x40339D: test_windows (test-isodetect.c:395)
==28623==    by 0x532B78A: tcase_run_tfun_nofork.isra.9 (check_run.c:390)
==28623==    by 0x532BB7C: srunner_iterate_tcase_tfuns (check_run.c:231)
==28623==    by 0x532BB7C: srunner_run_tcase (check_run.c:373)
==28623==    by 0x532BB7C: srunner_iterate_suites (check_run.c:195)
==28623==    by 0x532BB7C: srunner_run (check_run.c:782)
==28623==    by 0x403A1B: main (test-isodetect.c:500)
==28623==  Block was alloc'd at
==28623==    at 0x4C28C50: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==28623==    by 0x6A524D8: g_malloc (gmem.c:94)
==28623==    by 0x6A6B277: g_strndup (gstrfuncs.c:425)
==28623==    by 0x6A5EC90: g_match_info_fetch (gregex.c:1005)
==28623==    by 0x55583DF: get_raw_lang (osinfo_db.c:57)
==28623==    by 0x5558672: match_languages (osinfo_db.c:113)
==28623==    by 0x555A76C: fill_media (osinfo_db.c:639)
==28623==    by 0x555ABED: osinfo_db_identify_media (osinfo_db.c:709)
==28623==    by 0x4030AA: test_one (test-isodetect.c:346)
==28623==    by 0x40339D: test_windows (test-isodetect.c:395)
==28623==    by 0x532B78A: tcase_run_tfun_nofork.isra.9 (check_run.c:390)
==28623==    by 0x532BB7C: srunner_iterate_tcase_tfuns (check_run.c:231)
==28623==    by 0x532BB7C: srunner_run_tcase (check_run.c:373)
==28623==    by 0x532BB7C: srunner_iterate_suites (check_run.c:195)
==28623==    by 0x532BB7C: srunner_run (check_run.c:782)
==28623==    by 0x403A1B: main (test-isodetect.c:500)
---
 osinfo/osinfo_db.c | 29 +++++++++++++----------------
 1 file changed, 13 insertions(+), 16 deletions(-)

diff --git a/osinfo/osinfo_db.c b/osinfo/osinfo_db.c
index 4eeb440..82751a6 100644
--- a/osinfo/osinfo_db.c
+++ b/osinfo/osinfo_db.c
@@ -80,8 +80,8 @@ static const char *language_code_from_raw(OsinfoDatamap *lang_map,
     return lang;
 }
 
-static GList *match_languages(OsinfoDb *db, OsinfoMedia *media,
-                              OsinfoMedia *db_media)
+static void set_languages_for_media(OsinfoDb *db, OsinfoMedia *media,
+                                    OsinfoMedia *db_media)
 {
     const gchar *volume_id;
     const gchar *regex;
@@ -89,18 +89,19 @@ static GList *match_languages(OsinfoDb *db, OsinfoMedia *media,
     OsinfoDatamap *lang_map;
     gchar *raw_lang;
     GList *languages;
+    const char *lang;
 
-    g_return_val_if_fail(OSINFO_IS_MEDIA(media), NULL);
-    g_return_val_if_fail(OSINFO_IS_MEDIA(db_media), NULL);
+    g_return_if_fail(OSINFO_IS_MEDIA(media));
+    g_return_if_fail(OSINFO_IS_MEDIA(db_media));
 
     regex = osinfo_entity_get_param_value(OSINFO_ENTITY(db_media),
                                           OSINFO_MEDIA_PROP_LANG_REGEX);
     if (regex == NULL)
-        return NULL;
+        return;
 
     volume_id = osinfo_media_get_volume_id(media);
     if (volume_id == NULL)
-        return NULL;
+        return;
 
     lang_map_id = osinfo_entity_get_param_value(OSINFO_ENTITY(db_media),
                                                 OSINFO_MEDIA_PROP_LANG_MAP);
@@ -111,14 +112,14 @@ static GList *match_languages(OsinfoDb *db, OsinfoMedia *media,
     }
 
     raw_lang = get_raw_lang(volume_id, regex);
-
-    languages = g_list_append(NULL,
-                              (gpointer)language_code_from_raw(lang_map, raw_lang));
+    lang = language_code_from_raw(lang_map, raw_lang);
+    languages = g_list_append(NULL, (gpointer)lang);
+    osinfo_media_set_languages(media, languages);
+    g_list_free(languages);
     g_free(raw_lang);
-
-    return languages;
 }
 
+
 /**
  * SECTION:osinfo_db
  * @short_description: Database of all entities
@@ -625,7 +626,6 @@ static void fill_media(OsinfoDb *db, OsinfoMedia *media,
                         OsinfoMedia *matched_media,
                         OsinfoOs *os)
 {
-    GList *languages;
     gboolean is_installer;
     gboolean is_live;
     gint reboots;
@@ -636,10 +636,7 @@ static void fill_media(OsinfoDb *db, OsinfoMedia *media,
     const gchar *url;
     GList *variants, *node;
 
-    languages = match_languages(db, media, matched_media);
-    if (languages != NULL)
-        osinfo_media_set_languages(media, languages);
-    g_list_free(languages);
+    set_languages_for_media(db, media, matched_media);
 
     id = osinfo_entity_get_id(OSINFO_ENTITY(matched_media));
     g_object_set(G_OBJECT(media), "id", id, NULL);
-- 
2.5.0




More information about the Libosinfo mailing list