git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Jeff King <peff@peff.net>
To: Enzo Matsumiya <ematsumiya@suse.de>
Cc: git@vger.kernel.org
Subject: Re: [PATCH] pager: fix crash when pager program doesn't exist
Date: Fri, 19 Nov 2021 22:42:38 -0500	[thread overview]
Message-ID: <YZhurgbXq7vhDTXC@coredump.intra.peff.net> (raw)
In-Reply-To: <20211120023246.7ynehp5v3iypfr6w@cyberdelia>

On Fri, Nov 19, 2021 at 11:32:46PM -0300, Enzo Matsumiya wrote:

> > GIT_PAGER=no-such-command git -p log
> > 
> > I had to run it with ASan to trigger a failure, as use-after-free bugs
> > aren't always deterministic.
> 
> Please use my reproducer as it's 100% reliable and consistent (same
> memory regions are affected).
> 
> I couldn't reproduce the issue with yours.

Our reproducers are triggering the same behavior. But it won't be 100%
reliable in the sense that the behavior is undefined. Depending upon
random details of the allocator, we may get a segfault, or see random
trash on the heap, or even see the old data. That's why I suggested
using ASan; it poisons the freed memory to reliably detect problems.

But at any rate, yes, it's clear that there is a bug here.

> > diff --git a/run-command.c b/run-command.c
> > index f40df01c77..92e00d9455 100644
> > --- a/run-command.c
> > +++ b/run-command.c
> > @@ -21,6 +21,7 @@ void child_process_clear(struct child_process *child)
> > {
> > 	strvec_clear(&child->args);
> > 	strvec_clear(&child->env_array);
> > +	child_process_init(child);
> > }
> > 
> > struct child_to_clean {
> 
> Of course this one works as well. And is more elegant IMHO.

Yeah, I think so, too.

> Should I submit a v2 or will you?

Why don't you put together a v2, and I can review it.

-Peff

      parent reply	other threads:[~2021-11-20  3:42 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-11-19 23:47 [PATCH] pager: fix crash when pager program doesn't exist Enzo Matsumiya
2021-11-20  1:53 ` Jeff King
2021-11-20  2:32   ` Enzo Matsumiya
2021-11-20  3:06     ` Enzo Matsumiya
2021-11-20  3:38       ` Jeff King
2021-11-20  3:42     ` Jeff King [this message]

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

  List information: http://vger.kernel.org/majordomo-info.html

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

  git send-email \
    --in-reply-to=YZhurgbXq7vhDTXC@coredump.intra.peff.net \
    --to=peff@peff.net \
    --cc=ematsumiya@suse.de \
    --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.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

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 read-only IMAP folder(s) and NNTP newsgroup(s).