git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH 07/18] convert trivial cases to FLEX_ARRAY macros

From
Eric Sunshine <sunshine@sunshineco.com>
Date
Feb 16, 2016, 04:18 UTC
Message-ID
<CAPig+cQOF7CsJNoiu8FAMk+csrOWG2dbEyxqfpWNwvGdUsjxcw@mail.gmail.com>
In-Reply-To
<20160216033620.GA26430@sigill.intra.peff.net>
On Mon, Feb 15, 2016 at 10:36 PM, Jeff King <peff@peff.net> wrote:
Show 11 quoted lines
> On Mon, Feb 15, 2016 at 10:26:26PM -0500, Jeff King wrote:
>> We could do this on top of my series (I can also factor out the fix
>> separately to go at the beginning if we don't want to hold the bugfix
>> hostage).
>>
>> -- >8 --
>> Subject: [PATCH] reflog_expire_cfg: drop misleading "len" parameter
>
> Here it is as a separate fix. Applying my series on top would need a
> minor and obvious tweak. I'll hold a re-roll for more comments, but will
> otherwise plan to stick this at the front of the series.

Yep, I prefer this version of the patch too, as it makes it explicit that a bug is being fixed rather than it happening "by accident" via the FLEX_ALLOC_MEM conversion, which is easily overlooked.

Show 30 quoted lines
> -- >8 --
> Subject: [PATCH] reflog_expire_cfg: NUL-terminate pattern field
>
> You can tweak the reflog expiration for a particular subset
> of refs by configuring gc.foo.reflogexpire. We keep a linked
> list of reflog_expire_cfg structs, each of which holds the
> pattern and a "len" field for the length of the pattern. The
> pattern itself is _not_ NUL-terminated.
>
> However, we feed the pattern directly to wildmatch(), which
> expects a NUL-terminated string, meaning it may keep reading
> random junk after our struct.
>
> We can fix this by allocating an extra byte for the NUL
> (which is already zero because we use xcalloc). Let's also
> drop the misleading "len" field, which is no longer
> necessary. The existing use of "len" can be converted to use
> strncmp().
>
> Signed-off-by: Jeff King <peff@peff.net>
> ---
> diff --git a/builtin/reflog.c b/builtin/reflog.c
> @@ -408,13 +407,12 @@ static struct reflog_expire_cfg *find_cfg_ent(const char *pattern, size_t len)
>                 reflog_expire_cfg_tail = &reflog_expire_cfg;
>
>         for (ent = reflog_expire_cfg; ent; ent = ent->next)
> -               if (ent->len == len &&
> -                   !memcmp(ent->pattern, pattern, len))
> +               if (!strncmp(ent->pattern, pattern, len) &&
> +                   ent->pattern[len] == '\0')

If 'ent->pattern' is shorter than 'pattern' then the strncmp() will fail, thus it will short-circuit before ent->pattern[len] has a chance to access beyond the end of memory allocated for 'ent->pattern'. Okay, makes sense.

