From: Junio C Hamano <gitster@pobox•com>
To: Karsten Blees <karsten.blees@gmail•com>
Cc: Git List <git@vger•kernel.org>, msysGit <msysgit@googlegroups•com>
Subject: Re: [PATCH 2/2] dir: remove PATH_MAX limitation
Date: Wed, 09 Jul 2014 09:33:30 -0700 [thread overview]
Message-ID: <xmqqa98i7aqt.fsf@gitster.dls.corp.google.com> (raw)
In-Reply-To: <53B72DD5.6020603@gmail.com> (Karsten Blees's message of "Sat, 05 Jul 2014 00:42:29 +0200")
Karsten Blees <karsten.blees@gmail•com> writes:
> 'git status' segfaults if a directory is longer than PATH_MAX, because
> processing .gitignore files in prep_exclude() writes past the end of a
> PATH_MAX-bounded buffer.
>
> Remove the limitation by using strbuf instead.
>
> Note: this fix just 'abuses' strbuf as string allocator, len is always 0.
> prep_exclude() can probably be simplified using more strbuf APIs.
In addition to what Jeff already said, I am afraid that this may
make things a lot worse for normal case. By sizing the strbuf to
absolute minimum as you dig deeper at each level, wouldn't you copy
and reallocate the buffer a lot more, compared to the case where
everything fits in the original buffer?
> Signed-off-by: Karsten Blees <blees@dcon•de>
> ---
> dir.c | 35 +++++++++++++++++++----------------
> dir.h | 4 ++--
> 2 files changed, 21 insertions(+), 18 deletions(-)
>
> diff --git a/dir.c b/dir.c
> index e65888d..8d4d83c 100644
> --- a/dir.c
> +++ b/dir.c
> @@ -798,7 +798,7 @@ static void prep_exclude(struct dir_struct *dir, const char *base, int baselen)
> * path being checked. */
> while ((stk = dir->exclude_stack) != NULL) {
> if (stk->baselen <= baselen &&
> - !strncmp(dir->basebuf, base, stk->baselen))
> + !strncmp(dir->base.buf, base, stk->baselen))
> break;
> el = &group->el[dir->exclude_stack->exclude_ix];
> dir->exclude_stack = stk->prev;
> @@ -833,48 +833,50 @@ static void prep_exclude(struct dir_struct *dir, const char *base, int baselen)
> stk->baselen = cp - base;
> stk->exclude_ix = group->nr;
> el = add_exclude_list(dir, EXC_DIRS, NULL);
> - memcpy(dir->basebuf + current, base + current,
> + strbuf_grow(&dir->base, stk->baselen);
> + memcpy(dir->base.buf + current, base + current,
> stk->baselen - current);
>
> /* Abort if the directory is excluded */
> if (stk->baselen) {
> int dt = DT_DIR;
> - dir->basebuf[stk->baselen - 1] = 0;
> + dir->base.buf[stk->baselen - 1] = 0;
> dir->exclude = last_exclude_matching_from_lists(dir,
> - dir->basebuf, stk->baselen - 1,
> - dir->basebuf + current, &dt);
> - dir->basebuf[stk->baselen - 1] = '/';
> + dir->base.buf, stk->baselen - 1,
> + dir->base.buf + current, &dt);
> + dir->base.buf[stk->baselen - 1] = '/';
> if (dir->exclude &&
> dir->exclude->flags & EXC_FLAG_NEGATIVE)
> dir->exclude = NULL;
> if (dir->exclude) {
> - dir->basebuf[stk->baselen] = 0;
> + dir->base.buf[stk->baselen] = 0;
> dir->exclude_stack = stk;
> return;
> }
> }
>
> - /* Try to read per-directory file unless path is too long */
> - if (dir->exclude_per_dir &&
> - stk->baselen + strlen(dir->exclude_per_dir) < PATH_MAX) {
> - strcpy(dir->basebuf + stk->baselen,
> + /* Try to read per-directory file */
> + if (dir->exclude_per_dir) {
> + strbuf_grow(&dir->base, stk->baselen +
> + strlen(dir->exclude_per_dir));
> + strcpy(dir->base.buf + stk->baselen,
> dir->exclude_per_dir);
> /*
> - * dir->basebuf gets reused by the traversal, but we
> + * dir->base gets reused by the traversal, but we
> * need fname to remain unchanged to ensure the src
> * member of each struct exclude correctly
> * back-references its source file. Other invocations
> * of add_exclude_list provide stable strings, so we
> * strdup() and free() here in the caller.
> */
> - el->src = strdup(dir->basebuf);
> - add_excludes_from_file_to_list(dir->basebuf,
> - dir->basebuf, stk->baselen, el, 1);
> + el->src = strdup(dir->base.buf);
> + add_excludes_from_file_to_list(dir->base.buf,
> + dir->base.buf, stk->baselen, el, 1);
> }
> dir->exclude_stack = stk;
> current = stk->baselen;
> }
> - dir->basebuf[baselen] = '\0';
> + dir->base.buf[baselen] = '\0';
> }
>
> /*
> @@ -1671,4 +1673,5 @@ void clear_directory(struct dir_struct *dir)
> free(stk);
> stk = prev;
> }
> + strbuf_release(&dir->base);
> }
> diff --git a/dir.h b/dir.h
> index 55e5345..e870fb6 100644
> --- a/dir.h
> +++ b/dir.h
> @@ -111,13 +111,13 @@ struct dir_struct {
> * per-directory exclude lists.
> *
> * exclude_stack points to the top of the exclude_stack, and
> - * basebuf contains the full path to the current
> + * base contains the full path to the current
> * (sub)directory in the traversal. Exclude points to the
> * matching exclude struct if the directory is excluded.
> */
> struct exclude_stack *exclude_stack;
> struct exclude *exclude;
> - char basebuf[PATH_MAX];
> + struct strbuf base;
> };
>
> /*
> --
> 1.9.4.msysgit.0.5.g1471ac1
>
> --
next prev parent reply other threads:[~2014-07-09 16:33 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-07-04 22:41 [PATCH 1/2] symlinks: remove PATH_MAX limitation Karsten Blees
2014-07-04 22:42 ` [PATCH 2/2] dir: " Karsten Blees
2014-07-05 10:48 ` Duy Nguyen
2014-07-11 19:10 ` Karsten Blees
2014-07-09 5:42 ` Jeff King
2014-07-09 16:33 ` Junio C Hamano [this message]
2014-07-11 19:11 ` Karsten Blees
2014-07-11 22:29 ` Junio C Hamano
2014-07-11 23:43 ` Karsten Blees
2014-07-12 2:56 ` Duy Nguyen
2014-07-14 4:29 ` Junio C Hamano
2014-07-05 2:52 ` [PATCH 1/2] symlinks: " Johannes Schindelin
2014-07-07 18:30 ` Junio C Hamano
2014-07-11 19:11 ` Karsten Blees
2014-07-11 22:19 ` Junio C Hamano
2014-07-11 23:02 ` [PATCH] fixup! " Karsten Blees
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=xmqqa98i7aqt.fsf@gitster.dls.corp.google.com \
--to=gitster@pobox$(echo .)com \
--cc=git@vger$(echo .)kernel.org \
--cc=karsten.blees@gmail$(echo .)com \
--cc=msysgit@googlegroups$(echo .)com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
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