{"thread":{"id":"40669","subject":"git fsck failure on OS X with files >= 4 GiB","startedAt":"2015-10-28T23:10:10Z","lastAt":"2015-11-04T19:51:04Z","messageCount":27,"participants":["Rafael Espíndola","Filipe Cabecinhas","Atousa Duprat","Junio C Hamano","Atousa Pahlevan Duprat","Randall S. Becker","Eric Sunshine","atousa.p@gmail.com","Torsten Bögershausen"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"272462","messageId":"CAG3jReJn2Pz6-bXLw6baOZaE1BHYiC+1-zN0eagigfG3umWpJA@mail.gmail.com","threadId":"40669","inReplyTo":null,"subject":"git fsck failure on OS X with files >= 4 GiB","fromName":"Rafael Espíndola","fromEmail":"rafael.espindola@gmail.com","sentAt":"2015-10-28T23:10:10Z","receivedAt":"2015-10-28T23:10:10Z","isPatch":false,"sender":{"key":"rafael.espindola@gmail.com","avatar":null},"body":"I first noticed this with \"2.4.9 (Apple Git-60)\", but it reproduces\nwith git built from 37023ba381b6d251d7140a997b39b566dbc63c42.\n\nCreate two files with just 0s:\n\n-rw-r--r--  1 espindola  staff  4294967296 28 Oct 11:09 exactly-4gib\n-rw-r--r--  1 espindola  staff  4294967295 28 Oct 11:09 one-less-than-4gib\n\n\nand run\n\ngit init\ngit add one-less-than-4gib\ngit commit -m bar\ngit fsck\ngit add exactly-4gib\ngit commit -m bar\ngit fsck\n\nThe first fsck will run with no problems, but the second one fails:\n\nerror: packed cfdaf54c9ccfd8f5e4cee562f7d5f92df13d3106 from\n.git/objects/pack/pack-ff08480fd7f767b6bd0aeb559f0f5dea2245b0b3.pack\nis corrupt\n\nUsing the very same revision on freebsd doesn't cause any errors.\n\nCheers,\nRafael\n"},{"id":"272475","messageId":"CAEDE853qg_J_asKf4SsE7mB8L7E0Tb3ReJnNWDfAfXfPw+tjhQ@mail.gmail.com","threadId":"40669","inReplyTo":"CAG3jReJn2Pz6-bXLw6baOZaE1BHYiC+1-zN0eagigfG3umWpJA@mail.gmail.com","subject":"Re: git fsck failure on OS X with files >= 4 GiB","fromName":"Filipe Cabecinhas","fromEmail":"filcab@gmail.com","sentAt":"2015-10-29T06:46:38Z","receivedAt":"2015-10-29T06:46:38Z","isPatch":false,"sender":{"key":"filcab@gmail.com","avatar":"https://gravatar.com/avatar/ff795754240a8f18498a8da02074644ff5f1969ed999b4335171d98fdb83e20f?d=mp&s=160"},"body":"I did some debugging, and it seems CC_SHA1_Update (used by\nwrite_sha1_file_prepare if APPLE_COMMON_CRYPTO is defined in the\nMakefile) takes a uint32_t as a \"length\" parameter, which explains why\nit stops working at 4GiB (UINT_MAX+1).\n\nIn the OS X 10.11 SDK header CommonCrypto/CommonDigest.h, we have:\n\ntypedef uint32_t CC_LONG;       /* 32 bit unsigned integer */\n//...\nextern int CC_SHA1_Update(CC_SHA1_CTX *c, const void *data, CC_LONG len)\n\nA possible fix would be to either call SHA1_Update with a maximum of\nUINT_MAX, looping if necessary. Or have a compatibility SHA1_Update\nfor OS X which can handle data longer than UINT_MAX.\n\nI'm not sure what the git maintainers would prefer.\n\nRegards,\n\n  Filipe\n\n\nOn Wed, Oct 28, 2015 at 4:10 PM, Rafael Espíndola\n<rafael.espindola@gmail.com> wrote:\n>\n> I first noticed this with \"2.4.9 (Apple Git-60)\", but it reproduces\n> with git built from 37023ba381b6d251d7140a997b39b566dbc63c42.\n>\n> Create two files with just 0s:\n>\n> -rw-r--r--  1 espindola  staff  4294967296 28 Oct 11:09 exactly-4gib\n> -rw-r--r--  1 espindola  staff  4294967295 28 Oct 11:09 one-less-than-4gib\n>\n>\n> and run\n>\n> git init\n> git add one-less-than-4gib\n> git commit -m bar\n> git fsck\n> git add exactly-4gib\n> git commit -m bar\n> git fsck\n>\n> The first fsck will run with no problems, but the second one fails:\n>\n> error: packed cfdaf54c9ccfd8f5e4cee562f7d5f92df13d3106 from\n> .git/objects/pack/pack-ff08480fd7f767b6bd0aeb559f0f5dea2245b0b3.pack\n> is corrupt\n>\n> Using the very same revision on freebsd doesn't cause any errors.\n>\n> Cheers,\n> Rafael\n"},{"id":"272477","messageId":"CAG3jRe+23sy1k9QNdpdn3GF3nbzPMmYO=TM=SufEq83OtwNxbA@mail.gmail.com","threadId":"40669","inReplyTo":"CAEDE8505fXAwVXx=EZwxPHvXpMByzpnXJ9LBgfx3U6VUaFbPHw@mail.gmail.com","subject":"Re: git fsck failure on OS X with files >= 4 GiB","fromName":"Rafael Espíndola","fromEmail":"rafael.espindola@gmail.com","sentAt":"2015-10-29T10:46:53Z","receivedAt":"2015-10-29T10:46:53Z","isPatch":false,"sender":{"key":"rafael.espindola@gmail.com","avatar":null},"body":"Awesome, building with\n\nNO_OPENSSL = 1\nNO_GETTEXT = 1\n\nproduces a working git :-)\n\nCheers,\nRafael\n\n\nOn 28 October 2015 at 23:37, Filipe Cabecinhas <filcab@gmail.com> wrote:\n> I did some debugging, and it seems CC_SHA1_Update (used by\n> write_sha1_file_prepare if APPLE_COMMON_CRYPTO is defined in the Makefile)\n> takes a uint32_t as a \"length\" parameter, which explains why it stops\n> working at 4GiB (UINT_MAX+1).\n>\n> In the OS X 10.11 SDK header CommonCrypto/CommonDigest.h, we have:\n>\n> typedef uint32_t CC_LONG;       /* 32 bit unsigned integer */\n> //...\n> extern int CC_SHA1_Update(CC_SHA1_CTX *c, const void *data, CC_LONG len)\n>\n> A possible fix would be to either call SHA1_Update with a maximum of\n> UINT_MAX, looping if necessary. Or have a compatibility SHA1_Update for OS X\n> which can handle data longer than UINT_MAX.\n>\n> I'm not sure what the git maintainers would prefer.\n>\n> Regards,\n>\n>   Filipe\n>\n> On Wed, Oct 28, 2015 at 4:10 PM, Rafael Espíndola\n> <rafael.espindola@gmail.com> wrote:\n>>\n>> I first noticed this with \"2.4.9 (Apple Git-60)\", but it reproduces\n>> with git built from 37023ba381b6d251d7140a997b39b566dbc63c42.\n>>\n>> Create two files with just 0s:\n>>\n>> -rw-r--r--  1 espindola  staff  4294967296 28 Oct 11:09 exactly-4gib\n>> -rw-r--r--  1 espindola  staff  4294967295 28 Oct 11:09 one-less-than-4gib\n>>\n>>\n>> and run\n>>\n>> git init\n>> git add one-less-than-4gib\n>> git commit -m bar\n>> git fsck\n>> git add exactly-4gib\n>> git commit -m bar\n>> git fsck\n>>\n>> The first fsck will run with no problems, but the second one fails:\n>>\n>> error: packed cfdaf54c9ccfd8f5e4cee562f7d5f92df13d3106 from\n>> .git/objects/pack/pack-ff08480fd7f767b6bd0aeb559f0f5dea2245b0b3.pack\n>> is corrupt\n>>\n>> Using the very same revision on freebsd doesn't cause any errors.\n>>\n>> Cheers,\n>> Rafael\n>\n>\n"},{"id":"272481","messageId":"CAEDE853n2HR-SK9_sGn8n3j0xoTb3eQa86UvdW0DBrc+Z1sDrg@mail.gmail.com","threadId":"40669","inReplyTo":"CAG3jRe+23sy1k9QNdpdn3GF3nbzPMmYO=TM=SufEq83OtwNxbA@mail.gmail.com","subject":"Re: git fsck failure on OS X with files >= 4 GiB","fromName":"Filipe Cabecinhas","fromEmail":"filcab@gmail.com","sentAt":"2015-10-29T15:15:28Z","receivedAt":"2015-10-29T15:15:28Z","isPatch":false,"sender":{"key":"filcab@gmail.com","avatar":"https://gravatar.com/avatar/ff795754240a8f18498a8da02074644ff5f1969ed999b4335171d98fdb83e20f?d=mp&s=160"},"body":"Defining BLK_SHA1 = YesPlease (when calling make) should just change\nthe SHA functions, instead of completely removing OpenSSL or\nCommonCrypto.\n\nRegards,\n  Filipe\n\n\nOn Thu, Oct 29, 2015 at 3:46 AM, Rafael Espíndola\n<rafael.espindola@gmail.com> wrote:\n> Awesome, building with\n>\n> NO_OPENSSL = 1\n> NO_GETTEXT = 1\n>\n> produces a working git :-)\n>\n> Cheers,\n> Rafael\n>\n>\n> On 28 October 2015 at 23:37, Filipe Cabecinhas <filcab@gmail.com> wrote:\n>> I did some debugging, and it seems CC_SHA1_Update (used by\n>> write_sha1_file_prepare if APPLE_COMMON_CRYPTO is defined in the Makefile)\n>> takes a uint32_t as a \"length\" parameter, which explains why it stops\n>> working at 4GiB (UINT_MAX+1).\n>>\n>> In the OS X 10.11 SDK header CommonCrypto/CommonDigest.h, we have:\n>>\n>> typedef uint32_t CC_LONG;       /* 32 bit unsigned integer */\n>> //...\n>> extern int CC_SHA1_Update(CC_SHA1_CTX *c, const void *data, CC_LONG len)\n>>\n>> A possible fix would be to either call SHA1_Update with a maximum of\n>> UINT_MAX, looping if necessary. Or have a compatibility SHA1_Update for OS X\n>> which can handle data longer than UINT_MAX.\n>>\n>> I'm not sure what the git maintainers would prefer.\n>>\n>> Regards,\n>>\n>>   Filipe\n>>\n>> On Wed, Oct 28, 2015 at 4:10 PM, Rafael Espíndola\n>> <rafael.espindola@gmail.com> wrote:\n>>>\n>>> I first noticed this with \"2.4.9 (Apple Git-60)\", but it reproduces\n>>> with git built from 37023ba381b6d251d7140a997b39b566dbc63c42.\n>>>\n>>> Create two files with just 0s:\n>>>\n>>> -rw-r--r--  1 espindola  staff  4294967296 28 Oct 11:09 exactly-4gib\n>>> -rw-r--r--  1 espindola  staff  4294967295 28 Oct 11:09 one-less-than-4gib\n>>>\n>>>\n>>> and run\n>>>\n>>> git init\n>>> git add one-less-than-4gib\n>>> git commit -m bar\n>>> git fsck\n>>> git add exactly-4gib\n>>> git commit -m bar\n>>> git fsck\n>>>\n>>> The first fsck will run with no problems, but the second one fails:\n>>>\n>>> error: packed cfdaf54c9ccfd8f5e4cee562f7d5f92df13d3106 from\n>>> .git/objects/pack/pack-ff08480fd7f767b6bd0aeb559f0f5dea2245b0b3.pack\n>>> is corrupt\n>>>\n>>> Using the very same revision on freebsd doesn't cause any errors.\n>>>\n>>> Cheers,\n>>> Rafael\n>>\n>>\n"},{"id":"272483","messageId":"CA+izobtdwszVrYsnKU=_ytLuNbPGyRe_7kXqyrQO7u5Lo+OdPg@mail.gmail.com","threadId":"40669","inReplyTo":"CAEDE853n2HR-SK9_sGn8n3j0xoTb3eQa86UvdW0DBrc+Z1sDrg@mail.gmail.com","subject":"Re: git fsck failure on OS X with files >= 4 GiB","fromName":"Atousa Duprat","fromEmail":"atousa.p@gmail.com","sentAt":"2015-10-29T16:02:49Z","receivedAt":"2015-10-29T16:02:49Z","isPatch":false,"sender":{"key":"atousa.p@gmail.com","avatar":"https://gravatar.com/avatar/b5b7b70553497c4ce08a835c156ff3b245366eba956fbceff99553b1ccdc4e42?d=mp&s=160"},"body":"Here is a solution that avoids problems with OS-specific\nimplementations of SHA_Update() by limiting the size of each update\nrequest to 1GiB and calling the function repeatedly in a loop.\n\nAtousa\n\n--\n\n[PATCH] Limit the size of the data block passed to SHA1_Update()\n\nThis avoids issues where OS-specific implementations use\na 32-bit integer to specify block size.  Limit currently\nset to 1GiB.\n---\n cache.h | 20 +++++++++++++++++++-\n 1 file changed, 19 insertions(+), 1 deletion(-)\n\ndiff --git a/cache.h b/cache.h\nindex 79066e5..c305985 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -14,10 +14,28 @@\n #ifndef git_SHA_CTX\n #define git_SHA_CTX SHA_CTX\n #define git_SHA1_Init SHA1_Init\n-#define git_SHA1_Update SHA1_Update\n #define git_SHA1_Final SHA1_Final\n #endif\n\n+#define SHA1_MAX_BLOCK_SIZE (1024*1024*1024)\n+\n+static inline int git_SHA1_Update(SHA_CTX *c, const void *data, size_t len)\n+{\n+ size_t nr;\n+ size_t total = 0;\n+ char *cdata = (char*)data;\n+ while(len > 0) {\n+ nr = len;\n+ if(nr > SHA1_MAX_BLOCK_SIZE)\n+ nr = SHA1_MAX_BLOCK_SIZE;\n+ SHA1_Update(c, cdata, nr);\n+ total += nr;\n+ cdata += nr;\n+ len -= nr;\n+ }\n+ return total;\n+}\n+\n #include <zlib.h>\n typedef struct git_zstream {\n  z_stream z;\n-- \n2.4.9 (Apple Git-60)\n\n\nOn Thu, Oct 29, 2015 at 8:15 AM, Filipe Cabecinhas <filcab@gmail.com> wrote:\n> Defining BLK_SHA1 = YesPlease (when calling make) should just change\n> the SHA functions, instead of completely removing OpenSSL or\n> CommonCrypto.\n>\n> Regards,\n>   Filipe\n>\n>\n> On Thu, Oct 29, 2015 at 3:46 AM, Rafael Espíndola\n> <rafael.espindola@gmail.com> wrote:\n>> Awesome, building with\n>>\n>> NO_OPENSSL = 1\n>> NO_GETTEXT = 1\n>>\n>> produces a working git :-)\n>>\n>> Cheers,\n>> Rafael\n>>\n>>\n>> On 28 October 2015 at 23:37, Filipe Cabecinhas <filcab@gmail.com> wrote:\n>>> I did some debugging, and it seems CC_SHA1_Update (used by\n>>> write_sha1_file_prepare if APPLE_COMMON_CRYPTO is defined in the Makefile)\n>>> takes a uint32_t as a \"length\" parameter, which explains why it stops\n>>> working at 4GiB (UINT_MAX+1).\n>>>\n>>> In the OS X 10.11 SDK header CommonCrypto/CommonDigest.h, we have:\n>>>\n>>> typedef uint32_t CC_LONG;       /* 32 bit unsigned integer */\n>>> //...\n>>> extern int CC_SHA1_Update(CC_SHA1_CTX *c, const void *data, CC_LONG len)\n>>>\n>>> A possible fix would be to either call SHA1_Update with a maximum of\n>>> UINT_MAX, looping if necessary. Or have a compatibility SHA1_Update for OS X\n>>> which can handle data longer than UINT_MAX.\n>>>\n>>> I'm not sure what the git maintainers would prefer.\n>>>\n>>> Regards,\n>>>\n>>>   Filipe\n>>>\n>>> On Wed, Oct 28, 2015 at 4:10 PM, Rafael Espíndola\n>>> <rafael.espindola@gmail.com> wrote:\n>>>>\n>>>> I first noticed this with \"2.4.9 (Apple Git-60)\", but it reproduces\n>>>> with git built from 37023ba381b6d251d7140a997b39b566dbc63c42.\n>>>>\n>>>> Create two files with just 0s:\n>>>>\n>>>> -rw-r--r--  1 espindola  staff  4294967296 28 Oct 11:09 exactly-4gib\n>>>> -rw-r--r--  1 espindola  staff  4294967295 28 Oct 11:09 one-less-than-4gib\n>>>>\n>>>>\n>>>> and run\n>>>>\n>>>> git init\n>>>> git add one-less-than-4gib\n>>>> git commit -m bar\n>>>> git fsck\n>>>> git add exactly-4gib\n>>>> git commit -m bar\n>>>> git fsck\n>>>>\n>>>> The first fsck will run with no problems, but the second one fails:\n>>>>\n>>>> error: packed cfdaf54c9ccfd8f5e4cee562f7d5f92df13d3106 from\n>>>> .git/objects/pack/pack-ff08480fd7f767b6bd0aeb559f0f5dea2245b0b3.pack\n>>>> is corrupt\n>>>>\n>>>> Using the very same revision on freebsd doesn't cause any errors.\n>>>>\n>>>> Cheers,\n>>>> Rafael\n>>>\n>>>\n> --\n> To unsubscribe from this list: send the line \"unsubscribe git\" in\n> the body of a message to majordomo@vger.kernel.org\n> More majordomo info at  http://vger.kernel.org/majordomo-info.html\n\n\n\n-- \nAtousa Pahlevan, PhD\nM.Math. University of Waterloo, Canada\nPh.D. Department of Computer Science, University of Victoria, Canada\nVoice: 415-341-6206\nEmail: apahlevan@ieee.org\nWebsite: www.apahlevan.org\n"},{"id":"272485","messageId":"xmqqlhalsict.fsf@gitster.mtv.corp.google.com","threadId":"40669","inReplyTo":"CA+izobtdwszVrYsnKU=_ytLuNbPGyRe_7kXqyrQO7u5Lo+OdPg@mail.gmail.com","subject":"Re: git fsck failure on OS X with files >= 4 GiB","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-10-29T17:19:14Z","receivedAt":"2015-10-29T17:19:14Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Atousa Duprat <atousa.p@gmail.com> writes:\n\n> [PATCH] Limit the size of the data block passed to SHA1_Update()\n>\n> This avoids issues where OS-specific implementations use\n> a 32-bit integer to specify block size.  Limit currently\n> set to 1GiB.\n> ---\n>  cache.h | 20 +++++++++++++++++++-\n>  1 file changed, 19 insertions(+), 1 deletion(-)\n>\n> diff --git a/cache.h b/cache.h\n> index 79066e5..c305985 100644\n> --- a/cache.h\n> +++ b/cache.h\n> @@ -14,10 +14,28 @@\n>  #ifndef git_SHA_CTX\n>  #define git_SHA_CTX SHA_CTX\n>  #define git_SHA1_Init SHA1_Init\n> -#define git_SHA1_Update SHA1_Update\n>  #define git_SHA1_Final SHA1_Final\n>  #endif\n>\n> +#define SHA1_MAX_BLOCK_SIZE (1024*1024*1024)\n> +\n> +static inline int git_SHA1_Update(SHA_CTX *c, const void *data, size_t len)\n> +{\n> + size_t nr;\n> + size_t total = 0;\n> + char *cdata = (char*)data;\n> + while(len > 0) {\n> + nr = len;\n> + if(nr > SHA1_MAX_BLOCK_SIZE)\n> + nr = SHA1_MAX_BLOCK_SIZE;\n> + SHA1_Update(c, cdata, nr);\n> + total += nr;\n> + cdata += nr;\n> + len -= nr;\n> + }\n> + return total;\n> +}\n> +\n\nI think the idea illustrated above is a good start, but there are\na few issues:\n\n * SHA1_Update() is used in fairly many places; it is unclear if it\n   is a good idea to inline.\n\n * There is no need to punish implementations with working\n   SHA1_Update by another level of wrapping.\n\n * What would you do when you find an implementation for which 1G is\n   still too big?\n\nPerhaps something like this in the header\n\n#ifdef SHA1_MAX_BLOCK_SIZE\nextern int SHA1_Update_Chunked(SHA_CTX *, const void *, size_t);\n#define git_SHA1_Update SHA1_Update_Chunked\n#endif\n\nwith compat/sha1_chunked.c that has\n\n#ifdef SHA1_MAX_BLOCK_SIZE\nint SHA1_Update_Chunked(SHA_CTX *c, const void *data, size_t len)\n{\n\t... your looping implementation ...\n}\n#endif\n\nin it, that is only triggered via a Makefile macro, e.g. \nmight be a good workaround.\n\ndiff --git a/Makefile b/Makefile\nindex 8466333..83348b8 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -139,6 +139,10 @@ all::\n # Define PPC_SHA1 environment variable when running make to make use of\n # a bundled SHA1 routine optimized for PowerPC.\n #\n+# Define SHA1_MAX_BLOCK_SIZE if your SSH1_Update() implementation can\n+# hash only a limited amount of data in one call (e.g. APPLE_COMMON_CRYPTO\n+# may want 'SHA1_MAX_BLOCK_SIZE=1024L*1024L*1024L' defined).\n+#\n # Define NEEDS_CRYPTO_WITH_SSL if you need -lcrypto when using -lssl (Darwin).\n #\n # Define NEEDS_SSL_WITH_CRYPTO if you need -lssl when using -lcrypto (Darwin).\n@@ -1002,6 +1006,7 @@ ifeq ($(uname_S),Darwin)\n \tifndef NO_APPLE_COMMON_CRYPTO\n \t\tAPPLE_COMMON_CRYPTO = YesPlease\n \t\tCOMPAT_CFLAGS += -DAPPLE_COMMON_CRYPTO\n+\t\tSHA1_MAX_BLOCK_SIZE=1024L*1024L*1024L\n \tendif\n \tNO_REGEX = YesPlease\n \tPTHREAD_LIBS =\n@@ -1350,6 +1355,11 @@ endif\n endif\n endif\n \n+ifdef SHA1_MAX_BLOCK_SIZE\n+LIB_OBJS += compat/sha1_chunked.o\n+BASIC_CFLAGS += SHA1_MAX_BLOCK_SIZE=\"$(SHA1_MAX_BLOCK_SIZE)\"\n+endif\n+\n ifdef NO_PERL_MAKEMAKER\n \texport NO_PERL_MAKEMAKER\n endif\n"},{"id":"272513","messageId":"CA+izobsBmYHHepYka795K2VnVLYBmN2tFqEyzSweMoS9gvuRVw@mail.gmail.com","threadId":"40669","inReplyTo":"xmqqlhalsict.fsf@gitster.mtv.corp.google.com","subject":"Re: git fsck failure on OS X with files >= 4 GiB","fromName":"Atousa Duprat","fromEmail":"atousa.p@gmail.com","sentAt":"2015-10-30T02:15:05Z","receivedAt":"2015-10-30T02:15:05Z","isPatch":false,"sender":{"key":"atousa.p@gmail.com","avatar":"https://gravatar.com/avatar/b5b7b70553497c4ce08a835c156ff3b245366eba956fbceff99553b1ccdc4e42?d=mp&s=160"},"body":"Thank you for the feedback.  I have revised the proposed patch as\nsuggested, allowing the use of SHA1_MAX_BLOCK_SIZE to enable the\nchunked implementation.  When building for OSX with the CommonCrypto\nlibrary we error out if SHA1_MAX_BLOCK_SIZE is not defined, which will\navoid compiling a version of the tool that won't compute hashes\nproperly on large files.  It should be easy to enable this on other\nplatforms if needed.\n\nAtousa\n\nOn Thu, Oct 29, 2015 at 10:19 AM, Junio C Hamano <gitster@pobox.com> wrote:\n> Atousa Duprat <atousa.p@gmail.com> writes:\n>\n>> [PATCH] Limit the size of the data block passed to SHA1_Update()\n>>\n>> This avoids issues where OS-specific implementations use\n>> a 32-bit integer to specify block size.  Limit currently\n>> set to 1GiB.\n>> ---\n>>  cache.h | 20 +++++++++++++++++++-\n>>  1 file changed, 19 insertions(+), 1 deletion(-)\n>>\n>> diff --git a/cache.h b/cache.h\n>> index 79066e5..c305985 100644\n>> --- a/cache.h\n>> +++ b/cache.h\n>> @@ -14,10 +14,28 @@\n>>  #ifndef git_SHA_CTX\n>>  #define git_SHA_CTX SHA_CTX\n>>  #define git_SHA1_Init SHA1_Init\n>> -#define git_SHA1_Update SHA1_Update\n>>  #define git_SHA1_Final SHA1_Final\n>>  #endif\n>>\n>> +#define SHA1_MAX_BLOCK_SIZE (1024*1024*1024)\n>> +\n>> +static inline int git_SHA1_Update(SHA_CTX *c, const void *data, size_t len)\n>> +{\n>> + size_t nr;\n>> + size_t total = 0;\n>> + char *cdata = (char*)data;\n>> + while(len > 0) {\n>> + nr = len;\n>> + if(nr > SHA1_MAX_BLOCK_SIZE)\n>> + nr = SHA1_MAX_BLOCK_SIZE;\n>> + SHA1_Update(c, cdata, nr);\n>> + total += nr;\n>> + cdata += nr;\n>> + len -= nr;\n>> + }\n>> + return total;\n>> +}\n>> +\n>\n> I think the idea illustrated above is a good start, but there are\n> a few issues:\n>\n>  * SHA1_Update() is used in fairly many places; it is unclear if it\n>    is a good idea to inline.\n>\n>  * There is no need to punish implementations with working\n>    SHA1_Update by another level of wrapping.\n>\n>  * What would you do when you find an implementation for which 1G is\n>    still too big?\n>\n> Perhaps something like this in the header\n>\n> #ifdef SHA1_MAX_BLOCK_SIZE\n> extern int SHA1_Update_Chunked(SHA_CTX *, const void *, size_t);\n> #define git_SHA1_Update SHA1_Update_Chunked\n> #endif\n>\n> with compat/sha1_chunked.c that has\n>\n> #ifdef SHA1_MAX_BLOCK_SIZE\n> int SHA1_Update_Chunked(SHA_CTX *c, const void *data, size_t len)\n> {\n>         ... your looping implementation ...\n> }\n> #endif\n>\n> in it, that is only triggered via a Makefile macro, e.g.\n> might be a good workaround.\n>\n> diff --git a/Makefile b/Makefile\n> index 8466333..83348b8 100644\n> --- a/Makefile\n> +++ b/Makefile\n> @@ -139,6 +139,10 @@ all::\n>  # Define PPC_SHA1 environment variable when running make to make use of\n>  # a bundled SHA1 routine optimized for PowerPC.\n>  #\n> +# Define SHA1_MAX_BLOCK_SIZE if your SSH1_Update() implementation can\n> +# hash only a limited amount of data in one call (e.g. APPLE_COMMON_CRYPTO\n> +# may want 'SHA1_MAX_BLOCK_SIZE=1024L*1024L*1024L' defined).\n> +#\n>  # Define NEEDS_CRYPTO_WITH_SSL if you need -lcrypto when using -lssl (Darwin).\n>  #\n>  # Define NEEDS_SSL_WITH_CRYPTO if you need -lssl when using -lcrypto (Darwin).\n> @@ -1002,6 +1006,7 @@ ifeq ($(uname_S),Darwin)\n>         ifndef NO_APPLE_COMMON_CRYPTO\n>                 APPLE_COMMON_CRYPTO = YesPlease\n>                 COMPAT_CFLAGS += -DAPPLE_COMMON_CRYPTO\n> +               SHA1_MAX_BLOCK_SIZE=1024L*1024L*1024L\n>         endif\n>         NO_REGEX = YesPlease\n>         PTHREAD_LIBS =\n> @@ -1350,6 +1355,11 @@ endif\n>  endif\n>  endif\n>\n> +ifdef SHA1_MAX_BLOCK_SIZE\n> +LIB_OBJS += compat/sha1_chunked.o\n> +BASIC_CFLAGS += SHA1_MAX_BLOCK_SIZE=\"$(SHA1_MAX_BLOCK_SIZE)\"\n> +endif\n> +\n>  ifdef NO_PERL_MAKEMAKER\n>         export NO_PERL_MAKEMAKER\n>  endif\n\n\n\n-- \nAtousa Pahlevan, PhD\nM.Math. University of Waterloo, Canada\nPh.D. Department of Computer Science, University of Victoria, Canada\nVoice: 415-341-6206\nEmail: apahlevan@ieee.org\nWebsite: www.apahlevan.org\n"},{"id":"272588","messageId":"1446243122-21464-1-git-send-email-apahlevan@ieee.org","threadId":"40669","inReplyTo":"CA+izobsBmYHHepYka795K2VnVLYBmN2tFqEyzSweMoS9gvuRVw@mail.gmail.com","subject":"[PATCH] Limit the size of the data block passed to SHA1_Update()","fromName":"Atousa Pahlevan Duprat","fromEmail":"atousa.p@gmail.com","sentAt":"2015-10-30T22:12:02Z","receivedAt":"2015-10-30T22:12:02Z","isPatch":true,"sender":{"key":"atousa.p@gmail.com","avatar":"https://gravatar.com/avatar/b5b7b70553497c4ce08a835c156ff3b245366eba956fbceff99553b1ccdc4e42?d=mp&s=160"},"body":"Some implementations of SHA_Updates have inherent limits\non the max chunk size. SHA1_MAX_BLOCK_SIZE can be defined\nto set the max chunk size supported, if required.  This is\nenabled for OSX CommonCrypto library and set to 1GiB.\n---\n Makefile                     |  9 +++++++++\n cache.h                      |  7 ++++++-\n compat/apple-common-crypto.h |  4 ++++\n compat/sha1_chunked.c        | 20 ++++++++++++++++++++\n 4 files changed, 39 insertions(+), 1 deletion(-)\n create mode 100644 compat/sha1_chunked.c\n\ndiff --git a/Makefile b/Makefile\nindex 04c2231..5955542 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -141,6 +141,10 @@ all::\n # Define PPC_SHA1 environment variable when running make to make use of\n # a bundled SHA1 routine optimized for PowerPC.\n #\n+# Define SHA1_MAX_BLOCK_SIZE if your SSH1_Update() implementation can\n+# hash only a limited amount of data in one call (e.g. APPLE_COMMON_CRYPTO\n+# may want 'SHA1_MAX_BLOCK_SIZE=1024L*1024L*1024L' defined).\n+#\n # Define NEEDS_CRYPTO_WITH_SSL if you need -lcrypto when using -lssl (Darwin).\n #\n # Define NEEDS_SSL_WITH_CRYPTO if you need -lssl when using -lcrypto (Darwin).\n@@ -1346,6 +1350,7 @@ else\n ifdef APPLE_COMMON_CRYPTO\n \tCOMPAT_CFLAGS += -DCOMMON_DIGEST_FOR_OPENSSL\n \tSHA1_HEADER = <CommonCrypto/CommonDigest.h>\n+\tSHA1_MAX_BLOCK_SIZE = 1024L*1024L*1024L\n else\n \tSHA1_HEADER = <openssl/sha.h>\n \tEXTLIBS += $(LIB_4_CRYPTO)\n@@ -1353,6 +1358,10 @@ endif\n endif\n endif\n \n+ifdef SHA1_MAX_BLOCK_SIZE\n+\tLIB_OBJS += compat/sha1_chunked.o\n+\tBASIC_CFLAGS += -DSHA1_MAX_BLOCK_SIZE=\"$(SHA1_MAX_BLOCK_SIZE)\"\n+endif\n ifdef NO_PERL_MAKEMAKER\n \texport NO_PERL_MAKEMAKER\n endif\ndiff --git a/cache.h b/cache.h\nindex 79066e5..ec84b16 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -14,7 +14,12 @@\n #ifndef git_SHA_CTX\n #define git_SHA_CTX\tSHA_CTX\n #define git_SHA1_Init\tSHA1_Init\n-#define git_SHA1_Update\tSHA1_Update\n+#ifdef SHA1_MAX_BLOCK_SIZE\n+extern int SHA1_Update_Chunked(SHA_CTX *, const void *, size_t);\n+#define git_SHA1_Update SHA1_Update_Chunked\n+#else\n+#define git_SHA1_Update SHA1_Update\n+#endif\n #define git_SHA1_Final\tSHA1_Final\n #endif\n \ndiff --git a/compat/apple-common-crypto.h b/compat/apple-common-crypto.h\nindex c8b9b0e..83668fd 100644\n--- a/compat/apple-common-crypto.h\n+++ b/compat/apple-common-crypto.h\n@@ -16,6 +16,10 @@\n #undef TYPE_BOOL\n #endif\n \n+#ifndef SHA1_MAX_BLOCK_SIZE\n+#error \"Using Apple Common Crypto library requires setting SHA1_MAX_BLOCK_SIZE\"\n+#endif\n+\n #ifdef APPLE_LION_OR_NEWER\n #define git_CC_error_check(pattern, err) \\\n \tdo { \\\ndiff --git a/compat/sha1_chunked.c b/compat/sha1_chunked.c\nnew file mode 100644\nindex 0000000..4a8e4f7\n--- /dev/null\n+++ b/compat/sha1_chunked.c\n@@ -0,0 +1,20 @@\n+#include \"cache.h\"\n+\n+#ifdef SHA1_MAX_BLOCK_SIZE\n+int git_SHA1_Update(SHA_CTX *c, const void *data, size_t len)\n+{\n+\tsize_t nr;\n+\tsize_t total = 0;\n+\tchar *cdata = (char*)data;\n+\twhile(len > 0) {\n+\t\tnr = len;\n+\t\tif(nr > SHA1_MAX_BLOCK_SIZE)\n+\t\t\tnr = SHA1_MAX_BLOCK_SIZE;\n+\t\tSHA1_Update(c, cdata, nr);\n+\t\ttotal += nr;\n+\t\tcdata += nr;\n+\t\tlen -= nr;\n+\t}\n+\treturn total;\n+}\n+#endif\n-- \n2.4.9 (Apple Git-60)\n"},{"id":"272589","messageId":"1446243500-21580-1-git-send-email-apahlevan@ieee.org","threadId":"40669","inReplyTo":"CA+izobsBmYHHepYka795K2VnVLYBmN2tFqEyzSweMoS9gvuRVw@mail.gmail.com","subject":"[PATCH] Limit the size of the data block passed to SHA1_Update()","fromName":"Atousa Pahlevan Duprat","fromEmail":"atousa.p@gmail.com","sentAt":"2015-10-30T22:18:20Z","receivedAt":"2015-10-30T22:18:20Z","isPatch":true,"sender":{"key":"atousa.p@gmail.com","avatar":"https://gravatar.com/avatar/b5b7b70553497c4ce08a835c156ff3b245366eba956fbceff99553b1ccdc4e42?d=mp&s=160"},"body":"Some implementations of SHA_Updates have inherent limits\non the max chunk size. SHA1_MAX_BLOCK_SIZE can be defined\nto set the max chunk size supported, if required.  This is\nenabled for OSX CommonCrypto library and set to 1GiB.\n---\n Makefile                     |  9 +++++++++\n cache.h                      |  7 ++++++-\n compat/apple-common-crypto.h |  4 ++++\n compat/sha1_chunked.c        | 20 ++++++++++++++++++++\n 4 files changed, 39 insertions(+), 1 deletion(-)\n create mode 100644 compat/sha1_chunked.c\n\ndiff --git a/Makefile b/Makefile\nindex 04c2231..5955542 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -141,6 +141,10 @@ all::\n # Define PPC_SHA1 environment variable when running make to make use of\n # a bundled SHA1 routine optimized for PowerPC.\n #\n+# Define SHA1_MAX_BLOCK_SIZE if your SSH1_Update() implementation can\n+# hash only a limited amount of data in one call (e.g. APPLE_COMMON_CRYPTO\n+# may want 'SHA1_MAX_BLOCK_SIZE=1024L*1024L*1024L' defined).\n+#\n # Define NEEDS_CRYPTO_WITH_SSL if you need -lcrypto when using -lssl (Darwin).\n #\n # Define NEEDS_SSL_WITH_CRYPTO if you need -lssl when using -lcrypto (Darwin).\n@@ -1346,6 +1350,7 @@ else\n ifdef APPLE_COMMON_CRYPTO\n \tCOMPAT_CFLAGS += -DCOMMON_DIGEST_FOR_OPENSSL\n \tSHA1_HEADER = <CommonCrypto/CommonDigest.h>\n+\tSHA1_MAX_BLOCK_SIZE = 1024L*1024L*1024L\n else\n \tSHA1_HEADER = <openssl/sha.h>\n \tEXTLIBS += $(LIB_4_CRYPTO)\n@@ -1353,6 +1358,10 @@ endif\n endif\n endif\n \n+ifdef SHA1_MAX_BLOCK_SIZE\n+\tLIB_OBJS += compat/sha1_chunked.o\n+\tBASIC_CFLAGS += -DSHA1_MAX_BLOCK_SIZE=\"$(SHA1_MAX_BLOCK_SIZE)\"\n+endif\n ifdef NO_PERL_MAKEMAKER\n \texport NO_PERL_MAKEMAKER\n endif\ndiff --git a/cache.h b/cache.h\nindex 79066e5..ec84b16 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -14,7 +14,12 @@\n #ifndef git_SHA_CTX\n #define git_SHA_CTX\tSHA_CTX\n #define git_SHA1_Init\tSHA1_Init\n-#define git_SHA1_Update\tSHA1_Update\n+#ifdef SHA1_MAX_BLOCK_SIZE\n+extern int SHA1_Update_Chunked(SHA_CTX *, const void *, size_t);\n+#define git_SHA1_Update SHA1_Update_Chunked\n+#else\n+#define git_SHA1_Update SHA1_Update\n+#endif\n #define git_SHA1_Final\tSHA1_Final\n #endif\n \ndiff --git a/compat/apple-common-crypto.h b/compat/apple-common-crypto.h\nindex c8b9b0e..83668fd 100644\n--- a/compat/apple-common-crypto.h\n+++ b/compat/apple-common-crypto.h\n@@ -16,6 +16,10 @@\n #undef TYPE_BOOL\n #endif\n \n+#ifndef SHA1_MAX_BLOCK_SIZE\n+#error \"Using Apple Common Crypto library requires setting SHA1_MAX_BLOCK_SIZE\"\n+#endif\n+\n #ifdef APPLE_LION_OR_NEWER\n #define git_CC_error_check(pattern, err) \\\n \tdo { \\\ndiff --git a/compat/sha1_chunked.c b/compat/sha1_chunked.c\nnew file mode 100644\nindex 0000000..4a8e4f7\n--- /dev/null\n+++ b/compat/sha1_chunked.c\n@@ -0,0 +1,20 @@\n+#include \"cache.h\"\n+\n+#ifdef SHA1_MAX_BLOCK_SIZE\n+int git_SHA1_Update(SHA_CTX *c, const void *data, size_t len)\n+{\n+\tsize_t nr;\n+\tsize_t total = 0;\n+\tchar *cdata = (char*)data;\n+\twhile(len > 0) {\n+\t\tnr = len;\n+\t\tif(nr > SHA1_MAX_BLOCK_SIZE)\n+\t\t\tnr = SHA1_MAX_BLOCK_SIZE;\n+\t\tSHA1_Update(c, cdata, nr);\n+\t\ttotal += nr;\n+\t\tcdata += nr;\n+\t\tlen -= nr;\n+\t}\n+\treturn total;\n+}\n+#endif\n-- \n2.4.9 (Apple Git-60)\n"},{"id":"272590","messageId":"xmqqa8r0j8tu.fsf@gitster.mtv.corp.google.com","threadId":"40669","inReplyTo":"1446243122-21464-1-git-send-email-apahlevan@ieee.org","subject":"Re: [PATCH] Limit the size of the data block passed to SHA1_Update()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-10-30T22:22:05Z","receivedAt":"2015-10-30T22:22:05Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Atousa Pahlevan Duprat <atousa.p@gmail.com> writes:\n\n> Some implementations of SHA_Updates have inherent limits\n> on the max chunk size. SHA1_MAX_BLOCK_SIZE can be defined\n> to set the max chunk size supported, if required.  This is\n> enabled for OSX CommonCrypto library and set to 1GiB.\n> ---\n\nMissing sign-off.\n\n>  Makefile                     |  9 +++++++++\n>  cache.h                      |  7 ++++++-\n>  compat/apple-common-crypto.h |  4 ++++\n>  compat/sha1_chunked.c        | 20 ++++++++++++++++++++\n>  4 files changed, 39 insertions(+), 1 deletion(-)\n>  create mode 100644 compat/sha1_chunked.c\n>\n> diff --git a/Makefile b/Makefile\n> index 04c2231..5955542 100644\n> --- a/Makefile\n> +++ b/Makefile\n> @@ -141,6 +141,10 @@ all::\n>  # Define PPC_SHA1 environment variable when running make to make use of\n>  # a bundled SHA1 routine optimized for PowerPC.\n>  #\n> +# Define SHA1_MAX_BLOCK_SIZE if your SSH1_Update() implementation can\n> +# hash only a limited amount of data in one call (e.g. APPLE_COMMON_CRYPTO\n> +# may want 'SHA1_MAX_BLOCK_SIZE=1024L*1024L*1024L' defined).\n> +#\n>  # Define NEEDS_CRYPTO_WITH_SSL if you need -lcrypto when using -lssl (Darwin).\n>  #\n>  # Define NEEDS_SSL_WITH_CRYPTO if you need -lssl when using -lcrypto (Darwin).\n> @@ -1346,6 +1350,7 @@ else\n>  ifdef APPLE_COMMON_CRYPTO\n>  \tCOMPAT_CFLAGS += -DCOMMON_DIGEST_FOR_OPENSSL\n>  \tSHA1_HEADER = <CommonCrypto/CommonDigest.h>\n> +\tSHA1_MAX_BLOCK_SIZE = 1024L*1024L*1024L\n>  else\n>  \tSHA1_HEADER = <openssl/sha.h>\n>  \tEXTLIBS += $(LIB_4_CRYPTO)\n> @@ -1353,6 +1358,10 @@ endif\n>  endif\n>  endif\n>  \n> +ifdef SHA1_MAX_BLOCK_SIZE\n> +\tLIB_OBJS += compat/sha1_chunked.o\n> +\tBASIC_CFLAGS += -DSHA1_MAX_BLOCK_SIZE=\"$(SHA1_MAX_BLOCK_SIZE)\"\n> +endif\n>  ifdef NO_PERL_MAKEMAKER\n>  \texport NO_PERL_MAKEMAKER\n>  endif\n> diff --git a/cache.h b/cache.h\n> index 79066e5..ec84b16 100644\n> --- a/cache.h\n> +++ b/cache.h\n> @@ -14,7 +14,12 @@\n>  #ifndef git_SHA_CTX\n>  #define git_SHA_CTX\tSHA_CTX\n>  #define git_SHA1_Init\tSHA1_Init\n> -#define git_SHA1_Update\tSHA1_Update\n> +#ifdef SHA1_MAX_BLOCK_SIZE\n> +extern int SHA1_Update_Chunked(SHA_CTX *, const void *, size_t);\n> +#define git_SHA1_Update SHA1_Update_Chunked\n> +#else\n> +#define git_SHA1_Update SHA1_Update\n> +#endif\n>  #define git_SHA1_Final\tSHA1_Final\n>  #endif\n>  \n> diff --git a/compat/apple-common-crypto.h b/compat/apple-common-crypto.h\n> index c8b9b0e..83668fd 100644\n> --- a/compat/apple-common-crypto.h\n> +++ b/compat/apple-common-crypto.h\n> @@ -16,6 +16,10 @@\n>  #undef TYPE_BOOL\n>  #endif\n>  \n> +#ifndef SHA1_MAX_BLOCK_SIZE\n> +#error \"Using Apple Common Crypto library requires setting SHA1_MAX_BLOCK_SIZE\"\n> +#endif\n> +\n\nIt crossed my mind if this might be better to just define it to some\nreasonable value instead of erroring out, but because we do give a\ndefault value in the Makefile, it would be a sign that the user is\ndoing something _quite_ unusual if the symbol is not defined here,\nso I agree with your decision to error it out here.\n\n>  #ifdef APPLE_LION_OR_NEWER\n>  #define git_CC_error_check(pattern, err) \\\n>  \tdo { \\\n> diff --git a/compat/sha1_chunked.c b/compat/sha1_chunked.c\n> new file mode 100644\n> index 0000000..4a8e4f7\n> --- /dev/null\n> +++ b/compat/sha1_chunked.c\n> @@ -0,0 +1,20 @@\n> +#include \"cache.h\"\n> +\n> +#ifdef SHA1_MAX_BLOCK_SIZE\n> +int git_SHA1_Update(SHA_CTX *c, const void *data, size_t len)\n> +{\n> +\tsize_t nr;\n> +\tsize_t total = 0;\n> +\tchar *cdata = (char*)data;\n\nPlease have a single blank line between the decls at the beginning\nof a function and its first statement.  I am not sure about the cast\nhere, though.  Doesn't the function SHA1_Update() you are going to\ncall in the body of the loop take \"const void *\" as its second\nparameter?  That's how openssl/sha1.h and block-sha1/sha1.h declare\nthis function.\n\n> +\twhile(len > 0) {\n> +\t\tnr = len;\n> +\t\tif(nr > SHA1_MAX_BLOCK_SIZE)\n\nPlease have a SP around () for control statements, like while,\nswitch and if.\n\n> +\t\t\tnr = SHA1_MAX_BLOCK_SIZE;\n> +\t\tSHA1_Update(c, cdata, nr);\n> +\t\ttotal += nr;\n> +\t\tcdata += nr;\n> +\t\tlen -= nr;\n> +\t}\n> +\treturn total;\n> +}\n> +#endif\n\nThanks.\n"},{"id":"272591","messageId":"015b01d11362$0b8c1e70$22a45b50$@com","threadId":"40669","inReplyTo":"1446243500-21580-1-git-send-email-apahlevan@ieee.org","subject":"RE: [PATCH] Limit the size of the data block passed to SHA1_Update()","fromName":"Randall S. Becker","fromEmail":"rsbecker@nexbridge.com","sentAt":"2015-10-30T22:26:38Z","receivedAt":"2015-10-30T22:26:38Z","isPatch":true,"sender":{"key":"randall.becker@nexbridge.ca","avatar":"https://avatars.githubusercontent.com/u/28956764?v=4"},"body":"On October-30-15 6:18 PM, Atousa Pahlevan Duprat wrote:\n>Some implementations of SHA_Updates have inherent limits on the max chunk\nsize. >SHA1_MAX_BLOCK_SIZE can be defined to set the max chunk size\nsupported, if >required.  This is enabled for OSX CommonCrypto library and\nset to 1GiB.\n>---\n> <snip>\n\nDidn't we have this same issue with NonStop port about a year ago and\ndecided to provision this through the configure script?\n\nCheers,\nRandall\n"},{"id":"272624","messageId":"xmqqoaffhrfi.fsf@gitster.mtv.corp.google.com","threadId":"40669","inReplyTo":"015b01d11362$0b8c1e70$22a45b50$@com","subject":"Re: [PATCH] Limit the size of the data block passed to SHA1_Update()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-10-31T17:35:29Z","receivedAt":"2015-10-31T17:35:29Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Randall S. Becker\" <rsbecker@nexbridge.com> writes:\n\n> Didn't we have this same issue with NonStop port about a year ago and\n> decided to provision this through the configure script?\n\nI do not recall, but a support in configure.ac would be a nice\naddition.\n\nIt does not have to be a requirement for the first cut solution,\nthough.  The customization based on Makefile variables like in\nAtousa's change would serve as a foundation to add configure support\nas a follow-up change, so let's not make the scope of this topic too\nbig before we have a solid foundation to build on.\n"},{"id":"272642","messageId":"CAPig+cRRjCDhdT-DvGtZqns1mMxygnxi=ZnRKzg+H_do7oRpqQ@mail.gmail.com","threadId":"40669","inReplyTo":"1446243122-21464-1-git-send-email-apahlevan@ieee.org","subject":"Re: [PATCH] Limit the size of the data block passed to SHA1_Update()","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2015-11-01T01:32:17Z","receivedAt":"2015-11-01T01:32:17Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Fri, Oct 30, 2015 at 6:12 PM, Atousa Pahlevan Duprat\n<atousa.p@gmail.com> wrote:\n> Some implementations of SHA_Updates have inherent limits\n> on the max chunk size. SHA1_MAX_BLOCK_SIZE can be defined\n> to set the max chunk size supported, if required.  This is\n> enabled for OSX CommonCrypto library and set to 1GiB.\n> ---\n> diff --git a/compat/apple-common-crypto.h b/compat/apple-common-crypto.h\n> index c8b9b0e..83668fd 100644\n> --- a/compat/apple-common-crypto.h\n> +++ b/compat/apple-common-crypto.h\n> @@ -16,6 +16,10 @@\n>  #undef TYPE_BOOL\n>  #endif\n>\n> +#ifndef SHA1_MAX_BLOCK_SIZE\n> +#error \"Using Apple Common Crypto library requires setting SHA1_MAX_BLOCK_SIZE\"\n> +#endif\n\nYou can drop the quotes around the argument to #error.\n"},{"id":"272645","messageId":"1446359536-25829-1-git-send-email-apahlevan@ieee.org","threadId":"40669","inReplyTo":"CAPig+cRRjCDhdT-DvGtZqns1mMxygnxi=ZnRKzg+H_do7oRpqQ@mail.gmail.com","subject":"[PATCH] Limit the size of the data block passed to SHA1_Update()","fromName":"","fromEmail":"atousa.p@gmail.com","sentAt":"2015-11-01T06:32:16Z","receivedAt":"2015-11-01T06:32:16Z","isPatch":true,"sender":{"key":"atousa.p@gmail.com","avatar":"https://gravatar.com/avatar/b5b7b70553497c4ce08a835c156ff3b245366eba956fbceff99553b1ccdc4e42?d=mp&s=160"},"body":"From: Atousa Pahlevan Duprat <apahlevan@ieee.org>\n\nSome implementations of SHA_Updates have inherent limits\non the max chunk size. SHA1_MAX_BLOCK_SIZE can be defined\nto set the max chunk size supported, if required.  This is\nenabled for OSX CommonCrypto library and set to 1GiB.\n\nSigned-off-by: Atousa Pahlevan Duprat <apahlevan@ieee.org>\n---\n Makefile                     |  9 +++++++++\n cache.h                      |  7 ++++++-\n compat/apple-common-crypto.h |  4 ++++\n compat/sha1_chunked.c        | 21 +++++++++++++++++++++\n 4 files changed, 40 insertions(+), 1 deletion(-)\n create mode 100644 compat/sha1_chunked.c\n\ndiff --git a/Makefile b/Makefile\nindex 04c2231..5955542 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -141,6 +141,10 @@ all::\n # Define PPC_SHA1 environment variable when running make to make use of\n # a bundled SHA1 routine optimized for PowerPC.\n #\n+# Define SHA1_MAX_BLOCK_SIZE if your SSH1_Update() implementation can\n+# hash only a limited amount of data in one call (e.g. APPLE_COMMON_CRYPTO\n+# may want 'SHA1_MAX_BLOCK_SIZE=1024L*1024L*1024L' defined).\n+#\n # Define NEEDS_CRYPTO_WITH_SSL if you need -lcrypto when using -lssl (Darwin).\n #\n # Define NEEDS_SSL_WITH_CRYPTO if you need -lssl when using -lcrypto (Darwin).\n@@ -1346,6 +1350,7 @@ else\n ifdef APPLE_COMMON_CRYPTO\n \tCOMPAT_CFLAGS += -DCOMMON_DIGEST_FOR_OPENSSL\n \tSHA1_HEADER = <CommonCrypto/CommonDigest.h>\n+\tSHA1_MAX_BLOCK_SIZE = 1024L*1024L*1024L\n else\n \tSHA1_HEADER = <openssl/sha.h>\n \tEXTLIBS += $(LIB_4_CRYPTO)\n@@ -1353,6 +1358,10 @@ endif\n endif\n endif\n \n+ifdef SHA1_MAX_BLOCK_SIZE\n+\tLIB_OBJS += compat/sha1_chunked.o\n+\tBASIC_CFLAGS += -DSHA1_MAX_BLOCK_SIZE=\"$(SHA1_MAX_BLOCK_SIZE)\"\n+endif\n ifdef NO_PERL_MAKEMAKER\n \texport NO_PERL_MAKEMAKER\n endif\ndiff --git a/cache.h b/cache.h\nindex 79066e5..ec84b16 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -14,7 +14,12 @@\n #ifndef git_SHA_CTX\n #define git_SHA_CTX\tSHA_CTX\n #define git_SHA1_Init\tSHA1_Init\n-#define git_SHA1_Update\tSHA1_Update\n+#ifdef SHA1_MAX_BLOCK_SIZE\n+extern int SHA1_Update_Chunked(SHA_CTX *, const void *, size_t);\n+#define git_SHA1_Update SHA1_Update_Chunked\n+#else\n+#define git_SHA1_Update SHA1_Update\n+#endif\n #define git_SHA1_Final\tSHA1_Final\n #endif\n \ndiff --git a/compat/apple-common-crypto.h b/compat/apple-common-crypto.h\nindex c8b9b0e..d3fb264 100644\n--- a/compat/apple-common-crypto.h\n+++ b/compat/apple-common-crypto.h\n@@ -16,6 +16,10 @@\n #undef TYPE_BOOL\n #endif\n \n+#ifndef SHA1_MAX_BLOCK_SIZE\n+#error Using Apple Common Crypto library requires setting SHA1_MAX_BLOCK_SIZE\n+#endif\n+\n #ifdef APPLE_LION_OR_NEWER\n #define git_CC_error_check(pattern, err) \\\n \tdo { \\\ndiff --git a/compat/sha1_chunked.c b/compat/sha1_chunked.c\nnew file mode 100644\nindex 0000000..bf62b1b\n--- /dev/null\n+++ b/compat/sha1_chunked.c\n@@ -0,0 +1,21 @@\n+#include \"cache.h\"\n+\n+#ifdef SHA1_MAX_BLOCK_SIZE\n+int git_SHA1_Update(SHA_CTX *c, const void *data, size_t len)\n+{\n+\tsize_t nr;\n+\tsize_t total = 0;\n+\tchar *cdata = (char*)data;\n+\n+\twhile (len > 0) {\n+\t\tnr = len;\n+\t\tif (nr > SHA1_MAX_BLOCK_SIZE)\n+\t\t\tnr = SHA1_MAX_BLOCK_SIZE;\n+\t\tSHA1_Update(c, cdata, nr);\n+\t\ttotal += nr;\n+\t\tcdata += nr;\n+\t\tlen -= nr;\n+\t}\n+\treturn total;\n+}\n+#endif\n-- \n2.4.9 (Apple Git-60)\n"},{"id":"272646","messageId":"CA+izobt11C=0hSkW6tivjRDPfbVij9-KOuhikisXrG6PWgMAjA@mail.gmail.com","threadId":"40669","inReplyTo":"015b01d11362$0b8c1e70$22a45b50$@com","subject":"Re: [PATCH] Limit the size of the data block passed to SHA1_Update()","fromName":"Atousa Duprat","fromEmail":"atousa.p@gmail.com","sentAt":"2015-11-01T06:37:02Z","receivedAt":"2015-11-01T06:37:02Z","isPatch":true,"sender":{"key":"atousa.p@gmail.com","avatar":"https://gravatar.com/avatar/b5b7b70553497c4ce08a835c156ff3b245366eba956fbceff99553b1ccdc4e42?d=mp&s=160"},"body":"> Didn't we have this same issue with NonStop port about a year ago and\n> decided to provision this through the configure script?\n\nI'll be happy to look at it.  Can you point me to the right email\nthread or location in the configure.ac file?\n\nAtousa\n"},{"id":"272647","messageId":"CA+izobuGFGhho9X6xGdXMskay3UH6-FuzriNA7=mkveFjo0SBA@mail.gmail.com","threadId":"40669","inReplyTo":"xmqqa8r0j8tu.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] Limit the size of the data block passed to SHA1_Update()","fromName":"Atousa Duprat","fromEmail":"atousa.p@gmail.com","sentAt":"2015-11-01T06:41:17Z","receivedAt":"2015-11-01T06:41:17Z","isPatch":true,"sender":{"key":"atousa.p@gmail.com","avatar":"https://gravatar.com/avatar/b5b7b70553497c4ce08a835c156ff3b245366eba956fbceff99553b1ccdc4e42?d=mp&s=160"},"body":">> +int git_SHA1_Update(SHA_CTX *c, const void *data, size_t len)\n>> +{\n>> +     size_t nr;\n>> +     size_t total = 0;\n>> +     char *cdata = (char*)data;\n\n> I am not sure about the cast\n> here, though.  Doesn't the function SHA1_Update() you are going to\n> call in the body of the loop take \"const void *\" as its second\n> parameter?  That's how openssl/sha1.h and block-sha1/sha1.h declare\n> this function.\n\nIndeed, the declaration needs a const void *; but I need to advance by\na specific number of bytes in each iteration of the loop.  Hence the\ncast.\n\nAtousa\n"},{"id":"272651","messageId":"CAPig+cSLoC-O1YneUVRjBpZzHLmASmRQx_+8b2dsk34ReU-ovw@mail.gmail.com","threadId":"40669","inReplyTo":"1446359536-25829-1-git-send-email-apahlevan@ieee.org","subject":"Re: [PATCH] Limit the size of the data block passed to SHA1_Update()","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2015-11-01T08:30:08Z","receivedAt":"2015-11-01T08:30:08Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Sun, Nov 1, 2015 at 1:32 AM,  <atousa.p@gmail.com> wrote:\n> Some implementations of SHA_Updates have inherent limits\n> on the max chunk size. SHA1_MAX_BLOCK_SIZE can be defined\n> to set the max chunk size supported, if required.  This is\n> enabled for OSX CommonCrypto library and set to 1GiB.\n>\n> Signed-off-by: Atousa Pahlevan Duprat <apahlevan@ieee.org>\n> ---\n> diff --git a/compat/sha1_chunked.c b/compat/sha1_chunked.c\n> new file mode 100644\n> index 0000000..bf62b1b\n> --- /dev/null\n> +++ b/compat/sha1_chunked.c\n> @@ -0,0 +1,21 @@\n> +#include \"cache.h\"\n> +\n> +#ifdef SHA1_MAX_BLOCK_SIZE\n\nThis file is only compiled when SHA1_MAX_BLOCK_SIZE is defined, so\ndoes this #ifdef serve a purpose?\n\n> +int git_SHA1_Update(SHA_CTX *c, const void *data, size_t len)\n> +{\n> +       size_t nr;\n> +       size_t total = 0;\n> +       char *cdata = (char*)data;\n\nNit: This could be 'const char *'.\n\n> +       while (len > 0) {\n> +               nr = len;\n> +               if (nr > SHA1_MAX_BLOCK_SIZE)\n> +                       nr = SHA1_MAX_BLOCK_SIZE;\n> +               SHA1_Update(c, cdata, nr);\n> +               total += nr;\n> +               cdata += nr;\n> +               len -= nr;\n> +       }\n> +       return total;\n> +}\n> +#endif\n"},{"id":"272687","messageId":"xmqqlhahh8q7.fsf@gitster.mtv.corp.google.com","threadId":"40669","inReplyTo":"CA+izobuGFGhho9X6xGdXMskay3UH6-FuzriNA7=mkveFjo0SBA@mail.gmail.com","subject":"Re: [PATCH] Limit the size of the data block passed to SHA1_Update()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-11-01T18:31:44Z","receivedAt":"2015-11-01T18:31:44Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Atousa Duprat <atousa.p@gmail.com> writes:\n\n> Indeed, the declaration needs a const void *; but I need to advance by\n> a specific number of bytes in each iteration of the loop.  Hence the\n> cast.\n\nAh, of course you are right.  Silly me.\n\nThanks.\n"},{"id":"272688","messageId":"xmqqh9l5h8g3.fsf@gitster.mtv.corp.google.com","threadId":"40669","inReplyTo":"1446359536-25829-1-git-send-email-apahlevan@ieee.org","subject":"Re: [PATCH] Limit the size of the data block passed to SHA1_Update()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-11-01T18:37:48Z","receivedAt":"2015-11-01T18:37:48Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"atousa.p@gmail.com writes:\n\n> diff --git a/cache.h b/cache.h\n> index 79066e5..ec84b16 100644\n> --- a/cache.h\n> +++ b/cache.h\n> @@ -14,7 +14,12 @@\n>  #ifndef git_SHA_CTX\n>  #define git_SHA_CTX\tSHA_CTX\n>  #define git_SHA1_Init\tSHA1_Init\n> -#define git_SHA1_Update\tSHA1_Update\n> +#ifdef SHA1_MAX_BLOCK_SIZE\n> +extern int SHA1_Update_Chunked(SHA_CTX *, const void *, size_t);\n> +#define git_SHA1_Update SHA1_Update_Chunked\n> +#else\n> +#define git_SHA1_Update SHA1_Update\n> +#endif\n>  #define git_SHA1_Final\tSHA1_Final\n>  #endif\n\nHmm, I admit that this mess is my creation, but unfortunately it\ndoes not allow us to say:\n\n\tmake SHA1_MAX_BLOCK_SIZE='1024L*1024L*1024L'\n\nwhen using other SHA-1 implementations (e.g. blk_SHA1_Update()).\n\nIdeas for cleaning it up, anybody?\n"},{"id":"272754","messageId":"CA+izobvbDYLvShT8TdDhe9UiYHVWw+Le+Yy4yOnvCYOWE0bhQQ@mail.gmail.com","threadId":"40669","inReplyTo":"xmqqh9l5h8g3.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] Limit the size of the data block passed to SHA1_Update()","fromName":"Atousa Duprat","fromEmail":"atousa.p@gmail.com","sentAt":"2015-11-02T20:52:34Z","receivedAt":"2015-11-02T20:52:34Z","isPatch":true,"sender":{"key":"atousa.p@gmail.com","avatar":"https://gravatar.com/avatar/b5b7b70553497c4ce08a835c156ff3b245366eba956fbceff99553b1ccdc4e42?d=mp&s=160"},"body":"In the Makefile there is the following:\n\nifdef BLK_SHA1\n        SHA1_HEADER = \"block-sha1/sha1.h\"\n        LIB_OBJS += block-sha1/sha1.o\nelse\nifdef PPC_SHA1\n        SHA1_HEADER = \"ppc/sha1.h\"\n        LIB_OBJS += ppc/sha1.o ppc/sha1ppc.o\nelse\nifdef APPLE_COMMON_CRYPTO\n        COMPAT_CFLAGS += -DCOMMON_DIGEST_FOR_OPENSSL\n        SHA1_HEADER = <CommonCrypto/CommonDigest.h>\n        SHA1_MAX_BLOCK_SIZE = 1024L*1024L*1024L\nelse\n        SHA1_HEADER = <openssl/sha.h>\n        EXTLIBS += $(LIB_4_CRYPTO)\nendif\n\nwhich seems to imply that BLK_SHA1 and APPLE_COMMON_CRYPTO are\nmutually exclusive?\n\nOn Sun, Nov 1, 2015 at 10:37 AM, Junio C Hamano <gitster@pobox.com> wrote:\n> atousa.p@gmail.com writes:\n>\n>> diff --git a/cache.h b/cache.h\n>> index 79066e5..ec84b16 100644\n>> --- a/cache.h\n>> +++ b/cache.h\n>> @@ -14,7 +14,12 @@\n>>  #ifndef git_SHA_CTX\n>>  #define git_SHA_CTX  SHA_CTX\n>>  #define git_SHA1_Init        SHA1_Init\n>> -#define git_SHA1_Update      SHA1_Update\n>> +#ifdef SHA1_MAX_BLOCK_SIZE\n>> +extern int SHA1_Update_Chunked(SHA_CTX *, const void *, size_t);\n>> +#define git_SHA1_Update SHA1_Update_Chunked\n>> +#else\n>> +#define git_SHA1_Update SHA1_Update\n>> +#endif\n>>  #define git_SHA1_Final       SHA1_Final\n>>  #endif\n>\n> Hmm, I admit that this mess is my creation, but unfortunately it\n> does not allow us to say:\n>\n>         make SHA1_MAX_BLOCK_SIZE='1024L*1024L*1024L'\n>\n> when using other SHA-1 implementations (e.g. blk_SHA1_Update()).\n>\n> Ideas for cleaning it up, anybody?\n>\n\n\n\n-- \nAtousa Pahlevan, PhD\nM.Math. University of Waterloo, Canada\nPh.D. Department of Computer Science, University of Victoria, Canada\nVoice: 415-341-6206\nEmail: apahlevan@ieee.org\nWebsite: www.apahlevan.org\n"},{"id":"272759","messageId":"xmqqpozsdrnl.fsf@gitster.mtv.corp.google.com","threadId":"40669","inReplyTo":"CA+izobvbDYLvShT8TdDhe9UiYHVWw+Le+Yy4yOnvCYOWE0bhQQ@mail.gmail.com","subject":"Re: [PATCH] Limit the size of the data block passed to SHA1_Update()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-11-02T21:21:02Z","receivedAt":"2015-11-02T21:21:02Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Atousa Duprat <atousa.p@gmail.com> writes:\n\n> On Sun, Nov 1, 2015 at 10:37 AM, Junio C Hamano <gitster@pobox.com> wrote:\n>>\n>> Hmm, I admit that this mess is my creation, but unfortunately it\n>> does not allow us to say:\n>>\n>>         make SHA1_MAX_BLOCK_SIZE='1024L*1024L*1024L'\n>>\n>> when using other SHA-1 implementations (e.g. blk_SHA1_Update()).\n>>\n>> Ideas for cleaning it up, anybody?\n>>\n> In the Makefile there is the following:\n>\n> ifdef BLK_SHA1\n>         SHA1_HEADER = \"block-sha1/sha1.h\"\n>         LIB_OBJS += block-sha1/sha1.o\n> else\n> ifdef PPC_SHA1\n>         SHA1_HEADER = \"ppc/sha1.h\"\n>         LIB_OBJS += ppc/sha1.o ppc/sha1ppc.o\n> else\n> ifdef APPLE_COMMON_CRYPTO\n>         COMPAT_CFLAGS += -DCOMMON_DIGEST_FOR_OPENSSL\n>         SHA1_HEADER = <CommonCrypto/CommonDigest.h>\n>         SHA1_MAX_BLOCK_SIZE = 1024L*1024L*1024L\n> else\n>         SHA1_HEADER = <openssl/sha.h>\n>         EXTLIBS += $(LIB_4_CRYPTO)\n> endif\n>\n> which seems to imply that BLK_SHA1 and APPLE_COMMON_CRYPTO are\n> mutually exclusive?\n\nYes, you are correct that these two cannot be used at the same time.\nIn general (not limited to BLK_SHA1 and APPLE_COMMON_CRYPTO) you can\npick only _one_ underlying SHA-1 implementation to use with the\nsystem.\n\nIf you use APPLE_COMMON_CRYPTO, you may have to use the Chunked\nthing, because of APPLE_COMMON_CRYPTO implementation's limitation.\n\nBut the above two facts taken together does not have to imply that\nyou are forbidden from choosing to use Chunked thing if you are\nusing BLK_SHA1 or OpenSSL's SHA-1 implementation.  If only for\nmaking sure that the Chunked wrapper passes compilation test, for\ntrying it out to see how well it works, or just for satisfying\ncuriosity, it would be nice if we allowed such a combination.\n\nThe original arrangement of macro was:\n\n * The user code uses git_SHA1_Update()\n\n * cache.h renames git_SHA1_Update() to refer to the underlying\n   SHA1_Update() function, either from OpenSSL or AppleCommonCrypto,\n   or block-sha1/sha1.h renames git_SHA1_Update() to refer to our\n   implementation blk_SHA1_Update().\n\nWhat we want with Chunked is:\n\n * The user code uses git_SHA1_Update(); we must not change this, as\n   there are many existing calls.\n\n * We want git_SHA1_Update() to call the Chunked thing when\n   SHA1_MAX_BLOCK_SIZE is set.\n\n * The Chunked thing must delegate the actual hashing to underlying\n   SHA1_Update(), either from OpenSSL or AppleCommonCrypto.  If we\n   are using BLK_SHA1, we want the Chunked thing to instead call\n   blk_SHA1_Update().\n\nI do not seem to be able to find a way to do this with the current\ntwo-level indirection.  If we added another level, we can.\n\n* In cache.h, define platform_SHA1_Update() to refer to\n  SHA1_Update() from the platform (unless block-sha1/ is used).\n  git_SHA1_Update() in the user code may directly call it, or it may\n  go to the Chunked thing.\n\n  #ifndef git_SHA1_CTX\n  #define platform_SHA1_Update  SHA1_Update\n  #endif\n\n  #ifdef SHA1_MAX_BLOCK_SIZE\n  #define git_SHA1_Update       git_SHA1_Update_Chunked\n  #else\n  #define git_SHA1_Update       platform_SHA1_Update\n  #endif\n\n* In block-sha1/sha1.h, redirect platform_SHA1_Update() to\n  blk_SHA1_Update().\n\n  #define platform_SHA1_Update  blk_SHA1_Update\n\n* In compat/sha1_chunked.c, implement the Chunked thing in terms of\n  the platform_SHA1_Update():\n\n  git_SHA1_Update_Chunked(...)\n  {\n        ...\n        while (...) {\n                platform_SHA1_Update(...);\n        }\n  }\n\n\nI am not sure if the above is worth it, but I suspect the damage is\nlocalized enough that this may be OK.\n"},{"id":"272787","messageId":"1446533930-463-1-git-send-email-apahlevan@ieee.org","threadId":"40669","inReplyTo":"xmqqpozsdrnl.fsf@gitster.mtv.corp.google.com","subject":"[PATCH 1/2] Limit the size of the data block passed to SHA1_Update()","fromName":"","fromEmail":"atousa.p@gmail.com","sentAt":"2015-11-03T06:58:50Z","receivedAt":"2015-11-03T06:58:50Z","isPatch":true,"sender":{"key":"atousa.p@gmail.com","avatar":"https://gravatar.com/avatar/b5b7b70553497c4ce08a835c156ff3b245366eba956fbceff99553b1ccdc4e42?d=mp&s=160"},"body":"From: Atousa Pahlevan Duprat <apahlevan@ieee.org>\n\nSome implementations of SHA_Updates have inherent limits\non the max chunk size. SHA1_MAX_BLOCK_SIZE can be defined\nto set the max chunk size supported, if required.  This is\nenabled for OSX CommonCrypto library and set to 1GiB.\n\nSigned-off-by: Atousa Pahlevan Duprat <apahlevan@ieee.org>\n---\n Makefile                     | 16 +++++++++++++++-\n block-sha1/sha1.h            |  2 +-\n cache.h                      | 17 +++++++++++++----\n compat/apple-common-crypto.h |  4 ++++\n compat/sha1_chunked.c        | 19 +++++++++++++++++++\n 5 files changed, 52 insertions(+), 6 deletions(-)\n create mode 100644 compat/sha1_chunked.c\n\ndiff --git a/Makefile b/Makefile\nindex 04c2231..1b098cc 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -136,11 +136,15 @@ all::\n # to provide your own OpenSSL library, for example from MacPorts.\n #\n # Define BLK_SHA1 environment variable to make use of the bundled\n-# optimized C SHA1 routine.\n+# optimized C SHA1 routine.  This implies NO_APPLE_COMMON_CRYPTO.\n #\n # Define PPC_SHA1 environment variable when running make to make use of\n # a bundled SHA1 routine optimized for PowerPC.\n #\n+# Define SHA1_MAX_BLOCK_SIZE if your SSH1_Update() implementation can\n+# hash only a limited amount of data in one call (e.g. APPLE_COMMON_CRYPTO\n+# may want 'SHA1_MAX_BLOCK_SIZE=1024L*1024L*1024L' defined).\n+#\n # Define NEEDS_CRYPTO_WITH_SSL if you need -lcrypto when using -lssl (Darwin).\n #\n # Define NEEDS_SSL_WITH_CRYPTO if you need -lssl when using -lcrypto (Darwin).\n@@ -986,6 +990,10 @@ ifeq (no,$(USE_PARENS_AROUND_GETTEXT_N))\n endif\n endif\n \n+ifdef BLK_SHA1\n+\tNO_APPLE_COMMON_CRYPTO=1\n+endif\n+\n ifeq ($(uname_S),Darwin)\n \tifndef NO_FINK\n \t\tifeq ($(shell test -d /sw/lib && echo y),y)\n@@ -1346,6 +1354,8 @@ else\n ifdef APPLE_COMMON_CRYPTO\n \tCOMPAT_CFLAGS += -DCOMMON_DIGEST_FOR_OPENSSL\n \tSHA1_HEADER = <CommonCrypto/CommonDigest.h>\n+\t# Apple CommonCrypto requires chunking\n+\tSHA1_MAX_BLOCK_SIZE = 1024L*1024L*1024L\n else\n \tSHA1_HEADER = <openssl/sha.h>\n \tEXTLIBS += $(LIB_4_CRYPTO)\n@@ -1353,6 +1363,10 @@ endif\n endif\n endif\n \n+ifdef SHA1_MAX_BLOCK_SIZE\n+\tLIB_OBJS += compat/sha1_chunked.o\n+\tBASIC_CFLAGS += -DSHA1_MAX_BLOCK_SIZE=\"$(SHA1_MAX_BLOCK_SIZE)\"\n+endif\n ifdef NO_PERL_MAKEMAKER\n \texport NO_PERL_MAKEMAKER\n endif\ndiff --git a/block-sha1/sha1.h b/block-sha1/sha1.h\nindex b864df6..d085412 100644\n--- a/block-sha1/sha1.h\n+++ b/block-sha1/sha1.h\n@@ -18,5 +18,5 @@ void blk_SHA1_Final(unsigned char hashout[20], blk_SHA_CTX *ctx);\n \n #define git_SHA_CTX\tblk_SHA_CTX\n #define git_SHA1_Init\tblk_SHA1_Init\n-#define git_SHA1_Update\tblk_SHA1_Update\n+#define platform_SHA1_Update\tblk_SHA1_Update\n #define git_SHA1_Final\tblk_SHA1_Final\ndiff --git a/cache.h b/cache.h\nindex 79066e5..a501652 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -10,12 +10,21 @@\n #include \"trace.h\"\n #include \"string-list.h\"\n \n+// platform's underlying implementation of SHA1\n #include SHA1_HEADER\n #ifndef git_SHA_CTX\n-#define git_SHA_CTX\tSHA_CTX\n-#define git_SHA1_Init\tSHA1_Init\n-#define git_SHA1_Update\tSHA1_Update\n-#define git_SHA1_Final\tSHA1_Final\n+#define git_SHA_CTX\t\tSHA_CTX\n+#define git_SHA1_Init\t\tSHA1_Init\n+#define platform_SHA1_Update\tSHA1_Update\n+#define git_SHA1_Final\t\tSHA1_Final\n+#endif\n+\n+// choose whether chunked implementation or not\n+#ifdef SHA1_MAX_BLOCK_SIZE\n+int git_SHA1_Update_Chunked(SHA_CTX *c, const void *data, size_t len);\n+#define git_SHA1_Update       git_SHA1_Update_Chunked\n+#else\n+#define git_SHA1_Update       platform_SHA1_Update\n #endif\n \n #include <zlib.h>\ndiff --git a/compat/apple-common-crypto.h b/compat/apple-common-crypto.h\nindex c8b9b0e..d3fb264 100644\n--- a/compat/apple-common-crypto.h\n+++ b/compat/apple-common-crypto.h\n@@ -16,6 +16,10 @@\n #undef TYPE_BOOL\n #endif\n \n+#ifndef SHA1_MAX_BLOCK_SIZE\n+#error Using Apple Common Crypto library requires setting SHA1_MAX_BLOCK_SIZE\n+#endif\n+\n #ifdef APPLE_LION_OR_NEWER\n #define git_CC_error_check(pattern, err) \\\n \tdo { \\\ndiff --git a/compat/sha1_chunked.c b/compat/sha1_chunked.c\nnew file mode 100644\nindex 0000000..61f67de\n--- /dev/null\n+++ b/compat/sha1_chunked.c\n@@ -0,0 +1,19 @@\n+#include \"cache.h\"\n+\n+int git_SHA1_Update_Chunked(SHA_CTX *c, const void *data, size_t len)\n+{\n+\tsize_t nr;\n+\tsize_t total = 0;\n+\tconst char *cdata = (const char*)data;\n+\n+\twhile (len > 0) {\n+\t\tnr = len;\n+\t\tif (nr > SHA1_MAX_BLOCK_SIZE)\n+\t\t\tnr = SHA1_MAX_BLOCK_SIZE;\n+\t\tplatform_SHA1_Update(c, cdata, nr);\n+\t\ttotal += nr;\n+\t\tcdata += nr;\n+\t\tlen -= nr;\n+\t}\n+\treturn total;\n+}\n-- \n2.4.9 (Apple Git-60)\n"},{"id":"272827","messageId":"56389FBC.7050909@web.de","threadId":"40669","inReplyTo":"1446533930-463-1-git-send-email-apahlevan@ieee.org","subject":"Re: [PATCH 1/2] Limit the size of the data block passed to SHA1_Update()","fromName":"Torsten Bögershausen","fromEmail":"tboegi@web.de","sentAt":"2015-11-03T11:51:24Z","receivedAt":"2015-11-03T11:51:24Z","isPatch":true,"sender":{"key":"tboegi@web.de","avatar":"https://avatars.githubusercontent.com/u/7138363?v=4"},"body":"On 11/03/2015 07:58 AM, atousa.p@gmail.com wrote:\n> From: Atousa Pahlevan Duprat <apahlevan@ieee.org>\nMinor comments inline\n> diff --git a/block-sha1/sha1.h b/block-sha1/sha1.h\n> index b864df6..d085412 100644\n> --- a/block-sha1/sha1.h\n> +++ b/block-sha1/sha1.h\n> @@ -18,5 +18,5 @@ void blk_SHA1_Final(unsigned char hashout[20], blk_SHA_CTX *ctx);\n>   \n>   #define git_SHA_CTX\tblk_SHA_CTX\n>   #define git_SHA1_Init\tblk_SHA1_Init\n> -#define git_SHA1_Update\tblk_SHA1_Update\n> +#define platform_SHA1_Update\tblk_SHA1_Update\n>   #define git_SHA1_Final\tblk_SHA1_Final\n> diff --git a/cache.h b/cache.h\n> index 79066e5..a501652 100644\n> --- a/cache.h\n> +++ b/cache.h\n> @@ -10,12 +10,21 @@\n>   #include \"trace.h\"\n>   #include \"string-list.h\"\n>   \n> +// platform's underlying implementation of SHA1\nPlease use /* */ for comments\n>   #include SHA1_HEADER\n>   #ifndef git_SHA_CTX\n> -#define git_SHA_CTX\tSHA_CTX\n> -#define git_SHA1_Init\tSHA1_Init\n> -#define git_SHA1_Update\tSHA1_Update\n> -#define git_SHA1_Final\tSHA1_Final\n> +#define git_SHA_CTX\t\tSHA_CTX\n> +#define git_SHA1_Init\t\tSHA1_Init\n> +#define platform_SHA1_Update\tSHA1_Update\n> +#define git_SHA1_Final\t\tSHA1_Final\n> +#endif\n> +\n> +// choose whether chunked implementation or not\n> +#ifdef SHA1_MAX_BLOCK_SIZE\n> +int git_SHA1_Update_Chunked(SHA_CTX *c, const void *data, size_t len);\n> +#define git_SHA1_Update       git_SHA1_Update_Chunked\n> +#else\n> +#define git_SHA1_Update       platform_SHA1_Update\n>   #endif\n>   \n>   #include <zlib.h>\n> diff --git a/compat/apple-common-crypto.h b/compat/apple-common-crypto.h\n> index c8b9b0e..d3fb264 100644\n> --- a/compat/apple-common-crypto.h\n> +++ b/compat/apple-common-crypto.h\n> @@ -16,6 +16,10 @@\n>   #undef TYPE_BOOL\n>   #endif\n>   \n> +#ifndef SHA1_MAX_BLOCK_SIZE\n> +#error Using Apple Common Crypto library requires setting SHA1_MAX_BLOCK_SIZE\n> +#endif\n> +\n>   #ifdef APPLE_LION_OR_NEWER\n>   #define git_CC_error_check(pattern, err) \\\n>   \tdo { \\\n> diff --git a/compat/sha1_chunked.c b/compat/sha1_chunked.c\n> new file mode 100644\n> index 0000000..61f67de\n> --- /dev/null\n> +++ b/compat/sha1_chunked.c\n> @@ -0,0 +1,19 @@\n> +#include \"cache.h\"\n> +\n> +int git_SHA1_Update_Chunked(SHA_CTX *c, const void *data, size_t len)\n> +{\n> +\tsize_t nr;\n> +\tsize_t total = 0;\n> +\tconst char *cdata = (const char*)data;\n> +\n> +\twhile (len > 0) {\nsize_t is unsigned, isn't it ?\nBetter to use  \"while (len) {\"\n> +\t\tnr = len;\n> +\t\tif (nr > SHA1_MAX_BLOCK_SIZE)\n> +\t\t\tnr = SHA1_MAX_BLOCK_SIZE;\n> +\t\tplatform_SHA1_Update(c, cdata, nr);\n> +\t\ttotal += nr;\n> +\t\tcdata += nr;\n> +\t\tlen -= nr;\n> +\t}\n> +\treturn total;\n> +}\n"},{"id":"272882","messageId":"1446611086-6102-1-git-send-email-apahlevan@ieee.org","threadId":"40669","inReplyTo":"56389FBC.7050909@web.de","subject":"[PATCH] Limit the size of the data block passed to SHA1_Update()","fromName":"","fromEmail":"atousa.p@gmail.com","sentAt":"2015-11-04T04:24:46Z","receivedAt":"2015-11-04T04:24:46Z","isPatch":true,"sender":{"key":"atousa.p@gmail.com","avatar":"https://gravatar.com/avatar/b5b7b70553497c4ce08a835c156ff3b245366eba956fbceff99553b1ccdc4e42?d=mp&s=160"},"body":"From: Atousa Pahlevan Duprat <apahlevan@ieee.org>\n\nSome implementations of SHA_Updates have inherent limits\non the max chunk size. SHA1_MAX_BLOCK_SIZE can be defined\nto set the max chunk size supported, if required.  This is\nenabled for OSX CommonCrypto library and set to 1GiB.\n\nSigned-off-by: Atousa Pahlevan Duprat <apahlevan@ieee.org>\n---\n Makefile                     | 16 +++++++++++++++-\n block-sha1/sha1.h            |  2 +-\n cache.h                      | 17 +++++++++++++----\n compat/apple-common-crypto.h |  4 ++++\n compat/sha1_chunked.c        | 19 +++++++++++++++++++\n 5 files changed, 52 insertions(+), 6 deletions(-)\n create mode 100644 compat/sha1_chunked.c\n\ndiff --git a/Makefile b/Makefile\nindex 04c2231..1b098cc 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -136,11 +136,15 @@ all::\n # to provide your own OpenSSL library, for example from MacPorts.\n #\n # Define BLK_SHA1 environment variable to make use of the bundled\n-# optimized C SHA1 routine.\n+# optimized C SHA1 routine.  This implies NO_APPLE_COMMON_CRYPTO.\n #\n # Define PPC_SHA1 environment variable when running make to make use of\n # a bundled SHA1 routine optimized for PowerPC.\n #\n+# Define SHA1_MAX_BLOCK_SIZE if your SSH1_Update() implementation can\n+# hash only a limited amount of data in one call (e.g. APPLE_COMMON_CRYPTO\n+# may want 'SHA1_MAX_BLOCK_SIZE=1024L*1024L*1024L' defined).\n+#\n # Define NEEDS_CRYPTO_WITH_SSL if you need -lcrypto when using -lssl (Darwin).\n #\n # Define NEEDS_SSL_WITH_CRYPTO if you need -lssl when using -lcrypto (Darwin).\n@@ -986,6 +990,10 @@ ifeq (no,$(USE_PARENS_AROUND_GETTEXT_N))\n endif\n endif\n \n+ifdef BLK_SHA1\n+\tNO_APPLE_COMMON_CRYPTO=1\n+endif\n+\n ifeq ($(uname_S),Darwin)\n \tifndef NO_FINK\n \t\tifeq ($(shell test -d /sw/lib && echo y),y)\n@@ -1346,6 +1354,8 @@ else\n ifdef APPLE_COMMON_CRYPTO\n \tCOMPAT_CFLAGS += -DCOMMON_DIGEST_FOR_OPENSSL\n \tSHA1_HEADER = <CommonCrypto/CommonDigest.h>\n+\t# Apple CommonCrypto requires chunking\n+\tSHA1_MAX_BLOCK_SIZE = 1024L*1024L*1024L\n else\n \tSHA1_HEADER = <openssl/sha.h>\n \tEXTLIBS += $(LIB_4_CRYPTO)\n@@ -1353,6 +1363,10 @@ endif\n endif\n endif\n \n+ifdef SHA1_MAX_BLOCK_SIZE\n+\tLIB_OBJS += compat/sha1_chunked.o\n+\tBASIC_CFLAGS += -DSHA1_MAX_BLOCK_SIZE=\"$(SHA1_MAX_BLOCK_SIZE)\"\n+endif\n ifdef NO_PERL_MAKEMAKER\n \texport NO_PERL_MAKEMAKER\n endif\ndiff --git a/block-sha1/sha1.h b/block-sha1/sha1.h\nindex b864df6..d085412 100644\n--- a/block-sha1/sha1.h\n+++ b/block-sha1/sha1.h\n@@ -18,5 +18,5 @@ void blk_SHA1_Final(unsigned char hashout[20], blk_SHA_CTX *ctx);\n \n #define git_SHA_CTX\tblk_SHA_CTX\n #define git_SHA1_Init\tblk_SHA1_Init\n-#define git_SHA1_Update\tblk_SHA1_Update\n+#define platform_SHA1_Update\tblk_SHA1_Update\n #define git_SHA1_Final\tblk_SHA1_Final\ndiff --git a/cache.h b/cache.h\nindex 79066e5..e345e38 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -10,12 +10,21 @@\n #include \"trace.h\"\n #include \"string-list.h\"\n \n+/* platform's underlying implementation of SHA1 */\n #include SHA1_HEADER\n #ifndef git_SHA_CTX\n-#define git_SHA_CTX\tSHA_CTX\n-#define git_SHA1_Init\tSHA1_Init\n-#define git_SHA1_Update\tSHA1_Update\n-#define git_SHA1_Final\tSHA1_Final\n+#define git_SHA_CTX\t\tSHA_CTX\n+#define git_SHA1_Init\t\tSHA1_Init\n+#define platform_SHA1_Update\tSHA1_Update\n+#define git_SHA1_Final\t\tSHA1_Final\n+#endif\n+\n+/* choose chunked implementation or not */\n+#ifdef SHA1_MAX_BLOCK_SIZE\n+int git_SHA1_Update_Chunked(SHA_CTX *c, const void *data, size_t len);\n+#define git_SHA1_Update       git_SHA1_Update_Chunked\n+#else\n+#define git_SHA1_Update       platform_SHA1_Update\n #endif\n \n #include <zlib.h>\ndiff --git a/compat/apple-common-crypto.h b/compat/apple-common-crypto.h\nindex c8b9b0e..d3fb264 100644\n--- a/compat/apple-common-crypto.h\n+++ b/compat/apple-common-crypto.h\n@@ -16,6 +16,10 @@\n #undef TYPE_BOOL\n #endif\n \n+#ifndef SHA1_MAX_BLOCK_SIZE\n+#error Using Apple Common Crypto library requires setting SHA1_MAX_BLOCK_SIZE\n+#endif\n+\n #ifdef APPLE_LION_OR_NEWER\n #define git_CC_error_check(pattern, err) \\\n \tdo { \\\ndiff --git a/compat/sha1_chunked.c b/compat/sha1_chunked.c\nnew file mode 100644\nindex 0000000..6d0062b\n--- /dev/null\n+++ b/compat/sha1_chunked.c\n@@ -0,0 +1,19 @@\n+#include \"cache.h\"\n+\n+int git_SHA1_Update_Chunked(SHA_CTX *c, const void *data, size_t len)\n+{\n+\tsize_t nr;\n+\tsize_t total = 0;\n+\tconst char *cdata = (const char*)data;\n+\n+\twhile (len) {\n+\t\tnr = len;\n+\t\tif (nr > SHA1_MAX_BLOCK_SIZE)\n+\t\t\tnr = SHA1_MAX_BLOCK_SIZE;\n+\t\tplatform_SHA1_Update(c, cdata, nr);\n+\t\ttotal += nr;\n+\t\tcdata += nr;\n+\t\tlen -= nr;\n+\t}\n+\treturn total;\n+}\n-- \n2.4.9 (Apple Git-60)\n"},{"id":"272883","messageId":"CA+izobtfk_k9im1gy7MCOt4p+99PD4A2iP93haRGDzerugr8Yw@mail.gmail.com","threadId":"40669","inReplyTo":"56389FBC.7050909@web.de","subject":"Re: [PATCH 1/2] Limit the size of the data block passed to SHA1_Update()","fromName":"Atousa Duprat","fromEmail":"atousa.p@gmail.com","sentAt":"2015-11-04T04:27:31Z","receivedAt":"2015-11-04T04:27:31Z","isPatch":true,"sender":{"key":"atousa.p@gmail.com","avatar":"https://gravatar.com/avatar/b5b7b70553497c4ce08a835c156ff3b245366eba956fbceff99553b1ccdc4e42?d=mp&s=160"},"body":"Thank you for the feedback. The patch is updated as suggested.\n\n\nOn Tue, Nov 3, 2015 at 3:51 AM, Torsten Bögershausen <tboegi@web.de> wrote:\n> On 11/03/2015 07:58 AM, atousa.p@gmail.com wrote:\n>>\n>> From: Atousa Pahlevan Duprat <apahlevan@ieee.org>\n>\n> Minor comments inline\n>>\n>> diff --git a/block-sha1/sha1.h b/block-sha1/sha1.h\n>> index b864df6..d085412 100644\n>> --- a/block-sha1/sha1.h\n>> +++ b/block-sha1/sha1.h\n>> @@ -18,5 +18,5 @@ void blk_SHA1_Final(unsigned char hashout[20],\n>> blk_SHA_CTX *ctx);\n>>     #define git_SHA_CTX blk_SHA_CTX\n>>   #define git_SHA1_Init blk_SHA1_Init\n>> -#define git_SHA1_Update        blk_SHA1_Update\n>> +#define platform_SHA1_Update   blk_SHA1_Update\n>>   #define git_SHA1_Final        blk_SHA1_Final\n>> diff --git a/cache.h b/cache.h\n>> index 79066e5..a501652 100644\n>> --- a/cache.h\n>> +++ b/cache.h\n>> @@ -10,12 +10,21 @@\n>>   #include \"trace.h\"\n>>   #include \"string-list.h\"\n>>   +// platform's underlying implementation of SHA1\n>\n> Please use /* */ for comments\n>\n>>   #include SHA1_HEADER\n>>   #ifndef git_SHA_CTX\n>> -#define git_SHA_CTX    SHA_CTX\n>> -#define git_SHA1_Init  SHA1_Init\n>> -#define git_SHA1_Update        SHA1_Update\n>> -#define git_SHA1_Final SHA1_Final\n>> +#define git_SHA_CTX            SHA_CTX\n>> +#define git_SHA1_Init          SHA1_Init\n>> +#define platform_SHA1_Update   SHA1_Update\n>> +#define git_SHA1_Final         SHA1_Final\n>> +#endif\n>> +\n>> +// choose whether chunked implementation or not\n>> +#ifdef SHA1_MAX_BLOCK_SIZE\n>> +int git_SHA1_Update_Chunked(SHA_CTX *c, const void *data, size_t len);\n>> +#define git_SHA1_Update       git_SHA1_Update_Chunked\n>> +#else\n>> +#define git_SHA1_Update       platform_SHA1_Update\n>>   #endif\n>>     #include <zlib.h>\n>> diff --git a/compat/apple-common-crypto.h b/compat/apple-common-crypto.h\n>> index c8b9b0e..d3fb264 100644\n>> --- a/compat/apple-common-crypto.h\n>> +++ b/compat/apple-common-crypto.h\n>> @@ -16,6 +16,10 @@\n>>   #undef TYPE_BOOL\n>>   #endif\n>>   +#ifndef SHA1_MAX_BLOCK_SIZE\n>> +#error Using Apple Common Crypto library requires setting\n>> SHA1_MAX_BLOCK_SIZE\n>> +#endif\n>> +\n>>   #ifdef APPLE_LION_OR_NEWER\n>>   #define git_CC_error_check(pattern, err) \\\n>>         do { \\\n>> diff --git a/compat/sha1_chunked.c b/compat/sha1_chunked.c\n>> new file mode 100644\n>> index 0000000..61f67de\n>> --- /dev/null\n>> +++ b/compat/sha1_chunked.c\n>> @@ -0,0 +1,19 @@\n>> +#include \"cache.h\"\n>> +\n>> +int git_SHA1_Update_Chunked(SHA_CTX *c, const void *data, size_t len)\n>> +{\n>> +       size_t nr;\n>> +       size_t total = 0;\n>> +       const char *cdata = (const char*)data;\n>> +\n>> +       while (len > 0) {\n>\n> size_t is unsigned, isn't it ?\n> Better to use  \"while (len) {\"\n>\n>> +               nr = len;\n>> +               if (nr > SHA1_MAX_BLOCK_SIZE)\n>> +                       nr = SHA1_MAX_BLOCK_SIZE;\n>> +               platform_SHA1_Update(c, cdata, nr);\n>> +               total += nr;\n>> +               cdata += nr;\n>> +               len -= nr;\n>> +       }\n>> +       return total;\n>> +}\n>\n>\n\n\n\n-- \nAtousa Pahlevan, PhD\nM.Math. University of Waterloo, Canada\nPh.D. Department of Computer Science, University of Victoria, Canada\nVoice: 415-341-6206\nEmail: apahlevan@ieee.org\nWebsite: www.apahlevan.org\n"},{"id":"272893","messageId":"xmqqsi4lbsk0.fsf@gitster.mtv.corp.google.com","threadId":"40669","inReplyTo":"xmqqpozsdrnl.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] Limit the size of the data block passed to SHA1_Update()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-11-04T17:09:03Z","receivedAt":"2015-11-04T17:09:03Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n>> ifdef BLK_SHA1\n>>         SHA1_HEADER = \"block-sha1/sha1.h\"\n>>         LIB_OBJS += block-sha1/sha1.o\n>> else\n>> ifdef PPC_SHA1\n>>         SHA1_HEADER = \"ppc/sha1.h\"\n>>         LIB_OBJS += ppc/sha1.o ppc/sha1ppc.o\n>> else\n>> ifdef APPLE_COMMON_CRYPTO\n>>         COMPAT_CFLAGS += -DCOMMON_DIGEST_FOR_OPENSSL\n>>         SHA1_HEADER = <CommonCrypto/CommonDigest.h>\n>>         SHA1_MAX_BLOCK_SIZE = 1024L*1024L*1024L\n>> else\n>>         SHA1_HEADER = <openssl/sha.h>\n>>         EXTLIBS += $(LIB_4_CRYPTO)\n>> endif\n>>\n>> which seems to imply that BLK_SHA1 and APPLE_COMMON_CRYPTO are\n>> mutually exclusive?\n>\n> Yes, you are correct that these two cannot be used at the same time.\n> In general (not limited to BLK_SHA1 and APPLE_COMMON_CRYPTO) you can\n> pick only _one_ underlying SHA-1 implementation to use with the\n> system.\n\nOur \"seems to imply\" above is a faulty reading.  The four lines I\nwrote are not incorrect per-se, but this exchange was misleading.\n\nWhen BLK_SHA1 is defined, the above fragment from the Makefile is\nonly saying \"CommonCrypto may or may not be used for any other\npurposes, but for SHA-1 hashing, I'll use our own block-sha1/\nimplementation.\"  It does not say anything about how the system\nfavours CommonCrypto over OpenSSL with APPLE_COMMON_CRYPTO.\n\nAnd it is legit to define both APPLE_COMMON_CRYPTO and BLK_SHA1; the\nresulting build would still use SSL-related functions what other\npeople may use from OpenSSL from CommonCrypto.  Filipe Cabecinhas\npointed out that such a configuration works (and solves the issue\nthat triggered this thread) very early in the thread.\n\nI haven't looked carefully at the latest version of your patch, but\nI just wanted to make sure that I didn't mislead you to add an\nunnecessary \"we check if both APPLE_COMMON_CRYPTO and BLK_SHA1 are\ndefined and error out because they are incompatible\" check.  Sorry\nfor an earlier message that may have been confusing.\n\nThanks.\n"},{"id":"272905","messageId":"CAPig+cS7NktyhveQUiBFkRtJH1D-Aif861e9qshL2R=ZNg2+Lw@mail.gmail.com","threadId":"40669","inReplyTo":"1446611086-6102-1-git-send-email-apahlevan@ieee.org","subject":"Re: [PATCH] Limit the size of the data block passed to SHA1_Update()","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2015-11-04T19:51:04Z","receivedAt":"2015-11-04T19:51:04Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Tuesday, November 3, 2015, <atousa.p@gmail.com> wrote:\n> [PATCH] Limit the size of the data block passed to SHA1_Update()\n\nAs an aid for reviewers, please include the version number of the\npatch, such as [PATCH vN] where \"N\" is the revision. format-patch's -v\noption can help automate this.\n\n> Some implementations of SHA_Updates have inherent limits\n> on the max chunk size. SHA1_MAX_BLOCK_SIZE can be defined\n> to set the max chunk size supported, if required.  This is\n> enabled for OSX CommonCrypto library and set to 1GiB.\n>\n> Signed-off-by: Atousa Pahlevan Duprat <apahlevan@ieee.org>\n> ---\n\nIt is helpful to reviewers if you can describe here, below the \"---\"\nline, how this version of the patch differs from the previous one.\nMore below...\n\n> diff --git a/Makefile b/Makefile\n> @@ -136,11 +136,15 @@ all::\n>  # to provide your own OpenSSL library, for example from MacPorts.\n>  #\n>  # Define BLK_SHA1 environment variable to make use of the bundled\n> -# optimized C SHA1 routine.\n> +# optimized C SHA1 routine.  This implies NO_APPLE_COMMON_CRYPTO.\n>  #\n>  # Define PPC_SHA1 environment variable when running make to make use of\n>  # a bundled SHA1 routine optimized for PowerPC.\n>  #\n> +# Define SHA1_MAX_BLOCK_SIZE if your SSH1_Update() implementation can\n> +# hash only a limited amount of data in one call (e.g. APPLE_COMMON_CRYPTO\n> +# may want 'SHA1_MAX_BLOCK_SIZE=1024L*1024L*1024L' defined).\n> +#\n>  # Define NEEDS_CRYPTO_WITH_SSL if you need -lcrypto when using -lssl (Darwin).\n>  #\n>  # Define NEEDS_SSL_WITH_CRYPTO if you need -lssl when using -lcrypto (Darwin).\n> @@ -986,6 +990,10 @@ ifeq (no,$(USE_PARENS_AROUND_GETTEXT_N))\n>  endif\n>  endif\n>\n> +ifdef BLK_SHA1\n> +       NO_APPLE_COMMON_CRYPTO=1\n> +endif\n\nI think Junio already mentioned[1] that this change (and its\ncorresponding documentation update above) likely is undesirable, but I\njust wanted to mention that you would typically want to split such a\nchange out to a separate patch since it's unrelated to the overall\nintention of the current patch. More below...\n\n[1]: http://article.gmane.org/gmane.comp.version-control.git/280859\n\n>  ifeq ($(uname_S),Darwin)\n>         ifndef NO_FINK\n>                 ifeq ($(shell test -d /sw/lib && echo y),y)\n> @@ -1346,6 +1354,8 @@ else\n>  ifdef APPLE_COMMON_CRYPTO\n>         COMPAT_CFLAGS += -DCOMMON_DIGEST_FOR_OPENSSL\n>         SHA1_HEADER = <CommonCrypto/CommonDigest.h>\n> +       # Apple CommonCrypto requires chunking\n> +       SHA1_MAX_BLOCK_SIZE = 1024L*1024L*1024L\n>  else\n>         SHA1_HEADER = <openssl/sha.h>\n>         EXTLIBS += $(LIB_4_CRYPTO)\n> @@ -1353,6 +1363,10 @@ endif\n>  endif\n>  endif\n>\n> +ifdef SHA1_MAX_BLOCK_SIZE\n> +       LIB_OBJS += compat/sha1_chunked.o\n> +       BASIC_CFLAGS += -DSHA1_MAX_BLOCK_SIZE=\"$(SHA1_MAX_BLOCK_SIZE)\"\n> +endif\n> diff --git a/block-sha1/sha1.h b/block-sha1/sha1.h\n> @@ -18,5 +18,5 @@ void blk_SHA1_Final(unsigned char hashout[20], blk_SHA_CTX *ctx);\n>\n>  #define git_SHA_CTX    blk_SHA_CTX\n>  #define git_SHA1_Init  blk_SHA1_Init\n> -#define git_SHA1_Update        blk_SHA1_Update\n> +#define platform_SHA1_Update   blk_SHA1_Update\n>  #define git_SHA1_Final blk_SHA1_Final\n> diff --git a/cache.h b/cache.h\n> index 79066e5..e345e38 100644\n> --- a/cache.h\n> +++ b/cache.h\n> @@ -10,12 +10,21 @@\n>  #include \"trace.h\"\n>  #include \"string-list.h\"\n>\n> +/* platform's underlying implementation of SHA1 */\n>  #include SHA1_HEADER\n>  #ifndef git_SHA_CTX\n> -#define git_SHA_CTX    SHA_CTX\n> -#define git_SHA1_Init  SHA1_Init\n> -#define git_SHA1_Update        SHA1_Update\n> -#define git_SHA1_Final SHA1_Final\n> +#define git_SHA_CTX            SHA_CTX\n> +#define git_SHA1_Init          SHA1_Init\n> +#define platform_SHA1_Update   SHA1_Update\n> +#define git_SHA1_Final         SHA1_Final\n> +#endif\n\nIt's a bit ugly that this special-cases only \"update\" with the\n\"platform_\" abstraction. It _might_ be preferable to generalize this\nfor all the SHA1 operations. If so, that's something you'd want to do\nas a separate preparatory patch specifically aimed at adding this new\nabstraction layer.\n\n(In fact, even if you decide only to special-case \"update\", that still\nmight deserve a separate preparatory patch since it's a conceptually\ndistinct change from the overall focus of this patch, and would make\nthis patch less noisy, thus easier to review.)\n\n> +/* choose chunked implementation or not */\n> +#ifdef SHA1_MAX_BLOCK_SIZE\n> +int git_SHA1_Update_Chunked(SHA_CTX *c, const void *data, size_t len);\n> +#define git_SHA1_Update       git_SHA1_Update_Chunked\n> +#else\n> +#define git_SHA1_Update       platform_SHA1_Update\n>  #endif\n"}]}