{"thread":{"id":"46257","subject":"NULL dereference in ab/wildmatch","startedAt":"2017-06-25T14:13:01Z","lastAt":"2017-06-25T14:20:57Z","messageCount":2,"participants":["Jeff King","Ævar Arnfjörð Bjarmason"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"323240","messageId":"20170625140957.kjkctkaesm6il5u2@sigill.intra.peff.net","threadId":"46257","inReplyTo":null,"subject":"NULL dereference in ab/wildmatch","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-06-25T14:09:57Z","receivedAt":"2017-06-25T14:13:01Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Jun 25, 2017 at 02:50:10AM -0700, scan-admin@coverity.com wrote:\n\n> \n> ** CID 1413016:  Null pointer dereferences  (FORWARD_NULL)\n> /config.c: 213 in include_by_gitdir()\n> \n> \n> ________________________________________________________________________________________________________\n> *** CID 1413016:  Null pointer dereferences  (FORWARD_NULL)\n> /config.c: 213 in include_by_gitdir()\n> 207     {\n> 208     \tstruct strbuf text = STRBUF_INIT;\n> 209     \tstruct strbuf pattern = STRBUF_INIT;\n> 210     \tint ret = 0, prefix;\n> 211     \tconst char *git_dir;\n> 212     \tint already_tried_absolute = 0;\n> >>>     CID 1413016:  Null pointer dereferences  (FORWARD_NULL)\n> >>>     Assigning: \"code\" = \"NULL\".\n> 213     \tstruct wildmatch_compiled *code = NULL;\n> 214     \n> 215     \tif (opts->git_dir)\n> 216     \t\tgit_dir = opts->git_dir;\n> 217     \telse\n> 218     \t\tgoto done;\n\nI think this comes the \"goto done\" at the bottom of the context. After\nthat label, we call wildmatch_free() unconditionally. Probably it just\nneeds:\n\ndiff --git a/config.c b/config.c\nindex 515f8518e2..cfee92ebbf 100644\n--- a/config.c\n+++ b/config.c\n@@ -260,7 +260,8 @@ static int include_by_gitdir(const struct config_options *opts,\n done:\n \tstrbuf_release(&pattern);\n \tstrbuf_release(&text);\n-\twildmatch_free(code);\n+\tif (code)\n+\t\twildmatch_free(code);\n \treturn ret;\n }\n \n\nthough arguably wildmatch_free() should be taught to handle NULL.\n\n-Peff\n"},{"id":"323241","messageId":"CACBZZX59LvCsp-rf4DatJahwgsY0cki=6E6keMiUgxK0v0KTSQ@mail.gmail.com","threadId":"46257","inReplyTo":"20170625140957.kjkctkaesm6il5u2@sigill.intra.peff.net","subject":"Re: NULL dereference in ab/wildmatch","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2017-06-25T14:20:31Z","receivedAt":"2017-06-25T14:20:57Z","isPatch":false,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"On Sun, Jun 25, 2017 at 4:09 PM, Jeff King <peff@peff.net> wrote:\n> On Sun, Jun 25, 2017 at 02:50:10AM -0700, scan-admin@coverity.com wrote:\n>\n>>\n>> ** CID 1413016:  Null pointer dereferences  (FORWARD_NULL)\n>> /config.c: 213 in include_by_gitdir()\n>>\n>>\n>> ________________________________________________________________________________________________________\n>> *** CID 1413016:  Null pointer dereferences  (FORWARD_NULL)\n>> /config.c: 213 in include_by_gitdir()\n>> 207     {\n>> 208           struct strbuf text = STRBUF_INIT;\n>> 209           struct strbuf pattern = STRBUF_INIT;\n>> 210           int ret = 0, prefix;\n>> 211           const char *git_dir;\n>> 212           int already_tried_absolute = 0;\n>> >>>     CID 1413016:  Null pointer dereferences  (FORWARD_NULL)\n>> >>>     Assigning: \"code\" = \"NULL\".\n>> 213           struct wildmatch_compiled *code = NULL;\n>> 214\n>> 215           if (opts->git_dir)\n>> 216                   git_dir = opts->git_dir;\n>> 217           else\n>> 218                   goto done;\n>\n> I think this comes the \"goto done\" at the bottom of the context. After\n> that label, we call wildmatch_free() unconditionally. Probably it just\n> needs:\n>\n> diff --git a/config.c b/config.c\n> index 515f8518e2..cfee92ebbf 100644\n> --- a/config.c\n> +++ b/config.c\n> @@ -260,7 +260,8 @@ static int include_by_gitdir(const struct config_options *opts,\n>  done:\n>         strbuf_release(&pattern);\n>         strbuf_release(&text);\n> -       wildmatch_free(code);\n> +       if (code)\n> +               wildmatch_free(code);\n>         return ret;\n>  }\n>\n>\n> though arguably wildmatch_free() should be taught to handle NULL.\n\nThanks. I'll fix this one way or the other in a future submission;\n\nJunio: Could you please eject the RFC patches in the series (i.e. 2/3\n& 3/3), i.e. just keep \"wildmatch: remove unused wildopts parameter\".\nI'm slowly refactoring / fixing bugs / adding tests for wildmatch() &\nhandling the dir.c / tree-walk.c cases. I'll submit that later after\nthe current obviously-correct \"unused parameter\" fix has cooked for a\nbit.\n"}]}