git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jeff King <peff@peff.net>
To: git@vger.kernel.org
Cc: 王健强 <jianqiang.wang@securitygossip.com>
Subject: [PATCH 0/4] use xmalloc in more places
Date: Thu, 11 Apr 2019 09:47:36 -0400	[thread overview]
Message-ID: <20190411134736.GA28543@sigill.intra.peff.net> (raw)

It was reported on the Git security list that there are a few spots
which use a bare malloc() but don't check the return value, which could
dereference NULL. I don't think any of these are exploitable in an
interesting way, beyond Git just segfaulting more or less immediately.

But we should still be handling failures, and I think it makes sense to
be consistent about how we do it (and the other rules which come with
using xmalloc, like GIT_ALLOC_LIMIT).

This series cleans up most of the bare calls found by:

  git grep -E '[^a-z_](m|c|re)alloc\(' '*.c' :^compat :^contrib :^wrapper.c

The calls I've left are:

  - wrapper.c obviously needs to call the real functions :)

  - compat/ has functions emulating libc and system calls, and which are
    expected to return ENOMEM as appropriate

  - diff-delta will gracefully return NULL when trying to delta
    something too large, and pack-objects will skip past trying to find
    a delta. I've never seen this happen in practice, but then I
    primarily use Linux which is more than happy to overcommit on
    malloc(). I've left it unchanged, though possibly we could have an
    xmalloc_gently() if we want to enforce things like GIT_ALLOC_LIMIT
    but still do the graceful fallback thing.

  - test-hash tries to probe malloc() to see how big a buffer it can
    allocate. I doubt this even does anything useful on systems like
    Linux that overcommit. We also don't seem to ever invoke this with a
    buffer larger than 8k in the first place. So it could maybe go away
    entirely, but I left it here.

  [1/4]: test-prio-queue: use xmalloc
  [2/4]: xdiff: use git-compat-util
  [3/4]: xdiff: use xmalloc/xrealloc
  [4/4]: progress: use xmalloc/xcalloc

 progress.c                 | 18 +++++-------------
 t/helper/test-prio-queue.c |  2 +-
 xdiff/xdiff.h              |  4 ++--
 xdiff/xinclude.h           |  8 +-------
 4 files changed, 9 insertions(+), 23 deletions(-)


             reply	other threads:[~2019-04-11 13:47 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-04-11 13:47 Jeff King [this message]
2019-04-11 13:48 ` [PATCH 1/4] test-prio-queue: use xmalloc Jeff King
2019-04-11 13:48 ` [PATCH 2/4] xdiff: use git-compat-util Jeff King
2019-04-11 13:49 ` [PATCH 3/4] xdiff: use xmalloc/xrealloc Jeff King
2019-04-11 13:49 ` [PATCH 4/4] progress: use xmalloc/xcalloc Jeff King
2019-04-11 19:14 ` [PATCH 0/4] use xmalloc in more places Taylor Blau
2019-04-11 19:37   ` Jeff King
2019-04-11 19:43     ` Taylor Blau
2019-04-11 20:04       ` Jeff King

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190411134736.GA28543@sigill.intra.peff.net \
    --to=peff@peff.net \
    --cc=git@vger.kernel.org \
    --cc=jianqiang.wang@securitygossip.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).