Re: [PATCH v7 3/7] eoie: add End of Index Entry (EOIE) extension

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 10/1/2018 11:30 AM, Duy Nguyen wrote:
On Mon, Oct 1, 2018 at 3:46 PM Ben Peart <peartben@xxxxxxxxx> wrote:
@@ -2479,6 +2491,7 @@ static int do_write_index(struct index_state *istate, struct tempfile *tempfile,
         if (ce_write(&c, newfd, &hdr, sizeof(hdr)) < 0)
                 return -1;

+       offset = lseek(newfd, 0, SEEK_CUR) + write_buffer_len;

Note, lseek() could in theory return -1 on error. Looking at the error
code list in the man page it's pretty unlikely though, unless


Good catch. I'll add the logic to check for an error.

+static size_t read_eoie_extension(const char *mmap, size_t mmap_size)
+{
+       /*
+        * The end of index entries (EOIE) extension is guaranteed to be last
+        * so that it can be found by scanning backwards from the EOF.
+        *
+        * "EOIE"
+        * <4-byte length>
+        * <4-byte offset>
+        * <20-byte hash>
+        */
+       const char *index, *eoie;
+       uint32_t extsize;
+       size_t offset, src_offset;
+       unsigned char hash[GIT_MAX_RAWSZ];
+       git_hash_ctx c;
+
+       /* ensure we have an index big enough to contain an EOIE extension */
+       if (mmap_size < sizeof(struct cache_header) + EOIE_SIZE_WITH_HEADER + the_hash_algo->rawsz)

Using sizeof() for on-disk structures could be dangerous because you
don't know how much padding there could be (I'm not sure if it's
actually specified in the C language spec). I've checked, on at least
x86 and amd64, sizeof(struct cache_header) is 12 bytes, but I don't
know if there are any crazy architectures out there that set higher
padding.


This must be safe as the same code has been in do_read_index() and verify_index_from() for a long time.



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux