[Libosinfo] [PATCH 8/8] db: Fix valgrind issues with language detection
Daniel P. Berrange
berrange at redhat.com
Thu Nov 5 17:00:19 UTC 2015
On Thu, Nov 05, 2015 at 05:20:46PM +0100, Christophe Fergeau wrote:
> 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);
ACK
Regards,
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
More information about the Libosinfo
mailing list