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

[PATCH 0/18] snprintf cleanups

From
Jeff King <peff@peff.net>
Date
Mar 28, 2017, 19:42 UTC
Message-ID
<20170328194255.vf7nfzzmmzxsbn36@sigill.intra.peff.net>

Our code base calls snprintf() into a fixed-size buffer in a bunch of places. Sometimes we check the result, and sometimes we accept a silent truncation. In some cases an overflow is easy given long input. In some cases it's impossible. And in some cases it depends on how big PATH_MAX is on your filesystem, and whether it's actually enforced. :)

This series attempts to give more predictable and consistent results by removing arbitrary buffer limitations. It also tries to make further audits of snprintf() easier by converting to xsnprintf() where appropriate.

There are still some snprintf() calls left after this. A few are in code that's in flux, or is being cleaned up in nearby series (several of my recent cleanup series were split off from this). A few should probably remain (e.g., git-daemon will refuse to consider a repo name larger than PATH_MAX, which may be a reasonable defense against weird memory tricks. I wouldn't be sad to see this turned into a strbuf with an explicit length policy enforced separately, though). And there were a few that I just didn't get around to converting (the dumb-http walker, for example, but I think it may need a pretty involved audit overall).

It's a lot of patches, but hopefully they're all pretty straightforward to read.

  [01/18]: do not check odb_mkstemp return value for errors
  [02/18]: odb_mkstemp: write filename into strbuf
  [03/18]: odb_mkstemp: use git_path_buf
  [04/18]: diff: avoid fixed-size buffer for patch-ids
  [05/18]: tag: use strbuf to format tag header
  [06/18]: fetch: use heap buffer to format reflog
  [07/18]: avoid using fixed PATH_MAX buffers for refs
  [08/18]: avoid using mksnpath for refs
  [09/18]: create_branch: move msg setup closer to point of use
  [10/18]: create_branch: use xstrfmt for reflog message
  [11/18]: name-rev: replace static buffer with strbuf
  [12/18]: receive-pack: print --pack-header directly into argv array
  [13/18]: replace unchecked snprintf calls with heap buffers
  [14/18]: combine-diff: replace malloc/snprintf with xstrfmt
  [15/18]: convert unchecked snprintf into xsnprintf
  [16/18]: transport-helper: replace checked snprintf with xsnprintf
  [17/18]: gc: replace local buffer with git_path
  [18/18]: daemon: use an argv_array to exec children
 bisect.c               |  8 +++++---
 branch.c               | 16 ++++++++--------
 builtin/checkout.c     |  5 ++---
 builtin/fetch.c        |  6 ++++--
 builtin/gc.c           |  8 +-------
 builtin/index-pack.c   | 22 ++++++++++++----------
 builtin/ls-remote.c    | 10 ++++++----
 builtin/name-rev.c     | 21 ++++++++++++---------
 builtin/notes.c        |  9 ++++-----
 builtin/receive-pack.c | 17 ++++++++++-------
 builtin/replace.c      | 50 +++++++++++++++++++++++++++-----------------------
 builtin/rev-parse.c    |  5 +++--
 builtin/tag.c          | 42 ++++++++++++++++++------------------------
 cache.h                |  7 +++++--
 combine-diff.c         |  7 ++++---
 daemon.c               | 38 +++++++++++++++++---------------------
 diff.c                 | 20 +++++++++++++-------
 environment.c          | 14 ++++++--------
 fast-import.c          |  9 +++++----
 grep.c                 |  4 ++--
 http.c                 | 10 +++++-----
 imap-send.c            |  2 +-
 pack-bitmap-write.c    | 14 +++++++-------
 pack-write.c           | 16 ++++++++--------
 refs.c                 | 44 ++++++++++++++++++++++++++------------------
 sha1_file.c            |  4 ++--
 submodule.c            |  2 +-
 transport-helper.c     |  5 +----
 28 files changed, 215 insertions(+), 200 deletions(-)
Next: Jeff King
Message 1 of 30 in “snprintf cleanups”
  1. 0/18 snprintf cleanupsJeff King, Mar 28, 2017
  2. 01/18 do not check odb_mkstemp return value for errorsJeff King, Mar 28, 2017
  3. 02/18 odb_mkstemp: write filename into strbufJeff King, Mar 28, 2017
  4. 03/18 odb_mkstemp: use git_path_bufJeff King, Mar 28, 2017
  5. 05/18 tag: use strbuf to format tag headerJeff King, Mar 28, 2017
  6. 09/18 create_branch: move msg setup closer to point of useJeff King, Mar 28, 2017
  7. 08/18 avoid using mksnpath for refsJeff King, Mar 28, 2017
  8. 06/18 fetch: use heap buffer to format reflogJeff King, Mar 28, 2017
  9. 04/18 diff: avoid fixed-size buffer for patch-idsJeff King, Mar 28, 2017
  10. Jeff KingMar 28, 2017
  11. 13/18 replace unchecked snprintf calls with heap buffersJeff King, Mar 28, 2017
  12. 12/18 receive-pack: print --pack-header directly into argv arrayJeff King, Mar 28, 2017
  13. 07/18 avoid using fixed PATH_MAX buffers for refsJeff King, Mar 28, 2017
  14. Junio C HamanoApr 17, 2017
  15. Jeff KingApr 18, 2017
  16. Junio C HamanoApr 18, 2017
  17. 11/18 name-rev: replace static buffer with strbufJeff King, Mar 28, 2017
  18. 15/18 convert unchecked snprintf into xsnprintfJeff King, Mar 28, 2017
  19. 10/18 create_branch: use xstrfmt for reflog messageJeff King, Mar 28, 2017
  20. 14/18 combine-diff: replace malloc/snprintf with xstrfmtJeff King, Mar 28, 2017
  21. 17/18 gc: replace local buffer with git_pathJeff King, Mar 28, 2017
  22. 16/18 transport-helper: replace checked snprintf with xsnprintfJeff King, Mar 28, 2017
  23. 18/18 daemon: use an argv_array to exec childrenJeff King, Mar 28, 2017
  24. Junio C HamanoMar 28, 2017
  25. Jeff KingMar 29, 2017
  26. Junio C HamanoMar 29, 2017
  27. Jeff KingMar 30, 2017
  28. Junio C HamanoMar 30, 2017
  29. Jeff KingMar 30, 2017
  30. Makefile: detect errors in running spatchJeff King, Mar 29, 2017

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.