Show 11 quoted lines
>                         return ent;
>
> -       ent = xcalloc(1, (sizeof(*ent) + len));
> +       ent = xcalloc(1, sizeof(*ent) + len + 1);
>         memcpy(ent->pattern, pattern, len);
> -       ent->len = len;
>         *reflog_expire_cfg_tail = ent;
>         reflog_expire_cfg_tail = &(ent->next);
>         return ent;
> --
> 2.7.1.574.gccd43a9
Previous: Jeff KingNext: Jeff King
Message 19 of 93 in “hardening allocations against integer overflow”
  1. 0/18 hardening allocations against integer overflowJeff King, Feb 15, 2016
  2. 01/18 add helpers for detecting size_t overflowJeff King, Feb 15, 2016
  3. 02/18 tree-diff: catch integer overflow in combine_diff_path allocationJeff King, Feb 15, 2016
  4. 03/18 harden REALLOC_ARRAY and xcalloc against size_t overflowJeff King, Feb 15, 2016
  5. 04/18 add helpers for allocating flex-array structsJeff King, Feb 15, 2016
  6. Eric SunshineFeb 16, 2016
  7. Jeff KingFeb 16, 2016
  8. 05/18 convert trivial cases to ALLOC_ARRAYJeff King, Feb 15, 2016
  9. Eric SunshineFeb 16, 2016
  10. Jeff KingFeb 16, 2016
  11. Eric SunshineFeb 16, 2016
  12. Jeff KingFeb 16, 2016
  13. 06/18 use xmallocz to avoid size arithmeticJeff King, Feb 15, 2016
  14. 07/18 convert trivial cases to FLEX_ARRAY macrosJeff King, Feb 15, 2016
  15. Eric SunshineFeb 16, 2016
  16. Jeff KingFeb 16, 2016
  17. Jeff KingFeb 16, 2016
  18. Jeff KingFeb 16, 2016
  19. Eric SunshineFeb 16, 2016
  20. Jeff KingFeb 16, 2016
  21. Eric SunshineFeb 16, 2016
  22. 08/18 use st_add and st_mult for allocation size computationJeff King, Feb 15, 2016
  23. Eric SunshineFeb 16, 2016
  24. 09/18 write_untracked_extension: use FLEX_ALLOC helperJeff King, Feb 15, 2016
  25. 10/18 fast-import: simplify allocation in start_packfileJeff King, Feb 15, 2016
  26. 11/18 fetch-pack: simplify add_sought_entryJeff King, Feb 15, 2016
  27. 12/18 test-path-utils: fix normalize_path_copy output buffer sizeJeff King, Feb 15, 2016
  28. 13/18 sequencer: simplify memory allocation of get_messageJeff King, Feb 15, 2016
  29. Eric SunshineFeb 16, 2016
  30. 14/18 git-compat-util: drop mempcpy compat codeJeff King, Feb 15, 2016
  31. Eric SunshineFeb 16, 2016
  32. 15/18 transport_anonymize_url: use xstrfmtJeff King, Feb 15, 2016
  33. 16/18 diff_populate_gitlink: use a strbufJeff King, Feb 15, 2016
  34. 17/18 convert ewah/bitmap code to use xmallocJeff King, Feb 15, 2016
  35. 18/18 ewah: convert to REALLOC_ARRAY, etcJeff King, Feb 15, 2016
  36. Jeff KingFeb 15, 2016
  37. 0/21 hardening allocations against integer overflowJeff King, Feb 19, 2016
  38. 01/21 reflog_expire_cfg: NUL-terminate pattern fieldJeff King, Feb 19, 2016
  39. 02/21 add helpers for detecting size_t overflowJeff King, Feb 19, 2016
  40. 03/21 tree-diff: catch integer overflow in combine_diff_path allocationJeff King, Feb 19, 2016
  41. 04/21 harden REALLOC_ARRAY and xcalloc against size_t overflowJeff King, Feb 19, 2016
  42. René ScharfeFeb 20, 2016
  43. Jeff KingFeb 21, 2016
  44. 05/21 add helpers for allocating flex-array structsJeff King, Feb 19, 2016
  45. 06/21 convert manual allocations to argv_arrayJeff King, Feb 19, 2016
  46. Eric SunshineFeb 20, 2016
  47. Jeff KingFeb 20, 2016
  48. Eric SunshineFeb 20, 2016
  49. Jeff KingFeb 20, 2016
  50. Eric SunshineFeb 20, 2016
  51. Jeff KingFeb 20, 2016
  52. Eric SunshineFeb 20, 2016
  53. 07/21 convert trivial cases to ALLOC_ARRAYJeff King, Feb 19, 2016
  54. 08/21 use xmallocz to avoid size arithmeticJeff King, Feb 19, 2016
  55. 09/21 convert trivial cases to FLEX_ARRAY macrosJeff King, Feb 19, 2016
  56. 10/21 use st_add and st_mult for allocation size computationJeff King, Feb 19, 2016
  57. 11/21 prepare_{git,shell}_cmd: use argv_arrayJeff King, Feb 19, 2016
  58. 12/21 write_untracked_extension: use FLEX_ALLOC helperJeff King, Feb 19, 2016
  59. 13/21 fast-import: simplify allocation in start_packfileJeff King, Feb 19, 2016
  60. Junio C HamanoFeb 19, 2016
  61. Jeff KingFeb 19, 2016
  62. 14/21 fetch-pack: simplify add_sought_entryJeff King, Feb 19, 2016
  63. 15/21 test-path-utils: fix normalize_path_copy output buffer sizeJeff King, Feb 19, 2016
  64. 16/21 sequencer: simplify memory allocation of get_messageJeff King, Feb 19, 2016
  65. 17/21 git-compat-util: drop mempcpy compat codeJeff King, Feb 19, 2016
  66. 18/21 transport_anonymize_url: use xstrfmtJeff King, Feb 19, 2016
  67. 19/21 diff_populate_gitlink: use a strbufJeff King, Feb 19, 2016
  68. 20/21 convert ewah/bitmap code to use xmallocJeff King, Feb 19, 2016
  69. 21/21 ewah: convert to REALLOC_ARRAY, etcJeff King, Feb 19, 2016
  70. 0/22 hardening allocations against integer overflowJeff King, Feb 22, 2016
  71. 01/22 reflog_expire_cfg: NUL-terminate pattern fieldJeff King, Feb 22, 2016
  72. 02/22 add helpers for detecting size_t overflowJeff King, Feb 22, 2016
  73. 03/22 tree-diff: catch integer overflow in combine_diff_path allocationJeff King, Feb 22, 2016
  74. 04/22 harden REALLOC_ARRAY and xcalloc against size_t overflowJeff King, Feb 22, 2016
  75. 05/22 add helpers for allocating flex-array structsJeff King, Feb 22, 2016
  76. 06/22 argv-array: add detach functionJeff King, Feb 22, 2016
  77. 07/22 convert manual allocations to argv_arrayJeff King, Feb 22, 2016
  78. 08/22 convert trivial cases to ALLOC_ARRAYJeff King, Feb 22, 2016
  79. 09/22 use xmallocz to avoid size arithmeticJeff King, Feb 22, 2016
  80. 10/22 convert trivial cases to FLEX_ARRAY macrosJeff King, Feb 22, 2016
  81. 11/22 use st_add and st_mult for allocation size computationJeff King, Feb 22, 2016
  82. 12/22 prepare_{git,shell}_cmd: use argv_arrayJeff King, Feb 22, 2016
  83. 13/22 write_untracked_extension: use FLEX_ALLOC helperJeff King, Feb 22, 2016
  84. 14/22 fast-import: simplify allocation in start_packfileJeff King, Feb 22, 2016
  85. 15/22 fetch-pack: simplify add_sought_entryJeff King, Feb 22, 2016
  86. 16/22 test-path-utils: fix normalize_path_copy output buffer sizeJeff King, Feb 22, 2016
  87. 17/22 sequencer: simplify memory allocation of get_messageJeff King, Feb 22, 2016
  88. 18/22 git-compat-util: drop mempcpy compat codeJeff King, Feb 22, 2016
  89. 19/22 transport_anonymize_url: use xstrfmtJeff King, Feb 22, 2016
  90. 20/22 diff_populate_gitlink: use a strbufJeff King, Feb 22, 2016
  91. 21/22 convert ewah/bitmap code to use xmallocJeff King, Feb 22, 2016
  92. 22/22 ewah: convert to REALLOC_ARRAY, etcJeff King, Feb 22, 2016
  93. Junio C HamanoFeb 22, 2016

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.