{"id":807089,"url":"http://patchwork.ozlabs.org/api/1.2/patches/807089/?format=json","web_url":"http://patchwork.ozlabs.org/project/glibc/patch/50ea6b93-b58a-fdad-c178-a188ff6b6728@redhat.com/","project":{"id":41,"url":"http://patchwork.ozlabs.org/api/1.2/projects/41/?format=json","name":"GNU C Library","link_name":"glibc","list_id":"libc-alpha.sourceware.org","list_email":"libc-alpha@sourceware.org","web_url":"","scm_url":"","webscm_url":"","list_archive_url":"","list_archive_url_format":"","commit_url_format":""},"msgid":"<50ea6b93-b58a-fdad-c178-a188ff6b6728@redhat.com>","list_archive_url":null,"date":"2017-08-29T13:28:43","name":"Mangle NULL pointers in iconv/gconv [BZ #22025]","commit_ref":null,"pull_url":null,"state":"new","archived":false,"hash":"088c6f4e2875400cc50f9fd716c095071b4013fa","submitter":{"id":14312,"url":"http://patchwork.ozlabs.org/api/1.2/people/14312/?format=json","name":"Florian Weimer","email":"fweimer@redhat.com"},"delegate":null,"mbox":"http://patchwork.ozlabs.org/project/glibc/patch/50ea6b93-b58a-fdad-c178-a188ff6b6728@redhat.com/mbox/","series":[{"id":367,"url":"http://patchwork.ozlabs.org/api/1.2/series/367/?format=json","web_url":"http://patchwork.ozlabs.org/project/glibc/list/?series=367","date":"2017-08-29T13:28:43","name":"Mangle NULL pointers in iconv/gconv [BZ #22025]","version":1,"mbox":"http://patchwork.ozlabs.org/series/367/mbox/"}],"comments":"http://patchwork.ozlabs.org/api/patches/807089/comments/","check":"pending","checks":"http://patchwork.ozlabs.org/api/patches/807089/checks/","tags":{},"related":[],"headers":{"Return-Path":"","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":["patchwork-incoming@bilbo.ozlabs.org","mailing list libc-alpha@sourceware.org"],"Authentication-Results":["ozlabs.org;\n\tspf=pass (mailfrom) smtp.mailfrom=sourceware.org\n\t(client-ip=209.132.180.131; helo=sourceware.org;\n\tenvelope-from=libc-alpha-return-83813-incoming=patchwork.ozlabs.org@sourceware.org;\n\treceiver=)","ozlabs.org; dkim=pass (1024-bit key;\n\tsecure) header.d=sourceware.org header.i=@sourceware.org\n\theader.b=\"j7peV1DJ\"; dkim-atps=neutral","sourceware.org; auth=none","ext-mx01.extmail.prod.ext.phx2.redhat.com;\n\tdmarc=none (p=none dis=none) header.from=redhat.com","ext-mx01.extmail.prod.ext.phx2.redhat.com;\n\tspf=fail smtp.mailfrom=fweimer@redhat.com"],"Received":["from sourceware.org (server1.sourceware.org [209.132.180.131])\n\t(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256\n\tbits)) (No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 3xhTxZ0vDqz9t2v\n\tfor ;\n\tTue, 29 Aug 2017 23:29:05 +1000 (AEST)","(qmail 112119 invoked by alias); 29 Aug 2017 13:28:59 -0000","(qmail 112099 invoked by uid 89); 29 Aug 2017 13:28:57 -0000"],"DomainKey-Signature":"a=rsa-sha1; c=nofws; d=sourceware.org; h=list-id\n\t:list-unsubscribe:list-subscribe:list-archive:list-post\n\t:list-help:sender:to:from:subject:message-id:date:mime-version\n\t:content-type; q=dns; s=default; b=bmANsC9JpK/VDmfO7PohgOiNOVmFU\n\txPbVJuIjY8wRTj5i9TgEiVwY1c4v+QcSxgd2JIkt5vljvTxm8BWYns67u7yne9rc\n\tiaBUZcQ8mKlZKkqsoxS0YwaGEF8dEr3xCQmR0u+d9jf9Gg38mnZbFL3Mb1T66WXn\n\trRQvlG7WScKBq0=","DKIM-Signature":"v=1; a=rsa-sha1; c=relaxed; d=sourceware.org; h=list-id\n\t:list-unsubscribe:list-subscribe:list-archive:list-post\n\t:list-help:sender:to:from:subject:message-id:date:mime-version\n\t:content-type; s=default; bh=/U6w3yKfkVdFezx/8irEa3x9vPY=; b=j7p\n\teV1DJtJH4HW9L87caUS4+vZhGWooF8BU0u3lqpXMWvjhuGjjatsmu5LKPLs7UngP\n\tExg1uFaty3lORgzd4eY3nt8lsl6diPq6SjrXMeci4Vf3dhRsSCqKYtEgbnunF18A\n\tlw+oEs7BZkG2b+9zNm7xI8KtsrGHS7zkM4MP0aF8=","Mailing-List":"contact libc-alpha-help@sourceware.org; run by ezmlm","Precedence":"bulk","List-Id":"","List-Unsubscribe":"","List-Subscribe":"","List-Archive":"","List-Post":"","List-Help":",\n\t","Sender":"libc-alpha-owner@sourceware.org","X-Virus-Found":"No","X-Spam-SWARE-Status":"No, score=-26.9 required=5.0 tests=BAYES_00, GIT_PATCH_0,\n\tGIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, RP_MATCHES_RCVD,\n\tSPF_HELO_PASS autolearn=ham version=3.3.2 spammy=patsy","X-HELO":"mx1.redhat.com","DMARC-Filter":"OpenDMARC Filter v1.3.2 mx1.redhat.com 7EE328535D","To":"GNU C Library ,\n\tPatsy Franklin , Jeff Law ","From":"Florian Weimer ","Subject":"[PATCH] Mangle NULL pointers in iconv/gconv [BZ #22025]","Message-ID":"<50ea6b93-b58a-fdad-c178-a188ff6b6728@redhat.com>","Date":"Tue, 29 Aug 2017 15:28:43 +0200","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101\n\tThunderbird/52.2.1","MIME-Version":"1.0","Content-Type":"multipart/mixed;\n\tboundary=\"------------B6CEC3F1243E2D04B61A1E38\""},"content":"We have been carrying the attached patch for a while.\n\nThere is no test because triggering this failure is very hard even on 32\nbit.\n\nBased on my review, it fixes all NULL pointer inconsistencies except the\nmangling of __end_fct after a gconv_init failure. While writing a test\nfor this omission I found a heap corruption, so I filed a separate bug\nfor these remaining issues:\n\n \n\nThanks,\nFlorian","diff":"gconv: Consistently mangle NULL function pointers [BZ #22025]\n\nNot mangling NULL pointers is not safe because with very low\nprobability, a non-NULL function pointer can turn into a NULL pointer\nafter mangling.\n\n2017-08-29 Patsy Franklin \n\t Jeff Law \n\n\t[BZ #22025]\n\tMangle NULL pointers in iconv/gconv.\n\t* iconv/gconv_cache.c (find_module): Demangle init_fct before\n\tchecking for NULL. Mangle __btowc_fct if init_fct is non-NULL.\n\t* iconv/gconv_db.c (free_derivation): Check that __shlib_handle\n\tis non-NULL before demangling the end_fct. Check for NULL\n\tend_fct after demangling.\n\t(__gconv_release_step): Demangle the end_fct before checking\n\tit for NULL. Remove assert on __shlibc_handle != NULL.\n\t(gen_steps): Don't check btowc_fct for NULL before mangling.\n\tDemangle init_fct before checking for NULL.\n\t(increment_counter): Likewise.\n\t* gconv_dl.c (__gconv_find_shlib): Don't check init_fct or\n\tend_fct for NULL before mangling.\n\t* wcsmbs/btowc.c (__btowc): Demangle btowc_fct before checking\n\tfor NULL.\n\ndiff --git a/iconv/gconv_cache.c b/iconv/gconv_cache.c\nindex d6a47de838..7d2751a506 100644\n--- a/iconv/gconv_cache.c\n+++ b/iconv/gconv_cache.c\n@@ -207,17 +207,16 @@ find_module (const char *directory, const char *filename,\n result->__data = NULL;\n \n /* Call the init function. */\n- if (result->__init_fct != NULL)\n-\t{\n-\t __gconv_init_fct init_fct = result->__init_fct;\n+ __gconv_init_fct init_fct = result->__init_fct;\n #ifdef PTR_DEMANGLE\n-\t PTR_DEMANGLE (init_fct);\n+ PTR_DEMANGLE (init_fct);\n #endif\n+ if (init_fct != NULL)\n+\t{\n \t status = DL_CALL_FCT (init_fct, (result));\n \n #ifdef PTR_MANGLE\n-\t if (result->__btowc_fct != NULL)\n-\t PTR_MANGLE (result->__btowc_fct);\n+\t PTR_MANGLE (result->__btowc_fct);\n #endif\n \t}\n }\ndiff --git a/iconv/gconv_db.c b/iconv/gconv_db.c\nindex 7893fadba1..b748467de5 100644\n--- a/iconv/gconv_db.c\n+++ b/iconv/gconv_db.c\n@@ -179,16 +179,15 @@ free_derivation (void *p)\n size_t cnt;\n \n for (cnt = 0; cnt < deriv->nsteps; ++cnt)\n- if (deriv->steps[cnt].__counter > 0\n-\t&& deriv->steps[cnt].__end_fct != NULL)\n+ if ((deriv->steps[cnt].__counter > 0)\n+\t&& (deriv->steps[cnt].__shlib_handle != NULL))\n {\n-\tassert (deriv->steps[cnt].__shlib_handle != NULL);\n-\n \t__gconv_end_fct end_fct = deriv->steps[cnt].__end_fct;\n #ifdef PTR_DEMANGLE\n \tPTR_DEMANGLE (end_fct);\n #endif\n-\tDL_CALL_FCT (end_fct, (&deriv->steps[cnt]));\n+\tif (end_fct != NULL)\n+\t DL_CALL_FCT (end_fct, (&deriv->steps[cnt]));\n }\n \n /* Free the name strings. */\n@@ -212,16 +211,12 @@ __gconv_release_step (struct __gconv_step *step)\n if (step->__shlib_handle != NULL && --step->__counter == 0)\n {\n /* Call the destructor. */\n- if (step->__end_fct != NULL)\n-\t{\n-\t assert (step->__shlib_handle != NULL);\n-\n-\t __gconv_end_fct end_fct = step->__end_fct;\n+\t__gconv_end_fct end_fct = step->__end_fct;\n #ifdef PTR_DEMANGLE\n-\t PTR_DEMANGLE (end_fct);\n+\tPTR_DEMANGLE (end_fct);\n #endif\n-\t DL_CALL_FCT (end_fct, (step));\n-\t}\n+ if (end_fct != NULL)\n+\tDL_CALL_FCT (end_fct, (step));\n \n #ifndef STATIC_GCONV\n /* Release the loaded module. */\n@@ -313,13 +308,11 @@ gen_steps (struct derivation_step *best, const char *toset,\n \n \t /* Call the init function. */\n \t __gconv_init_fct init_fct = result[step_cnt].__init_fct;\n-\t if (init_fct != NULL)\n-\t\t{\n-\t\t assert (result[step_cnt].__shlib_handle != NULL);\n-\n # ifdef PTR_DEMANGLE\n-\t\t PTR_DEMANGLE (init_fct);\n+\t PTR_DEMANGLE (init_fct);\n # endif\n+\t if (init_fct != NULL)\n+\t\t{\n \t\t status = DL_CALL_FCT (init_fct, (&result[step_cnt]));\n \n \t\t if (__builtin_expect (status, __GCONV_OK) != __GCONV_OK)\n@@ -332,8 +325,7 @@ gen_steps (struct derivation_step *best, const char *toset,\n \t\t }\n \n # ifdef PTR_MANGLE\n-\t\t if (result[step_cnt].__btowc_fct != NULL)\n-\t\t PTR_MANGLE (result[step_cnt].__btowc_fct);\n+\t\t PTR_MANGLE (result[step_cnt].__btowc_fct);\n # endif\n \t\t}\n \t }\n@@ -415,16 +407,15 @@ increment_counter (struct __gconv_step *steps, size_t nsteps)\n \n \t /* Call the init function. */\n \t __gconv_init_fct init_fct = step->__init_fct;\n+#ifdef PTR_DEMANGLE\n+\t PTR_DEMANGLE (init_fct);\n+#endif\n \t if (init_fct != NULL)\n \t {\n-#ifdef PTR_DEMANGLE\n-\t PTR_DEMANGLE (init_fct);\n-#endif\n \t DL_CALL_FCT (init_fct, (step));\n \n #ifdef PTR_MANGLE\n-\t if (step->__btowc_fct != NULL)\n-\t\tPTR_MANGLE (step->__btowc_fct);\n+\t PTR_MANGLE (step->__btowc_fct);\n #endif\n \t }\n \t}\ndiff --git a/iconv/gconv_dl.c b/iconv/gconv_dl.c\nindex 241836204d..d7dbba90a2 100644\n--- a/iconv/gconv_dl.c\n+++ b/iconv/gconv_dl.c\n@@ -131,10 +131,8 @@ __gconv_find_shlib (const char *name)\n \n #ifdef PTR_MANGLE\n \t\t PTR_MANGLE (found->fct);\n-\t\t if (found->init_fct != NULL)\n-\t\t PTR_MANGLE (found->init_fct);\n-\t\t if (found->end_fct != NULL)\n-\t\t PTR_MANGLE (found->end_fct);\n+\t\t PTR_MANGLE (found->init_fct);\n+\t\t PTR_MANGLE (found->end_fct);\n #endif\n \n \t\t /* We have succeeded in loading the shared object. */\ndiff --git a/wcsmbs/btowc.c b/wcsmbs/btowc.c\nindex 22464dc5e2..97fb7170f3 100644\n--- a/wcsmbs/btowc.c\n+++ b/wcsmbs/btowc.c\n@@ -46,15 +46,15 @@ __btowc (int c)\n /* Get the conversion functions. */\n fcts = get_gconv_fcts (_NL_CURRENT_DATA (LC_CTYPE));\n __gconv_btowc_fct btowc_fct = fcts->towc->__btowc_fct;\n+#ifdef PTR_DEMANGLE\n+ if (fcts->towc->__shlib_handle != NULL)\n+ PTR_DEMANGLE (btowc_fct);\n+#endif\n \n if (__builtin_expect (fcts->towc_nsteps == 1, 1)\n && __builtin_expect (btowc_fct != NULL, 1))\n {\n /* Use the shortcut function. */\n-#ifdef PTR_DEMANGLE\n- if (fcts->towc->__shlib_handle != NULL)\n-\tPTR_DEMANGLE (btowc_fct);\n-#endif\n return DL_CALL_FCT (btowc_fct, (fcts->towc, (unsigned char) c));\n }\n else\n","prefixes":[]}