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

From: Michael Haggerty <mhagger@alum.mit.edu>
Date: Dec 6, 2011, 14:54 UTC
Message-ID: <4EDE2C95.5040804@alum.mit.edu>
In-Reply-To: <CABPQNSaE=TWGbBRMnthEuT181=XbOafPfgx88_JQnnQ6TiYyqw@mail.gmail.com>

On 12/06/2011 01:17 PM, Erik Faye-Lund wrote:

Show 17 quoted lines

> 2011/12/5 Nguyễn Thái Ngọc Duy <pclouds@gmail.com>:
>> 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.
> 
> Wouldn't it make more sense to make the limit a config option rather
> than a hard-coded value of 128 (which seems arbitrary to me)? After
> all, different platforms have different stack-limitations...

I'm confused: is the data only ever read by the same host that generated it? Because if not, then the "creator" had better never be configured to use a chain depth that the "reader" cannot handle. This in turn imply that there should be a common limit that is supported by all git clients and is a documented part of the protocol. (Or the code has to be rewritten to use an explicit stack instead of recursion.)

Michael

-- 
Michael Haggerty
mhagger@alum.mit.edu
http://softwareswirl.blogspot.com/