Re: [PATCH] bugreport.c: fix a crash in `git bugreport` with `--no-suffix` option

From: Taylor Blau <me@ttaylorr.com>
Date: Mar 16, 2024, 01:55 UTC
Message-ID: <ZfT8I3eoU0dBQMsv@nand.local>
In-Reply-To: <xmqqle6mdsw7.fsf@gitster.g>

On Wed, Mar 13, 2024 at 08:59:52AM -0700, Junio C Hamano wrote:

Show 19 quoted lines

> "barroit via GitGitGadget" <gitgitgadget@gmail.com> writes:
>
> > From: Jiamu Sun <barroit@linux.com>
> >
> > executing `git bugreport --no-suffix` led to a segmentation fault
> > due to strbuf_addftime() being called with a NULL option_suffix
> > variable. This occurs because negating the "--[no-]suffix" option
> > causes the parser to set option_suffix to NULL, which is not
> > handled prior to calling strbuf_addftime().
> >
> > Signed-off-by: Jiamu Sun <barroit@linux.com>
> > ---
>
> "git blame" points at 238b439d (bugreport: add tool to generate
> debugging info, 2020-04-16) that is the very beginning of this tool,
> and the bug survived 4f6460df (builtin/bugreport.c: use thread-safe
> localtime_r(), 2020-11-30).  Apparently neither commit considered
> "--suffix=<string>" would invite users to say "--no-suffix" (authors
> of them CC'ed for their input).

I can't speak for 238b439d, but at least in the case of 4f6460df, the conversion was purely about changing localtime() to localtime_r(), and nothing more.

The commit message indicates that I was blindly grepping around for 'localtime\(_.\)\?' without looking too much at the surrounding context.

Thanks, Taylor