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

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

2011/12/5 Nguyễn Thái Ngọc Duy [off-list ref]:

quoted

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 <redacted>
---
 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/