git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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

From
Erik Faye-Lund <kusmabite@gmail.com>
Date
Dec 6, 2011, 12:41 UTC
Message-ID
<CABPQNSbOvVkSFAE32hXoZEZeHqg-+nHc93uGytfCPQMicu0uVg@mail.gmail.com>
In-Reply-To
<CACsJy8BuUn4htSR6jAJfbueOshE6-YVV5dZGSWTGXbkuA0HO=A@mail.gmail.com>
On Tue, Dec 6, 2011 at 1:32 PM, Nguyen Thai Ngoc Duy <pclouds@gmail.com> wrote:
Show 22 quoted lines
> 2011/12/6 Erik Faye-Lund <kusmabite@gmail.com>:
>> 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...
>
> 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 ;)

Previous: Nguyen Thai Ngoc DuyNext: Nguyen Thai Ngoc Duy
Message 4 of 19 in “Set hard limit on delta chain depth”
  1. Set hard limit on delta chain depthNguyễn Thái Ngọc Duy, Dec 5, 2011
  2. Erik Faye-LundDec 6, 2011
  3. Nguyen Thai Ngoc DuyDec 6, 2011
  4. Erik Faye-LundDec 6, 2011
  5. Nguyen Thai Ngoc DuyDec 6, 2011
  6. Michael HaggertyDec 6, 2011
  7. Nguyen Thai Ngoc DuyDec 6, 2011
  8. Shawn PearceDec 6, 2011
  9. Jeff KingDec 6, 2011
  10. Junio C HamanoDec 6, 2011
  11. Nguyen Thai Ngoc DuyDec 6, 2011
  12. Junio C HamanoDec 10, 2011
  13. index-pack: eliminate unlimited recursion in get_delta_base()Nguyễn Thái Ngọc Duy, Dec 7, 2011
  14. Shawn PearceDec 8, 2011
  15. Nguyen Thai Ngoc DuyDec 8, 2011
  16. 1/2 index_pack: indent find_unresolved_detals one levelpclouds@gmail.com, Dec 8, 2011
  17. Junio C HamanoDec 9, 2011
  18. 2/2 index-pack: a naive attempt to flatten find_unresolved_deltaspclouds@gmail.com, Dec 8, 2011
  19. Shawn PearceDec 8, 2011

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.