Re: [PATCH] Set hard limit on delta chain depth

From: Nguyen Thai Ngoc Duy <pclouds@gmail.com>
Date: Dec 6, 2011, 15:45 UTC
Message-ID: <CACsJy8AZg3DgZzmPyXhCH9bGBqo9UN7-zLt_feTtpyajf5U1tw@mail.gmail.com>
In-Reply-To: <7vvcpthh97.fsf@alter.siamese.dyndns.org>

2011/12/6 Junio C Hamano <gitster@pobox.com>:

Show 19 quoted lines

> Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:
>
>> Too deep delta chains can cause stack overflow in get_base_data(). Set
>> a hard limit so that index-pack does not run out of stack. Also stop
>> people from producing such a long delta chains using "pack-object
>> --depth=<too large>"
>>
>> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
>> ---
>>  I used to make very long delta chains and triggered this in index-pack.
>>  I did not care reporting because it's my fault anyway. Think again,
>>  index-pack is called at server side and a malicious client can
>>  trigger this. This patch does not improve the situation much, but at
>>  least we won't get sigsegv at server side.
>
> Why should we treat this condition any differently from the case where the
> sender of a pack used beefier machine than you have and stuffed a huge
> object that the index-pack running on your box cannot hold in core,
> causing xmalloc() to die on your machine?

That's interesting. First of all xmalloc() is controlled by us while index-pack code might lead to stack overflow exploit (never done it, not sure if it's really pratical to do in this case).

But can I really use up all memory at server side by sending a huge pack?

> I do not think this is the right way to handle the issue. Your other patch
> to flatten the recursion to iteration looked a lot saner approach.

It may take me some time as I'm not really familar with this code. Anybody is welcome to step up and flatten the function.

-- 
Duy