Git development
 help / color / mirror / Atom feed
From: Jeff King <peff@peff.net>
To: git@vger.kernel.org
Subject: [PATCH] hash-object: fix descriptor leak with --literally
Date: Wed, 18 Jan 2023 20:57:56 -0500	[thread overview]
Message-ID: <Y8ijpJqtkDTi792i@coredump.intra.peff.net> (raw)

In hash_object(), we open a descriptor for each file to hash (whether we
got the filename from the command line or --stdin-paths), but never
close it. For the traditional code path which feeds the result to
index_fd(), this is OK; it closes the descriptor for us.

But 5ba9a93b39 (hash-object: add --literally option, 2014-09-11) a
second code path which does not close the descriptor. There we need to
do so ourselves.

You can see the problem in a clone of git.git like this:

  $ git ls-files -s | grep ^100644 | cut -f2 |
    git hash-object --stdin-paths --literally >/dev/null
  fatal: could not open 'builtin/var.c' for reading: Too many open files

After this patch, it completes successfully. I didn't bother with a
test, as it's a pain to deal with descriptor limits portably, and the
fix is so trivial.

Signed-off-by: Jeff King <peff@peff.net>
---
Something I ran into while testing my hash-object fsck series, but I
broke it off here because it's really an independent bug-fix.

I do think the world would be less confusing if index_fd() didn't close
the descriptor we pass it, and then hash_file() could just do:

  fd = open();
  hash_fd(fd);
  close(fd);

which is much more readable. But it has many other callers. So even if
we wanted to untangle all that, I think it makes sense to do this
obvious fix in the meantime.

 builtin/hash-object.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/builtin/hash-object.c b/builtin/hash-object.c
index b506381502..44db83f07f 100644
--- a/builtin/hash-object.c
+++ b/builtin/hash-object.c
@@ -27,6 +27,7 @@ static int hash_literally(struct object_id *oid, int fd, const char *type, unsig
 	else
 		ret = write_object_file_literally(buf.buf, buf.len, type, oid,
 						 flags);
+	close(fd);
 	strbuf_release(&buf);
 	return ret;
 }
-- 
2.39.1.616.gd06fca9e99

             reply	other threads:[~2023-01-19  1:58 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-01-19  1:57 Jeff King [this message]
2023-01-19  6:26 ` [PATCH] hash-object: fix descriptor leak with --literally Junio C Hamano
2023-01-19  8:20   ` 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=Y8ijpJqtkDTi792i@coredump.intra.peff.net \
    --to=peff@peff.net \
    --cc=git@vger.kernel.org \
    /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