{"thread":{"id":"7531","subject":"[PATCH 02/13] declare overflow during base128 decoding when 1 MSB nonzero, not 7","startedAt":"2007-04-05T22:24:49Z","lastAt":"2007-04-06T00:45:37Z","messageCount":6,"participants":["Dana How","Junio C Hamano","Nicolas Pitre"],"isPatch":true,"patchVersion":1,"patchTotal":13},"messages":[{"id":"38743","messageId":"56b7f5510704051524p28eafc18mae3131ef13cdabfa@mail.gmail.com","threadId":"7531","inReplyTo":null,"subject":"[PATCH 02/13] declare overflow during base128 decoding when 1 MSB nonzero, not 7","fromName":"Dana How","fromEmail":"danahow@gmail.com","sentAt":"2007-04-05T22:24:49Z","receivedAt":"2007-04-05T22:24:49Z","isPatch":true,"sender":{"key":"danahow@gmail.com","avatar":null},"body":"---\n builtin-pack-objects.c | 2 +-\n builtin-unpack-objects.c | 2 +-\n index-pack.c | 2 +-\n sha1_file.c | 2 +-\n 4 files changed, 4 insertions(+), 4 deletions(-)\n\n-- \nDana L. How danahow@gmail.com +1 650 804 5991 cell\n\n\nFrom 90937b722f391abe1cbc035c29b33f500c389334 Mon Sep 17 00:00:00 2001\nFrom: Dana How \nDate: Thu, 5 Apr 2007 12:15:22 -0700\nSubject: [PATCH 02/13] declare overflow during base128 decoding when 1 MSB nonzero, not 7\n\n---\n builtin-pack-objects.c | 2 +-\n builtin-unpack-objects.c | 2 +-\n index-pack.c | 2 +-\n sha1_file.c | 2 +-\n 4 files changed, 4 insertions(+), 4 deletions(-)\n\ndiff --git a/builtin-pack-objects.c b/builtin-pack-objects.c\nindex b5f9648..50246e1 100644\n--- a/builtin-pack-objects.c\n+++ b/builtin-pack-objects.c\n@@ -1014,7 +1014,7 @@ static void check_object(struct object_entry *entry)\n \t\t\t\tofs = c & 127;\n \t\t\t\twhile (c & 128) {\n \t\t\t\t\tofs += 1;\n-\t\t\t\t\tif (!ofs || ofs & ~(~0UL >> 7))\n+\t\t\t\t\tif (!ofs || ofs & ~(~0UL >> 1))\n \t\t\t\t\t\tdie(\"delta base offset overflow in pack for %s\",\n \t\t\t\t\t\t sha1_to_hex(entry->sha1));\n \t\t\t\t\tc = buf[used_0++];\ndiff --git a/builtin-unpack-objects.c b/builtin-unpack-objects.c\nindex 3956c56..84a6c5c 100644\n--- a/builtin-unpack-objects.c\n+++ b/builtin-unpack-objects.c\n@@ -209,7 +209,7 @@ static void unpack_delta_entry(enum object_type type, unsigned long delta_size,\n \t\tbase_offset = c & 127;\n \t\twhile (c & 128) {\n \t\t\tbase_offset += 1;\n-\t\t\tif (!base_offset || base_offset & ~(~0UL >> 7))\n+\t\t\tif (!base_offset || base_offset & ~(~0UL >> 1))\n \t\t\t\tdie(\"offset value overflow for delta base object\");\n \t\t\tpack = fill(1);\n \t\t\tc = *pack;\ndiff --git a/index-pack.c b/index-pack.c\nindex 3c768fb..02722f7 100644\n--- a/index-pack.c\n+++ b/index-pack.c\n@@ -249,7 +249,7 @@ static void *unpack_raw_entry(struct object_entry *obj, union delta_base *delta_\n \t\tbase_offset = c & 127;\n \t\twhile (c & 128) {\n \t\t\tbase_offset += 1;\n-\t\t\tif (!base_offset || base_offset & ~(~0UL >> 7))\n+\t\t\tif (!base_offset || base_offset & ~(~0UL >> 1))\n \t\t\t\tbad_object(obj->offset, \"offset value overflow for delta base object\");\n \t\t\tp = fill(1);\n \t\t\tc = *p;\ndiff --git a/sha1_file.c b/sha1_file.c\nindex 9c26038..7082d3e 100644\n--- a/sha1_file.c\n+++ b/sha1_file.c\n@@ -1149,7 +1149,7 @@ static off_t get_delta_base(struct packed_git *p,\n \t\tbase_offset = c & 127;\n \t\twhile (c & 128) {\n \t\t\tbase_offset += 1;\n-\t\t\tif (!base_offset || base_offset & ~(~0UL >> 7))\n+\t\t\tif (!base_offset || base_offset & ~(~0UL >> 1))\n \t\t\t\tdie(\"offset value overflow for delta base object\");\n \t\t\tc = base_info[used++];\n \t\t\tbase_offset = (base_offset << 7) + (c & 127);\n-- \n1.5.1.rc2.18.g9c88-dirty\n\n"},{"id":"38755","messageId":"7vr6qymo3r.fsf@assigned-by-dhcp.cox.net","threadId":"7531","inReplyTo":"56b7f5510704051524p28eafc18mae3131ef13cdabfa@mail.gmail.com","subject":"Re: [PATCH 02/13] declare overflow during base128 decoding when 1 MSB nonzero, not 7","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-04-05T22:51:36Z","receivedAt":"2007-04-05T22:51:36Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Dana How\" writes:\n\n> Subject: [PATCH 02/13] declare overflow during base128 decoding when 1 MSB nonzero, not 7\n>\n> ---\n> builtin-pack-objects.c | 2 +-\n> builtin-unpack-objects.c | 2 +-\n> index-pack.c | 2 +-\n> sha1_file.c | 2 +-\n> 4 files changed, 4 insertions(+), 4 deletions(-)\n>\n> diff --git a/builtin-pack-objects.c b/builtin-pack-objects.c\n> index b5f9648..50246e1 100644\n> --- a/builtin-pack-objects.c\n> +++ b/builtin-pack-objects.c\n> @@ -1014,7 +1014,7 @@ static void check_object(struct object_entry *entry)\n> \t\t\t\tofs = c & 127;\n> \t\t\t\twhile (c & 128) {\n> \t\t\t\t\tofs += 1;\n> -\t\t\t\t\tif (!ofs || ofs & ~(~0UL >> 7))\n> +\t\t\t\t\tif (!ofs || ofs & ~(~0UL >> 1))\n> \t\t\t\t\t\tdie(\"delta base offset overflow in pack for %s\",\n> \t\t\t\t\t\t sha1_to_hex(entry->sha1));\n> \t\t\t\t\tc = buf[used_0++];\n\nThe line after these context does this:\n\n\tofs = (ofs << 7) + (c & 127);\n\nIf you do not check the top 7 bits, wouldn't you miss overflow?\n"},{"id":"38760","messageId":"56b7f5510704051608g4bf9ffb7t9f94b532c7a0ebdf@mail.gmail.com","threadId":"7531","inReplyTo":"7vr6qymo3r.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH 02/13] declare overflow during base128 decoding when 1 MSB nonzero, not 7","fromName":"Dana How","fromEmail":"danahow@gmail.com","sentAt":"2007-04-05T23:08:55Z","receivedAt":"2007-04-05T23:08:55Z","isPatch":true,"sender":{"key":"danahow@gmail.com","avatar":null},"body":"On 4/5/07, Junio C Hamano wrote:\n> \"Dana How\" writes:\n> > ofs = c & 127;\n> > while (c & 128) {\n> > ofs += 1;\n> > - if (!ofs || ofs & ~(~0UL >> 7))\n> > + if (!ofs || ofs & ~(~0UL >> 1))\n> > die(\"delta base offset overflow in pack for %s\",\n> > sha1_to_hex(entry->sha1));\n> > c = buf[used_0++];\n> The line after these context does this:\n> ofs = (ofs << 7) + (c & 127);\n> If you do not check the top 7 bits, wouldn't you miss overflow?\n\nYou are correct in all 4 cases --\nthis patch can be dropped from the set;\n it has no overlap in any context.\n\nConcerning SEEK_SET,\nyou are correct: my unistd.h #define's SEEK_SET to 0.\nBut if someone edits this, the arg order might remain.\n\nI will follow your commit text suggestions.\n\nLet me think about your comment on 08/13.\n\nThanks!\n-- \nDana L. How danahow@gmail.com +1 650 804 5991 cell\n"},{"id":"38765","messageId":"7vvegal506.fsf@assigned-by-dhcp.cox.net","threadId":"7531","inReplyTo":"56b7f5510704051608g4bf9ffb7t9f94b532c7a0ebdf@mail.gmail.com","subject":"Re: [PATCH 02/13] declare overflow during base128 decoding when 1 MSB nonzero, not 7","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-04-06T00:29:29Z","receivedAt":"2007-04-06T00:29:29Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Dana How\" writes:\n\n> Concerning SEEK_SET,\n> you are correct: my unistd.h #define's SEEK_SET to 0.\n\nMy understanding of the history is back when there was seek()\nbut not lseek() nor SEEK_X macros, everybody passed bare\nintegers and it was quite well established that 0 meant what\nSEEK_SET means now, so I would be quite surprised if there is\n_any_ platform that defines SEEK_SET to anything but zero.\n\nHaving said that, I wonder if this kind of thing can be caught\nwith sparse. We should be able to express (or perhaps sparse\ncould have a built-in rule that says) something like \"although\nthe standard says the third parameter to lseek() can be any\ninteger, you should not use anything but SEEK_X preprocessor\nmacros; also SEEK_X preprocessor macros, although they are\ntypically just \"#define\"s of small integer literals, can never\nappear anywhere other than the third parameter as seek() and\nlseek()\". I realize that there needs some data-flow analysis to\nallow a code like this, though:\n\n\tint move_to(int fd, ofs_t offset, int relative)\n\t{\n\t\tint whence = SEEK_SET;\n\n\t if (relative) {\n \twhence = SEEK_CUR;\n\t offset += relative;\n\t }\n\t\treturn lseek(fd, offset, whence) != (off_t)-1;\n\t}\n\n> But if someone edits this, the arg order might remain.\n\nVery true, and that is why I said I am going to take the patch.\n"},{"id":"38766","messageId":"alpine.LFD.0.98.0704052033350.28181@xanadu.home","threadId":"7531","inReplyTo":"56b7f5510704051524p28eafc18mae3131ef13cdabfa@mail.gmail.com","subject":"Re: [PATCH 02/13] declare overflow during base128 decoding when 1 MSB nonzero, not 7","fromName":"Nicolas Pitre","fromEmail":"nico@cam.org","sentAt":"2007-04-06T00:35:41Z","receivedAt":"2007-04-06T00:35:41Z","isPatch":true,"sender":{"key":"nico@fluxnic.net","avatar":"https://avatars.githubusercontent.com/u/702790?v=4"},"body":"\nThis patch looks completely wrong to me. Please explain why you're \ndoing this?\n\n\nNicolas\n"},{"id":"38768","messageId":"alpine.LFD.0.98.0704052045070.28181@xanadu.home","threadId":"7531","inReplyTo":"alpine.LFD.0.98.0704052033350.28181@xanadu.home","subject":"Re: [PATCH 02/13] declare overflow during base128 decoding when 1 MSB nonzero, not 7","fromName":"Nicolas Pitre","fromEmail":"nico@cam.org","sentAt":"2007-04-06T00:45:37Z","receivedAt":"2007-04-06T00:45:37Z","isPatch":true,"sender":{"key":"nico@fluxnic.net","avatar":"https://avatars.githubusercontent.com/u/702790?v=4"},"body":"On Thu, 5 Apr 2007, Nicolas Pitre wrote:\n\n> \n> This patch looks completely wrong to me. Please explain why you're \n> doing this?\n\nNever mind. I see that Junio and you already cleared that one.\n\n\nNicolas\n"}]}