From: Erik Faye-Lund Date: Tue, 06 Dec 2011 12:41:38 GMT Subject: Re: [PATCH] Set hard limit on delta chain depth Message-ID: In-Reply-To: On Tue, Dec 6, 2011 at 1:32 PM, Nguyen Thai Ngoc Duy wrote: > 2011/12/6 Erik Faye-Lund : >> 2011/12/5 Nguyễn Thái Ngọc Duy : >>> 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=" >>> >>> Signed-off-by: Nguyễn Thái Ngọc Duy >>> --- >>>  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... > > Then it'd make more sense to make a compile time config based on > platform. Can how much stack each recursion use be calculated at compile-time? If so, I agree with you. > We could have a config option that can override the default, > but I really don't see the point of making long delta chains. Aha, I figured you _did_ see a point in this, because 128 seemed excessive to me already. I was thinking more that some platforms can have a much smaller stack than (I would expect to) fit in 128 recursions (I've worked relatively recently with platforms with as small as a static 2k stack per process), so you might not be fixing the issue for such platforms. But that's not really your responsibility either ;)