[PATCH v2 0/8] Some object db protection when add_submodule_odb is used

[PATCH v2 0/8] Some object db protection when add_submodule_odb is used

[Nguyễn Thái Ngọc Duy]

The idea behind this series is, after add_submodule_odb, odb may have
new temporary objects that only appear after the call. These temporary
objects may lead to repo corruption (e.g. some new objects are created
and point to these temporary objects). This series attempts to catch
those cases. It would make it safer to dig deeper into submodule's odb,
e.g. to implement unified git-diff.

Previous approach [1] is record the odb source, then check if the
source is from submodule's odb. But that means we rely on the
lookup order in sha1_file.c. This approach instead allows the caller
to select what odb sources it wants to look up from.

The checks are also less drastic than before. Checks are now done at
higher level, e.g. commit_tree(), instead of at write_sha1_file,
because we do allow to write objects that point to nowhere.

Another new thing from previous round is I completely forbid the use
of add_submodule_odb in security sensitive commands like index-pack or
rev-list. We could loosen up later if we need to.

For fun, I set object_database_contaminated to 1 by default and ran
the test suite. It passed :)

[1] http://thread.gmane.org/gmane.comp.version-control.git/214412/focus=214417

Nguyễn Thái Ngọc Duy (8):
  sha1_file: allow to select pack origin when looking up an object
  sha1_file: keep track of alternate source of objects
  sha1_file: mark alt object database from add_submodule_odb()
  sha1_file: new object source for submodule's alt object database
  commit.c: refuse to write commits referring to external objects
  cache-tree.c: refuse to write trees referring to external objects
  mktag: refuse to write tags referring to external objects
  sha1_file: do write objects even if found in ODB_EXTALT database

 builtin/index-pack.c |   2 +-
 builtin/mktag.c      |   5 +-
 cache-tree.c         |   3 +-
 cache.h              |  38 +++++++--
 commit.c             |   9 ++
 environment.c        |   2 +
 fast-import.c        |   4 +-
 git.c                |  10 ++-
 pack-check.c         |   2 +-
 sha1_file.c          | 236 ++++++++++++++++++++++++++++++++++++---------------
 streaming.c          |   4 +-
 submodule.c          |   5 +-
 12 files changed, 233 insertions(+), 87 deletions(-)

-- 
1.8.2.83.gc99314b

[PATCH v2 1/8] sha1_file: allow to select pack origin when looking up an object

[Nguyễn Thái Ngọc Duy]

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 cache.h       |  29 +++++++++--
 fast-import.c |   2 +-
 pack-check.c  |   2 +-
 sha1_file.c   | 165 +++++++++++++++++++++++++++++++++++++++-------------------
 streaming.c   |   4 +-
 5 files changed, 141 insertions(+), 61 deletions(-)

diff --git a/cache.h b/cache.h
index 94ca1ac..bed403a 100644
--- a/cache.h
+++ b/cache.h
@@ -744,12 +744,33 @@ char *strip_path_suffix(const char *path, const char *suffix);
 int daemon_avoid_alias(const char *path);
 int offset_1st_component(const char *path);
 
+/*
+ * odb origin bit mask:
+ * - 8 low bits are for requested objects
+ * - 8 high bits are for their base objects
+ */
+#define ODB_LOCAL	1
+#define ODB_ALT		2
+#define ODB_CACHED	4
+/*
+ * This flag is used to track if the origin selection is from
+ * "odb_default" below. Not a real source, not to be passed to any
+ * function cals explicitly.
+ */
+#define ODB_DEFAULT	8
+
+extern unsigned int odb_default;
+
 /* object replacement */
 #define READ_SHA1_FILE_REPLACE 1
-extern void *read_sha1_file_extended(const unsigned char *sha1, enum object_type *type, unsigned long *size, unsigned flag);
+extern void *read_sha1_file_extended(const unsigned char *sha1,
+				     unsigned int origin,
+				     enum object_type *type,
+				     unsigned long *size,
+				     unsigned flag);
 static inline void *read_sha1_file(const unsigned char *sha1, enum object_type *type, unsigned long *size)
 {
-	return read_sha1_file_extended(sha1, type, size, READ_SHA1_FILE_REPLACE);
+	return read_sha1_file_extended(sha1, odb_default, type, size, READ_SHA1_FILE_REPLACE);
 }
 extern const unsigned char *do_lookup_replace_object(const unsigned char *sha1);
 static inline const unsigned char *lookup_replace_object(const unsigned char *sha1)
@@ -765,7 +786,7 @@ extern int hash_sha1_file(const void *buf, unsigned long len, const char *type,
 extern int write_sha1_file(const void *buf, unsigned long len, const char *type, unsigned char *return_sha1);
 extern int pretend_sha1_file(void *, unsigned long, enum object_type, unsigned char *);
 extern int force_object_loose(const unsigned char *sha1, time_t mtime);
-extern void *map_sha1_file(const unsigned char *sha1, unsigned long *size);
+extern void *map_sha1_file(const unsigned char *sha1, unsigned int, unsigned long *size);
 extern int unpack_sha1_header(git_zstream *stream, unsigned char *map, unsigned long mapsize, void *buffer, unsigned long bufsiz);
 extern int parse_sha1_header(const char *hdr, unsigned long *sizep);
 
@@ -1091,7 +1112,7 @@ extern const unsigned char *nth_packed_object_sha1(struct packed_git *, uint32_t
 extern off_t nth_packed_object_offset(const struct packed_git *, uint32_t);
 extern off_t find_pack_entry_one(const unsigned char *, struct packed_git *);
 extern int is_pack_valid(struct packed_git *);
-extern void *unpack_entry(struct packed_git *, off_t, enum object_type *, unsigned long *);
+extern void *unpack_entry(struct packed_git *, unsigned int, off_t, enum object_type *, unsigned long *);
 extern unsigned long unpack_object_header_buffer(const unsigned char *buf, unsigned long len, enum object_type *type, unsigned long *sizep);
 extern unsigned long get_size_from_delta(struct packed_git *, struct pack_window **, off_t);
 extern int unpack_object_header(struct packed_git *, struct pack_window **, off_t *, unsigned long *);
diff --git a/fast-import.c b/fast-import.c
index 5f539d7..8542786 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -1303,7 +1303,7 @@ static void *gfi_unpack_entry(
 		 */
 		p->pack_size = pack_size + 20;
 	}
-	return unpack_entry(p, oe->idx.offset, &type, sizep);
+	return unpack_entry(p, odb_default, oe->idx.offset, &type, sizep);
 }
 
 static const char *get_mode(const char *str, uint16_t *modep)
diff --git a/pack-check.c b/pack-check.c
index 63a595c..981dc98 100644
--- a/pack-check.c
+++ b/pack-check.c
@@ -116,7 +116,7 @@ static int verify_packfile(struct packed_git *p,
 					    sha1_to_hex(entries[i].sha1),
 					    p->pack_name, (uintmax_t)offset);
 		}
-		data = unpack_entry(p, entries[i].offset, &type, &size);
+		data = unpack_entry(p, odb_default, entries[i].offset, &type, &size);
 		if (!data)
 			err = error("cannot unpack %s from %s at offset %"PRIuMAX"",
 				    sha1_to_hex(entries[i].sha1), p->pack_name,
diff --git a/sha1_file.c b/sha1_file.c
index 99ead7c..d1f44c9 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -37,6 +37,12 @@ static inline uintmax_t sz_fmt(size_t s) { return s; }
 const unsigned char null_sha1[20];
 
 /*
+ * clear_delta_base_cache() may be needed if odb_default is changed to
+ * a narrower origin set.
+ */
+unsigned int odb_default = ODB_DEFAULT | ODB_LOCAL | ODB_ALT | ODB_CACHED;
+
+/*
  * This is meant to hold a *small* number of objects that you would
  * want read_sha1_file() to be able to return, but yet you do not want
  * to write them into the object store (e.g. a browse-only
@@ -59,11 +65,15 @@ static struct cached_object empty_tree = {
 
 static struct packed_git *last_found_pack;
 
-static struct cached_object *find_cached_object(const unsigned char *sha1)
+static struct cached_object *find_cached_object(const unsigned char *sha1,
+						unsigned int origin)
 {
 	int i;
 	struct cached_object *co = cached_objects;
 
+	if (!(origin & ODB_CACHED))
+		return NULL;
+
 	for (i = 0; i < cached_object_nr; i++, co++) {
 		if (!hashcmp(co->sha1, sha1))
 			return co;
@@ -404,6 +414,12 @@ void foreach_alt_odb(alt_odb_fn fn, void *cb)
 			return;
 }
 
+static inline int match_origin(unsigned int origin, int pack_local)
+{
+	return (pack_local && (origin & ODB_LOCAL)) ||
+		(!pack_local && (origin & ODB_ALT));
+}
+
 void prepare_alt_odb(void)
 {
 	const char *alt;
@@ -420,28 +436,31 @@ void prepare_alt_odb(void)
 	read_info_alternates(get_object_directory(), 0);
 }
 
-static int has_loose_object_local(const unsigned char *sha1)
+static int has_loose_object_extended(const unsigned char *sha1,
+				     unsigned int origin)
 {
-	char *name = sha1_file_name(sha1);
-	return !access(name, F_OK);
-}
-
-int has_loose_object_nonlocal(const unsigned char *sha1)
-{
-	struct alternate_object_database *alt;
-	prepare_alt_odb();
-	for (alt = alt_odb_list; alt; alt = alt->next) {
-		fill_sha1_path(alt->name, sha1);
-		if (!access(alt->base, F_OK))
+	if (origin & ODB_LOCAL) {
+		char *name = sha1_file_name(sha1);
+		if (!access(name, F_OK))
 			return 1;
 	}
+
+	if (origin & ODB_ALT) {
+		struct alternate_object_database *alt;
+		prepare_alt_odb();
+		for (alt = alt_odb_list; alt; alt = alt->next) {
+			fill_sha1_path(alt->name, sha1);
+			if (!access(alt->base, F_OK))
+				return 1;
+		}
+	}
 	return 0;
 }
 
-static int has_loose_object(const unsigned char *sha1)
+int has_loose_object_nonlocal(const unsigned char *sha1)
 {
-	return has_loose_object_local(sha1) ||
-	       has_loose_object_nonlocal(sha1);
+	unsigned int origin = ODB_ALT;
+	return has_loose_object_extended(sha1, origin);
 }
 
 static unsigned int pack_used_ctr;
@@ -1303,16 +1322,21 @@ static int git_open_noatime(const char *name)
 	}
 }
 
-static int open_sha1_file(const unsigned char *sha1)
+static int open_sha1_file(const unsigned char *sha1, unsigned int origin)
 {
 	int fd;
 	char *name = sha1_file_name(sha1);
 	struct alternate_object_database *alt;
 
-	fd = git_open_noatime(name);
-	if (fd >= 0)
+	if ((origin & ODB_LOCAL) &&
+	    (fd = git_open_noatime(name)) >= 0)
 		return fd;
 
+	if (!(origin & ODB_ALT)) {
+		errno = ENOENT;
+		return -1;
+	}
+
 	prepare_alt_odb();
 	errno = ENOENT;
 	for (alt = alt_odb_list; alt; alt = alt->next) {
@@ -1325,12 +1349,14 @@ static int open_sha1_file(const unsigned char *sha1)
 	return -1;
 }
 
-void *map_sha1_file(const unsigned char *sha1, unsigned long *size)
+void *map_sha1_file(const unsigned char *sha1,
+		    unsigned int origin,
+		    unsigned long *size)
 {
 	void *map;
 	int fd;
 
-	fd = open_sha1_file(sha1);
+	fd = open_sha1_file(sha1, origin);
 	map = NULL;
 	if (fd >= 0) {
 		struct stat st;
@@ -1872,7 +1898,8 @@ static void clear_delta_base_cache_entry(struct delta_base_cache_entry *ent)
 	delta_base_cached -= ent->size;
 }
 
-static void *cache_or_unpack_entry(struct packed_git *p, off_t base_offset,
+static void *cache_or_unpack_entry(struct packed_git *p,
+				   unsigned int origin, off_t base_offset,
 	unsigned long *base_size, enum object_type *type, int keep_cache)
 {
 	struct delta_base_cache_entry *ent;
@@ -1880,8 +1907,9 @@ static void *cache_or_unpack_entry(struct packed_git *p, off_t base_offset,
 
 	ent = get_delta_base_cache_entry(p, base_offset);
 
-	if (!eq_delta_base_cache_entry(ent, p, base_offset))
-		return unpack_entry(p, base_offset, type, base_size);
+	if (!(origin & ODB_DEFAULT) || /* only cache the default case */
+	    !eq_delta_base_cache_entry(ent, p, base_offset))
+		return unpack_entry(p, origin, base_offset, type, base_size);
 
 	ret = ent->data;
 
@@ -1951,7 +1979,9 @@ add_delta_base_cache(struct packed_git *p, off_t base_offset,
 	return ent;
 }
 
-static void *read_object(const unsigned char *sha1, enum object_type *type,
+static void *read_object(const unsigned char *sha1,
+			 unsigned int origin,
+			 enum object_type *type,
 			 unsigned long *size);
 
 static void write_pack_access_log(struct packed_git *p, off_t obj_offset)
@@ -1981,7 +2011,8 @@ struct unpack_entry_stack_ent {
 	unsigned long size;
 };
 
-void *unpack_entry(struct packed_git *p, off_t obj_offset,
+void *unpack_entry(struct packed_git *p,
+		   unsigned int origin, off_t obj_offset,
 		   enum object_type *final_type, unsigned long *final_size)
 {
 	struct pack_window *w_curs = NULL;
@@ -2093,7 +2124,7 @@ void *unpack_entry(struct packed_git *p, off_t obj_offset,
 
 		data = NULL;
 
-		if (base)
+		if (base && (origin & ODB_DEFAULT))
 			ent = add_delta_base_cache(p, obj_offset, base, base_size, type);
 
 		if (!base) {
@@ -2113,7 +2144,7 @@ void *unpack_entry(struct packed_git *p, off_t obj_offset,
 				      sha1_to_hex(base_sha1), (uintmax_t)obj_offset,
 				      p->pack_name);
 				mark_bad_packed_object(p, base_sha1);
-				base = read_object(base_sha1, &type, &base_size);
+				base = read_object(base_sha1, origin, &type, &base_size);
 			}
 		}
 
@@ -2277,11 +2308,15 @@ int is_pack_valid(struct packed_git *p)
 }
 
 static int fill_pack_entry(const unsigned char *sha1,
+			   unsigned int origin,
 			   struct pack_entry *e,
 			   struct packed_git *p)
 {
 	off_t offset;
 
+	if (!match_origin(origin, p->pack_local))
+		return 0;
+
 	if (p->num_bad_objects) {
 		unsigned i;
 		for (i = 0; i < p->num_bad_objects; i++)
@@ -2310,7 +2345,9 @@ static int fill_pack_entry(const unsigned char *sha1,
 	return 1;
 }
 
-static int find_pack_entry(const unsigned char *sha1, struct pack_entry *e)
+static int find_pack_entry(const unsigned char *sha1,
+			   unsigned int origin,
+			   struct pack_entry *e)
 {
 	struct packed_git *p;
 
@@ -2318,11 +2355,12 @@ static int find_pack_entry(const unsigned char *sha1, struct pack_entry *e)
 	if (!packed_git)
 		return 0;
 
-	if (last_found_pack && fill_pack_entry(sha1, e, last_found_pack))
+	if (last_found_pack &&
+	    fill_pack_entry(sha1, origin, e, last_found_pack))
 		return 1;
 
 	for (p = packed_git; p; p = p->next) {
-		if (p == last_found_pack || !fill_pack_entry(sha1, e, p))
+		if (p == last_found_pack || !fill_pack_entry(sha1, origin, e, p))
 			continue;
 
 		last_found_pack = p;
@@ -2352,7 +2390,7 @@ static int sha1_loose_object_info(const unsigned char *sha1, unsigned long *size
 	git_zstream stream;
 	char hdr[32];
 
-	map = map_sha1_file(sha1, &mapsize);
+	map = map_sha1_file(sha1, odb_default, &mapsize);
 	if (!map)
 		return error("unable to find %s", sha1_to_hex(sha1));
 	if (unpack_sha1_header(&stream, map, mapsize, hdr, sizeof(hdr)) < 0)
@@ -2373,8 +2411,9 @@ int sha1_object_info_extended(const unsigned char *sha1, struct object_info *oi)
 	struct cached_object *co;
 	struct pack_entry e;
 	int status, rtype;
+	unsigned int origin = odb_default;
 
-	co = find_cached_object(sha1);
+	co = find_cached_object(sha1, origin);
 	if (co) {
 		if (oi->sizep)
 			*(oi->sizep) = co->size;
@@ -2382,7 +2421,7 @@ int sha1_object_info_extended(const unsigned char *sha1, struct object_info *oi)
 		return co->type;
 	}
 
-	if (!find_pack_entry(sha1, &e)) {
+	if (!find_pack_entry(sha1, origin, &e)) {
 		/* Most likely it's a loose object. */
 		status = sha1_loose_object_info(sha1, oi->sizep);
 		if (status >= 0) {
@@ -2392,7 +2431,7 @@ int sha1_object_info_extended(const unsigned char *sha1, struct object_info *oi)
 
 		/* Not a loose object; someone else may have just packed it. */
 		reprepare_packed_git();
-		if (!find_pack_entry(sha1, &e))
+		if (!find_pack_entry(sha1, origin, &e))
 			return status;
 	}
 
@@ -2422,15 +2461,26 @@ int sha1_object_info(const unsigned char *sha1, unsigned long *sizep)
 }
 
 static void *read_packed_sha1(const unsigned char *sha1,
+			      unsigned int origin,
 			      enum object_type *type, unsigned long *size)
 {
 	struct pack_entry e;
 	void *data;
 
-	if (!find_pack_entry(sha1, &e))
+	if (!find_pack_entry(sha1, origin, &e))
 		return NULL;
-	data = cache_or_unpack_entry(e.p, e.offset, size, type, 1);
-	if (!data) {
+	data = cache_or_unpack_entry(e.p, origin, e.offset, size, type, 1);
+	if (!data &&
+	    /*
+	     * If a user attempts to read from a single source via
+	     * read_sha1_file_extended and the object has base in
+	     * another source, it'll come here. It's not as bad as the
+	     * corrupted pack we're handling in the below block
+	     * because the user asked to do so and must be able to
+	     * deal with the consequences. Just return NULL in this
+	     * case without marking the sha-1 "bad".
+	     */
+	    !(origin & ODB_DEFAULT)) {
 		/*
 		 * We're probably in deep shit, but let's try to fetch
 		 * the required object anyway from another pack or loose.
@@ -2440,7 +2490,7 @@ static void *read_packed_sha1(const unsigned char *sha1,
 		error("failed to read object %s at offset %"PRIuMAX" from %s",
 		      sha1_to_hex(sha1), (uintmax_t)e.offset, e.p->pack_name);
 		mark_bad_packed_object(e.p, sha1);
-		data = read_object(sha1, type, size);
+		data = read_object(sha1, origin, type, size);
 	}
 	return data;
 }
@@ -2451,7 +2501,7 @@ int pretend_sha1_file(void *buf, unsigned long len, enum object_type type,
 	struct cached_object *co;
 
 	hash_sha1_file(buf, len, typename(type), sha1);
-	if (has_sha1_file(sha1) || find_cached_object(sha1))
+	if (has_sha1_file(sha1) || find_cached_object(sha1, odb_default))
 		return 0;
 	if (cached_object_alloc <= cached_object_nr) {
 		cached_object_alloc = alloc_nr(cached_object_alloc);
@@ -2468,31 +2518,33 @@ int pretend_sha1_file(void *buf, unsigned long len, enum object_type type,
 	return 0;
 }
 
-static void *read_object(const unsigned char *sha1, enum object_type *type,
+static void *read_object(const unsigned char *sha1,
+			 unsigned int origin,
+			 enum object_type *type,
 			 unsigned long *size)
 {
 	unsigned long mapsize;
 	void *map, *buf;
 	struct cached_object *co;
 
-	co = find_cached_object(sha1);
+	co = find_cached_object(sha1, origin);
 	if (co) {
 		*type = co->type;
 		*size = co->size;
 		return xmemdupz(co->buf, co->size);
 	}
 
-	buf = read_packed_sha1(sha1, type, size);
+	buf = read_packed_sha1(sha1, origin, type, size);
 	if (buf)
 		return buf;
-	map = map_sha1_file(sha1, &mapsize);
+	map = map_sha1_file(sha1, origin, &mapsize);
 	if (map) {
 		buf = unpack_sha1_file(map, mapsize, type, size, sha1);
 		munmap(map, mapsize);
 		return buf;
 	}
 	reprepare_packed_git();
-	return read_packed_sha1(sha1, type, size);
+	return read_packed_sha1(sha1, origin, type, size);
 }
 
 /*
@@ -2501,6 +2553,7 @@ static void *read_object(const unsigned char *sha1, enum object_type *type,
  * messages themselves.
  */
 void *read_sha1_file_extended(const unsigned char *sha1,
+			      unsigned int origin,
 			      enum object_type *type,
 			      unsigned long *size,
 			      unsigned flag)
@@ -2512,7 +2565,7 @@ void *read_sha1_file_extended(const unsigned char *sha1,
 		? lookup_replace_object(sha1) : sha1;
 
 	errno = 0;
-	data = read_object(repl, type, size);
+	data = read_object(repl, origin, type, size);
 	if (data)
 		return data;
 
@@ -2524,7 +2577,7 @@ void *read_sha1_file_extended(const unsigned char *sha1,
 		die("replacement %s not found for %s",
 		    sha1_to_hex(repl), sha1_to_hex(sha1));
 
-	if (has_loose_object(repl)) {
+	if (has_loose_object_extended(repl, origin)) {
 		path = sha1_file_name(sha1);
 		die("loose object %s (stored in %s) is corrupt",
 		    sha1_to_hex(repl), path);
@@ -2809,9 +2862,9 @@ int force_object_loose(const unsigned char *sha1, time_t mtime)
 	int hdrlen;
 	int ret;
 
-	if (has_loose_object(sha1))
+	if (has_loose_object_extended(sha1, odb_default))
 		return 0;
-	buf = read_packed_sha1(sha1, &type, &len);
+	buf = read_packed_sha1(sha1, odb_default, &type, &len);
 	if (!buf)
 		return error("cannot read sha1_file for %s", sha1_to_hex(sha1));
 	hdrlen = sprintf(hdr, "%s %lu", typename(type), len) + 1;
@@ -2832,16 +2885,22 @@ int has_pack_index(const unsigned char *sha1)
 int has_sha1_pack(const unsigned char *sha1)
 {
 	struct pack_entry e;
-	return find_pack_entry(sha1, &e);
+	return find_pack_entry(sha1, odb_default, &e);
 }
 
-int has_sha1_file(const unsigned char *sha1)
+static int has_sha1_file_extended(const unsigned char *sha1,
+				  unsigned origin)
 {
 	struct pack_entry e;
 
-	if (find_pack_entry(sha1, &e))
+	if (find_pack_entry(sha1, origin, &e))
 		return 1;
-	return has_loose_object(sha1);
+	return has_loose_object_extended(sha1, origin);
+}
+
+int has_sha1_file(const unsigned char *sha1)
+{
+	return has_sha1_file_extended(sha1, odb_default);
 }
 
 static void check_tree(const void *buf, size_t size)
diff --git a/streaming.c b/streaming.c
index cabcd9d..975002c 100644
--- a/streaming.c
+++ b/streaming.c
@@ -332,7 +332,7 @@ static struct stream_vtbl loose_vtbl = {
 
 static open_method_decl(loose)
 {
-	st->u.loose.mapped = map_sha1_file(sha1, &st->u.loose.mapsize);
+	st->u.loose.mapped = map_sha1_file(sha1, odb_default, &st->u.loose.mapsize);
 	if (!st->u.loose.mapped)
 		return -1;
 	if (unpack_sha1_header(&st->z,
@@ -483,7 +483,7 @@ static struct stream_vtbl incore_vtbl = {
 
 static open_method_decl(incore)
 {
-	st->u.incore.buf = read_sha1_file_extended(sha1, type, &st->size, 0);
+	st->u.incore.buf = read_sha1_file_extended(sha1, odb_default, type, &st->size, 0);
 	st->u.incore.read_ptr = 0;
 	st->vtbl = &incore_vtbl;
 
-- 
1.8.2.83.gc99314b

[PATCH v2 2/8] sha1_file: keep track of alternate source of objects

[Nguyễn Thái Ngọc Duy]

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 builtin/index-pack.c |  2 +-
 cache.h              |  3 ++-
 fast-import.c        |  2 +-
 sha1_file.c          | 15 +++++++++------
 4 files changed, 13 insertions(+), 9 deletions(-)

diff --git a/builtin/index-pack.c b/builtin/index-pack.c
index 79dfe47..aab9de5 100644
--- a/builtin/index-pack.c
+++ b/builtin/index-pack.c
@@ -1414,7 +1414,7 @@ static void read_v2_anomalous_offsets(struct packed_git *p,
 
 static void read_idx_option(struct pack_idx_option *opts, const char *pack_name)
 {
-	struct packed_git *p = add_packed_git(pack_name, strlen(pack_name), 1);
+	struct packed_git *p = add_packed_git(pack_name, strlen(pack_name), NULL);
 
 	if (!p)
 		die(_("Cannot open existing pack file '%s'"), pack_name);
diff --git a/cache.h b/cache.h
index bed403a..dfb78ca 100644
--- a/cache.h
+++ b/cache.h
@@ -1014,6 +1014,7 @@ struct pack_window {
 extern struct packed_git {
 	struct packed_git *next;
 	struct pack_window *windows;
+	struct alternate_object_database *alt;
 	off_t pack_size;
 	const void *index_data;
 	size_t index_size;
@@ -1107,7 +1108,7 @@ extern void close_pack_windows(struct packed_git *);
 extern void unuse_pack(struct pack_window **);
 extern void free_pack_by_name(const char *);
 extern void clear_delta_base_cache(void);
-extern struct packed_git *add_packed_git(const char *, int, int);
+extern struct packed_git *add_packed_git(const char *, int, struct alternate_object_database *);
 extern const unsigned char *nth_packed_object_sha1(struct packed_git *, uint32_t);
 extern off_t nth_packed_object_offset(const struct packed_git *, uint32_t);
 extern off_t find_pack_entry_one(const unsigned char *, struct packed_git *);
diff --git a/fast-import.c b/fast-import.c
index 8542786..749e9db 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -964,7 +964,7 @@ static void end_packfile(void)
 		idx_name = keep_pack(create_index());
 
 		/* Register the packfile with core git's machinery. */
-		new_p = add_packed_git(idx_name, strlen(idx_name), 1);
+		new_p = add_packed_git(idx_name, strlen(idx_name), NULL);
 		if (!new_p)
 			die("core git rejected index %s", idx_name);
 		all_packs[pack_id] = new_p;
diff --git a/sha1_file.c b/sha1_file.c
index d1f44c9..1a744ae 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -958,7 +958,8 @@ static void try_to_free_pack_memory(size_t size)
 	release_pack_memory(size, -1);
 }
 
-struct packed_git *add_packed_git(const char *path, int path_len, int local)
+struct packed_git *add_packed_git(const char *path, int path_len,
+				  struct alternate_object_database *alt)
 {
 	static int have_set_try_to_free_routine;
 	struct stat st;
@@ -994,7 +995,8 @@ struct packed_git *add_packed_git(const char *path, int path_len, int local)
 	 * actually mapping the pack file.
 	 */
 	p->pack_size = st.st_size;
-	p->pack_local = local;
+	p->pack_local = !alt;
+	p->alt = alt;
 	p->mtime = st.st_mtime;
 	if (path_len < 40 || get_sha1_hex(path + path_len - 40, p->sha1))
 		hashclr(p->sha1);
@@ -1082,7 +1084,8 @@ static void report_pack_garbage(struct string_list *list)
 	report_helper(list, seen_bits, first, list->nr);
 }
 
-static void prepare_packed_git_one(char *objdir, int local)
+static void prepare_packed_git_one(char *objdir,
+				   struct alternate_object_database *alt)
 {
 	/* Ensure that this buffer is large enough so that we can
 	   append "/pack/" without clobbering the stack even if
@@ -1133,7 +1136,7 @@ static void prepare_packed_git_one(char *objdir, int local)
 			     * See if it really is a valid .idx file with
 			     * corresponding .pack file that we can map.
 			     */
-			    (p = add_packed_git(path, len + namelen, local)) != NULL)
+			    (p = add_packed_git(path, len + namelen, alt)) != NULL)
 				install_packed_git(p);
 		}
 
@@ -1213,11 +1216,11 @@ void prepare_packed_git(void)
 
 	if (prepare_packed_git_run_once)
 		return;
-	prepare_packed_git_one(get_object_directory(), 1);
+	prepare_packed_git_one(get_object_directory(), NULL);
 	prepare_alt_odb();
 	for (alt = alt_odb_list; alt; alt = alt->next) {
 		alt->name[-1] = 0;
-		prepare_packed_git_one(alt->base, 0);
+		prepare_packed_git_one(alt->base, alt);
 		alt->name[-1] = '/';
 	}
 	rearrange_packed_git();
-- 
1.8.2.83.gc99314b

[PATCH v2 3/8] sha1_file: mark alt object database from add_submodule_odb()

[Nguyễn Thái Ngọc Duy]

A submodule's object database may be imported to in-core object pool
for a quick peek without paying the price of running a separate git
command. These databases are marked in for stricter checks later to
avoid accidentially refering to a submodule's SHA-1 from main repo
(except in gitlinks).

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 cache.h       |  4 +++-
 environment.c |  2 ++
 sha1_file.c   | 37 ++++++++++++++++++++++++++++---------
 submodule.c   |  2 +-
 4 files changed, 34 insertions(+), 11 deletions(-)

diff --git a/cache.h b/cache.h
index dfb78ca..b8d8e03 100644
--- a/cache.h
+++ b/cache.h
@@ -571,6 +571,7 @@ extern int fsync_object_files;
 extern int core_preload_index;
 extern int core_apply_sparse_checkout;
 extern int precomposed_unicode;
+extern int object_database_contaminated;
 
 /*
  * The character that begins a commented line in user-editable file
@@ -993,11 +994,12 @@ extern void remove_scheduled_dirs(void);
 
 extern struct alternate_object_database {
 	struct alternate_object_database *next;
+	int external;
 	char *name;
 	char base[FLEX_ARRAY]; /* more */
 } *alt_odb_list;
 extern void prepare_alt_odb(void);
-extern void read_info_alternates(const char * relative_base, int depth);
+extern void read_external_info_alternates(const char * relative_base, int depth);
 extern void add_to_alternates_file(const char *reference);
 typedef int alt_odb_fn(struct alternate_object_database *, void *);
 extern void foreach_alt_odb(alt_odb_fn, void*);
diff --git a/environment.c b/environment.c
index e2e75c1..5bed7fc 100644
--- a/environment.c
+++ b/environment.c
@@ -72,6 +72,8 @@ char comment_line_char = '#';
 /* Parallel index stat data preload? */
 int core_preload_index = 0;
 
+int object_database_contaminated;
+
 /* This is set by setup_git_dir_gently() and/or git_default_config() */
 char *git_work_tree_cfg;
 static char *work_tree;
diff --git a/sha1_file.c b/sha1_file.c
index 1a744ae..eb682b3 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -65,6 +65,8 @@ static struct cached_object empty_tree = {
 
 static struct packed_git *last_found_pack;
 
+static void read_info_alternates(const char * relative_base,
+				 int depth, int external);
 static struct cached_object *find_cached_object(const unsigned char *sha1,
 						unsigned int origin)
 {
@@ -263,7 +265,10 @@ static int git_open_noatime(const char *name);
  * SHA1, an extra slash for the first level indirection, and the
  * terminating NUL.
  */
-static int link_alt_odb_entry(const char *entry, const char *relative_base, int depth)
+static int link_alt_odb_entry(const char *entry,
+			      const char *relative_base,
+			      int depth,
+			      int external)
 {
 	const char *objdir = get_object_directory();
 	struct alternate_object_database *ent;
@@ -293,6 +298,7 @@ static int link_alt_odb_entry(const char *entry, const char *relative_base, int
 	memcpy(ent->base, pathbuf.buf, pfxlen);
 	strbuf_release(&pathbuf);
 
+	ent->external = external;
 	ent->name = ent->base + pfxlen + 1;
 	ent->base[pfxlen + 3] = '/';
 	ent->base[pfxlen] = ent->base[entlen-1] = 0;
@@ -326,15 +332,19 @@ static int link_alt_odb_entry(const char *entry, const char *relative_base, int
 	ent->next = NULL;
 
 	/* recursively add alternates */
-	read_info_alternates(ent->base, depth + 1);
+	read_info_alternates(ent->base, depth + 1, 0);
 
 	ent->base[pfxlen] = '/';
 
+	if (external)
+		object_database_contaminated = 1;
+
 	return 0;
 }
 
 static void link_alt_odb_entries(const char *alt, int len, int sep,
-				 const char *relative_base, int depth)
+				 const char *relative_base,
+				 int depth, int external)
 {
 	struct string_list entries = STRING_LIST_INIT_NODUP;
 	char *alt_copy;
@@ -356,14 +366,16 @@ static void link_alt_odb_entries(const char *alt, int len, int sep,
 			error("%s: ignoring relative alternate object store %s",
 					relative_base, entry);
 		} else {
-			link_alt_odb_entry(entry, relative_base, depth);
+			link_alt_odb_entry(entry, relative_base,
+					   depth, external);
 		}
 	}
 	string_list_clear(&entries, 0);
 	free(alt_copy);
 }
 
-void read_info_alternates(const char * relative_base, int depth)
+static void read_info_alternates(const char * relative_base,
+				 int depth, int external)
 {
 	char *map;
 	size_t mapsz;
@@ -387,11 +399,18 @@ void read_info_alternates(const char * relative_base, int depth)
 	map = xmmap(NULL, mapsz, PROT_READ, MAP_PRIVATE, fd, 0);
 	close(fd);
 
-	link_alt_odb_entries(map, mapsz, '\n', relative_base, depth);
+	link_alt_odb_entries(map, mapsz, '\n', relative_base,
+			     depth, external);
 
 	munmap(map, mapsz);
 }
 
+void read_external_info_alternates(const char *relative_base,
+				   int depth)
+{
+	read_info_alternates(relative_base, depth, 1);
+}
+
 void add_to_alternates_file(const char *reference)
 {
 	struct lock_file *lock = xcalloc(1, sizeof(struct lock_file));
@@ -401,7 +420,7 @@ void add_to_alternates_file(const char *reference)
 	if (commit_lock_file(lock))
 		die("could not close alternates file");
 	if (alt_odb_tail)
-		link_alt_odb_entries(alt, strlen(alt), '\n', NULL, 0);
+		link_alt_odb_entries(alt, strlen(alt), '\n', NULL, 0, 0);
 }
 
 void foreach_alt_odb(alt_odb_fn fn, void *cb)
@@ -431,9 +450,9 @@ void prepare_alt_odb(void)
 	if (!alt) alt = "";
 
 	alt_odb_tail = &alt_odb_list;
-	link_alt_odb_entries(alt, strlen(alt), PATH_SEP, NULL, 0);
+	link_alt_odb_entries(alt, strlen(alt), PATH_SEP, NULL, 0, 0);
 
-	read_info_alternates(get_object_directory(), 0);
+	read_info_alternates(get_object_directory(), 0, 0);
 }
 
 static int has_loose_object_extended(const unsigned char *sha1,
diff --git a/submodule.c b/submodule.c
index e728025..3cb63f7 100644
--- a/submodule.c
+++ b/submodule.c
@@ -65,7 +65,7 @@ static int add_submodule_odb(const char *path)
 	alt_odb_list = alt_odb;
 
 	/* add possible alternates from the submodule */
-	read_info_alternates(objects_directory.buf, 0);
+	read_external_info_alternates(objects_directory.buf, 0);
 	prepare_alt_odb();
 done:
 	strbuf_release(&objects_directory);
-- 
1.8.2.83.gc99314b

[PATCH v2 4/8] sha1_file: new object source for submodule's alt object database

[Nguyễn Thái Ngọc Duy]

This patch separates submodule odb sources from ordinary alternate
sources. The new sources can be accessed with ODB_EXTALT (e.g. via
read_sha1_file_extended).

ODB_EXTALT is only added to odb_default in certain cases. Basically:

 - External commands do not access submodule odb by default
 - unpack-objects, index-pack and rev-list do not
 - All other builtin commands do

unpack-objects, index-pack and rev-list take new objects from outside
and have to make sure the repository is still in good state. They
should not pay attention to submodule's odb, especially rev-list
because it does connectivity check.

External commands also do not have default access to submodule odb,
simply because I see no reasons why the should. They don't usually
play a big role in the user front, where submodule integration happens
and requires looking into submodule odb.

The die() in add_submodule_odb() may be too strong. There might be a
use case where somebody wants to add_submodule_odb() and look some up
with read_sha1_file_extended() even if odb_default does not contain
ODB_EXTALT. Right now such a use case may need to work around die() by
temporarily adding ODB_EXTALT to odb_default. Not nice, but as no such
use case exists yet to worry about.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 cache.h     |  1 +
 git.c       | 10 +++++++---
 sha1_file.c | 24 +++++++++++++++++-------
 submodule.c |  3 +++
 4 files changed, 28 insertions(+), 10 deletions(-)

diff --git a/cache.h b/cache.h
index b8d8e03..bc3ccd8 100644
--- a/cache.h
+++ b/cache.h
@@ -759,6 +759,7 @@ int offset_1st_component(const char *path);
  * function cals explicitly.
  */
 #define ODB_DEFAULT	8
+#define ODB_EXTALT	16
 
 extern unsigned int odb_default;
 
diff --git a/git.c b/git.c
index 1ada169..49a66fa 100644
--- a/git.c
+++ b/git.c
@@ -242,6 +242,7 @@ static int handle_alias(int *argcp, const char ***argv)
  * RUN_SETUP for reading from the configuration file.
  */
 #define NEED_WORK_TREE		(1<<3)
+#define NO_ODB_EXTALT		(1<<4)
 
 struct cmd_struct {
 	const char *cmd;
@@ -258,6 +259,9 @@ static int run_builtin(struct cmd_struct *p, int argc, const char **argv)
 	prefix = NULL;
 	help = argc == 2 && !strcmp(argv[1], "-h");
 	if (!help) {
+		if (!(p->option & NO_ODB_EXTALT))
+			odb_default |= ODB_EXTALT;
+
 		if (p->option & RUN_SETUP)
 			prefix = setup_git_directory();
 		if (p->option & RUN_SETUP_GENTLY) {
@@ -349,7 +353,7 @@ static void handle_internal_command(int argc, const char **argv)
 		{ "grep", cmd_grep, RUN_SETUP_GENTLY },
 		{ "hash-object", cmd_hash_object },
 		{ "help", cmd_help },
-		{ "index-pack", cmd_index_pack, RUN_SETUP_GENTLY },
+		{ "index-pack", cmd_index_pack, RUN_SETUP_GENTLY | NO_ODB_EXTALT },
 		{ "init", cmd_init_db },
 		{ "init-db", cmd_init_db },
 		{ "log", cmd_log, RUN_SETUP },
@@ -392,7 +396,7 @@ static void handle_internal_command(int argc, const char **argv)
 		{ "repo-config", cmd_repo_config, RUN_SETUP_GENTLY },
 		{ "rerere", cmd_rerere, RUN_SETUP },
 		{ "reset", cmd_reset, RUN_SETUP },
-		{ "rev-list", cmd_rev_list, RUN_SETUP },
+		{ "rev-list", cmd_rev_list, RUN_SETUP | NO_ODB_EXTALT },
 		{ "rev-parse", cmd_rev_parse },
 		{ "revert", cmd_revert, RUN_SETUP | NEED_WORK_TREE },
 		{ "rm", cmd_rm, RUN_SETUP },
@@ -408,7 +412,7 @@ static void handle_internal_command(int argc, const char **argv)
 		{ "tag", cmd_tag, RUN_SETUP },
 		{ "tar-tree", cmd_tar_tree },
 		{ "unpack-file", cmd_unpack_file, RUN_SETUP },
-		{ "unpack-objects", cmd_unpack_objects, RUN_SETUP },
+		{ "unpack-objects", cmd_unpack_objects, RUN_SETUP | NO_ODB_EXTALT },
 		{ "update-index", cmd_update_index, RUN_SETUP },
 		{ "update-ref", cmd_update_ref, RUN_SETUP },
 		{ "update-server-info", cmd_update_server_info, RUN_SETUP },
diff --git a/sha1_file.c b/sha1_file.c
index eb682b3..53f93ab 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -37,6 +37,10 @@ static inline uintmax_t sz_fmt(size_t s) { return s; }
 const unsigned char null_sha1[20];
 
 /*
+ * Some commands may not want to touch ODB_EXTALT at all. So
+ * ODB_EXTALT is only enabled later, not now, when we know which
+ * command is running.
+ *
  * clear_delta_base_cache() may be needed if odb_default is changed to
  * a narrower origin set.
  */
@@ -433,10 +437,12 @@ void foreach_alt_odb(alt_odb_fn fn, void *cb)
 			return;
 }
 
-static inline int match_origin(unsigned int origin, int pack_local)
+static inline int match_origin(unsigned int origin,
+			       struct alternate_object_database *alt)
 {
-	return (pack_local && (origin & ODB_LOCAL)) ||
-		(!pack_local && (origin & ODB_ALT));
+	return (!alt && (origin & ODB_LOCAL)) ||
+		(alt && (origin & ODB_ALT)    && !alt->external) ||
+		(alt && (origin & ODB_EXTALT) &&  alt->external);
 }
 
 void prepare_alt_odb(void)
@@ -464,10 +470,12 @@ static int has_loose_object_extended(const unsigned char *sha1,
 			return 1;
 	}
 
-	if (origin & ODB_ALT) {
+	if (origin & (ODB_ALT | ODB_EXTALT)) {
 		struct alternate_object_database *alt;
 		prepare_alt_odb();
 		for (alt = alt_odb_list; alt; alt = alt->next) {
+			if (!match_origin(origin, alt))
+				continue;
 			fill_sha1_path(alt->name, sha1);
 			if (!access(alt->base, F_OK))
 				return 1;
@@ -478,7 +486,7 @@ static int has_loose_object_extended(const unsigned char *sha1,
 
 int has_loose_object_nonlocal(const unsigned char *sha1)
 {
-	unsigned int origin = ODB_ALT;
+	unsigned int origin = ODB_ALT | ODB_EXTALT;
 	return has_loose_object_extended(sha1, origin);
 }
 
@@ -1354,7 +1362,7 @@ static int open_sha1_file(const unsigned char *sha1, unsigned int origin)
 	    (fd = git_open_noatime(name)) >= 0)
 		return fd;
 
-	if (!(origin & ODB_ALT)) {
+	if (!(origin & (ODB_ALT | ODB_EXTALT))) {
 		errno = ENOENT;
 		return -1;
 	}
@@ -1362,6 +1370,8 @@ static int open_sha1_file(const unsigned char *sha1, unsigned int origin)
 	prepare_alt_odb();
 	errno = ENOENT;
 	for (alt = alt_odb_list; alt; alt = alt->next) {
+		if (!match_origin(origin, alt))
+			continue;
 		name = alt->name;
 		fill_sha1_path(name, sha1);
 		fd = git_open_noatime(alt->base);
@@ -2336,7 +2346,7 @@ static int fill_pack_entry(const unsigned char *sha1,
 {
 	off_t offset;
 
-	if (!match_origin(origin, p->pack_local))
+	if (!match_origin(origin, p->alt))
 		return 0;
 
 	if (p->num_bad_objects) {
diff --git a/submodule.c b/submodule.c
index 3cb63f7..1271366 100644
--- a/submodule.c
+++ b/submodule.c
@@ -37,6 +37,9 @@ static int add_submodule_odb(const char *path)
 	int ret = 0;
 	const char *git_dir;
 
+	if (!(odb_default & ODB_EXTALT))
+		die("BUG: this command does not support submodule odb");
+
 	strbuf_addf(&objects_directory, "%s/.git", path);
 	git_dir = read_gitfile(objects_directory.buf);
 	if (git_dir) {
-- 
1.8.2.83.gc99314b

[PATCH v2 5/8] commit.c: refuse to write commits referring to external objects

[Nguyễn Thái Ngọc Duy]

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 cache.h     | 1 +
 commit.c    | 9 +++++++++
 sha1_file.c | 7 +++++++
 3 files changed, 17 insertions(+)

diff --git a/cache.h b/cache.h
index bc3ccd8..57b6d30 100644
--- a/cache.h
+++ b/cache.h
@@ -804,6 +804,7 @@ extern int move_temp_to_file(const char *tmpfile, const char *filename);
 
 extern int has_sha1_pack(const unsigned char *sha1);
 extern int has_sha1_file(const unsigned char *sha1);
+extern int has_sha1_file_proper(const unsigned char *sha1);
 extern int has_loose_object_nonlocal(const unsigned char *sha1);
 
 extern int has_pack_index(const unsigned char *sha1);
diff --git a/commit.c b/commit.c
index 888e02a..3edbe22 100644
--- a/commit.c
+++ b/commit.c
@@ -1343,6 +1343,10 @@ int commit_tree_extended(const struct strbuf *msg, unsigned char *tree,
 	/* Not having i18n.commitencoding is the same as having utf-8 */
 	encoding_is_utf8 = is_encoding_utf8(git_commit_encoding);
 
+	if (object_database_contaminated && !has_sha1_file_proper(tree))
+		return error(_("cannot create a commit with external tree %s"),
+			     sha1_to_hex(tree));
+
 	strbuf_init(&buffer, 8192); /* should avoid reallocs for the headers */
 	strbuf_addf(&buffer, "tree %s\n", sha1_to_hex(tree));
 
@@ -1355,6 +1359,11 @@ int commit_tree_extended(const struct strbuf *msg, unsigned char *tree,
 		struct commit_list *next = parents->next;
 		struct commit *parent = parents->item;
 
+		if (object_database_contaminated &&
+		    !has_sha1_file_proper(parent->object.sha1))
+			return error(_("cannot create a commit with external commit %s"),
+				     sha1_to_hex(parent->object.sha1));
+
 		strbuf_addf(&buffer, "parent %s\n",
 			    sha1_to_hex(parent->object.sha1));
 		free(parents);
diff --git a/sha1_file.c b/sha1_file.c
index 53f93ab..b8f2afe 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -2935,6 +2935,13 @@ int has_sha1_file(const unsigned char *sha1)
 	return has_sha1_file_extended(sha1, odb_default);
 }
 
+int has_sha1_file_proper(const unsigned char *sha1)
+{
+	if (find_cached_object(sha1, ODB_CACHED))
+		return 1;
+	return has_sha1_file_extended(sha1, ODB_LOCAL | ODB_ALT);
+}
+
 static void check_tree(const void *buf, size_t size)
 {
 	struct tree_desc desc;
-- 
1.8.2.83.gc99314b

[PATCH v2 6/8] cache-tree.c: refuse to write trees referring to external objects

[Nguyễn Thái Ngọc Duy]

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 cache-tree.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/cache-tree.c b/cache-tree.c
index 37e4d00..478da88 100644
--- a/cache-tree.c
+++ b/cache-tree.c
@@ -343,7 +343,8 @@ static int update_one(struct cache_tree *it,
 			entlen = pathlen - baselen;
 			i++;
 		}
-		if (mode != S_IFGITLINK && !missing_ok && !has_sha1_file(sha1)) {
+		if (mode != S_IFGITLINK && !missing_ok &&
+		    !has_sha1_file_proper(sha1)) {
 			strbuf_release(&buffer);
 			return error("invalid object %06o %s for '%.*s'",
 				mode, sha1_to_hex(sha1), entlen+baselen, path);
-- 
1.8.2.83.gc99314b

[PATCH v2 7/8] mktag: refuse to write tags referring to external objects

[Nguyễn Thái Ngọc Duy]

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 builtin/mktag.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/builtin/mktag.c b/builtin/mktag.c
index 640ab64..2284280 100644
--- a/builtin/mktag.c
+++ b/builtin/mktag.c
@@ -23,7 +23,10 @@ static int verify_object(const unsigned char *sha1, const char *expected_type)
 	int ret = -1;
 	enum object_type type;
 	unsigned long size;
-	void *buffer = read_sha1_file(sha1, &type, &size);
+	void *buffer = read_sha1_file_extended(sha1,
+					       ODB_CACHED | ODB_LOCAL | ODB_ALT,
+					       &type, &size,
+					       READ_SHA1_FILE_REPLACE);
 	const unsigned char *repl = lookup_replace_object(sha1);
 
 	if (buffer) {
-- 
1.8.2.83.gc99314b

[PATCH v2 8/8] sha1_file: do write objects even if found in ODB_EXTALT database

[Nguyễn Thái Ngọc Duy]

Those in ODB_EXTALT are temporary and will be gone soon. Make a
permanent copy for safety.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 sha1_file.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sha1_file.c b/sha1_file.c
index b8f2afe..ce3698a 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -2880,7 +2880,7 @@ int write_sha1_file(const void *buf, unsigned long len, const char *type, unsign
 	write_sha1_file_prepare(buf, len, type, sha1, hdr, &hdrlen);
 	if (returnsha1)
 		hashcpy(returnsha1, sha1);
-	if (has_sha1_file(sha1))
+	if (has_sha1_file_proper(sha1))
 		return 0;
 	return write_loose_object(sha1, hdr, hdrlen, buf, len, 0);
 }
-- 
1.8.2.83.gc99314b

Re: [PATCH v2 1/8] sha1_file: allow to select pack origin when looking up an object

[Eric Sunshine]

On Mon, Apr 29, 2013 at 11:42 PM, Nguyễn Thái Ngọc Duy
<pclouds@gmail.com> wrote:
> diff --git a/cache.h b/cache.h
> index 94ca1ac..bed403a 100644
> --- a/cache.h
> +++ b/cache.h
> @@ -744,12 +744,33 @@ char *strip_path_suffix(const char *path, const char *suffix);
>  int daemon_avoid_alias(const char *path);
>  int offset_1st_component(const char *path);
>
> +/*
> + * odb origin bit mask:
> + * - 8 low bits are for requested objects
> + * - 8 high bits are for their base objects
> + */
> +#define ODB_LOCAL      1
> +#define ODB_ALT                2
> +#define ODB_CACHED     4
> +/*
> + * This flag is used to track if the origin selection is from
> + * "odb_default" below. Not a real source, not to be passed to any
> + * function cals explicitly.

s/cals/calls/

> + */
> +#define ODB_DEFAULT    8
> +
> +extern unsigned int odb_default;
> +
>  /* object replacement */
>  #define READ_SHA1_FILE_REPLACE 1
> -extern void *read_sha1_file_extended(const unsigned char *sha1, enum object_type *type, unsigned long *size, unsigned flag);
> +extern void *read_sha1_file_extended(const unsigned char *sha1,
> +                                    unsigned int origin,
> +                                    enum object_type *type,
> +                                    unsigned long *size,
> +                                    unsigned flag);
>  static inline void *read_sha1_file(const unsigned char *sha1, enum object_type *type, unsigned long *size)
>  {
> -       return read_sha1_file_extended(sha1, type, size, READ_SHA1_FILE_REPLACE);
> +       return read_sha1_file_extended(sha1, odb_default, type, size, READ_SHA1_FILE_REPLACE);
>  }
>  extern const unsigned char *do_lookup_replace_object(const unsigned char *sha1);
>  static inline const unsigned char *lookup_replace_object(const unsigned char *sha1)

Re: [PATCH v2 3/8] sha1_file: mark alt object database from add_submodule_odb()

[Eric Sunshine]

On Mon, Apr 29, 2013 at 11:42 PM, Nguyễn Thái Ngọc Duy
<pclouds@gmail.com> wrote:
> A submodule's object database may be imported to in-core object pool
> for a quick peek without paying the price of running a separate git
> command. These databases are marked in for stricter checks later to

s/marked in/marked/ perhaps?

> avoid accidentially refering to a submodule's SHA-1 from main repo

s/accidentially/accidentally/
s/refering/referring/

> (except in gitlinks).
>
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>

Re: [PATCH v2 4/8] sha1_file: new object source for submodule's alt object database

[Eric Sunshine]

On Mon, Apr 29, 2013 at 11:42 PM, Nguyễn Thái Ngọc Duy
<pclouds@gmail.com> wrote:
> This patch separates submodule odb sources from ordinary alternate
> sources. The new sources can be accessed with ODB_EXTALT (e.g. via
> read_sha1_file_extended).
>
> ODB_EXTALT is only added to odb_default in certain cases. Basically:
>
>  - External commands do not access submodule odb by default
>  - unpack-objects, index-pack and rev-list do not
>  - All other builtin commands do
>
> unpack-objects, index-pack and rev-list take new objects from outside
> and have to make sure the repository is still in good state. They
> should not pay attention to submodule's odb, especially rev-list
> because it does connectivity check.
>
> External commands also do not have default access to submodule odb,
> simply because I see no reasons why the should. They don't usually

s/the should/they should/

> play a big role in the user front, where submodule integration happens
> and requires looking into submodule odb.
>
> The die() in add_submodule_odb() may be too strong. There might be a
> use case where somebody wants to add_submodule_odb() and look some up
> with read_sha1_file_extended() even if odb_default does not contain
> ODB_EXTALT. Right now such a use case may need to work around die() by
> temporarily adding ODB_EXTALT to odb_default. Not nice, but as no such

s/as// perhaps?

> use case exists yet to worry about.
>
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>

Re: [PATCH v2 0/8] Some object db protection when add_submodule_odb is used

[Thomas Rast]

Nguyễn Thái Ngọc Duy <pclouds@gmail.com> writes:

> The idea behind this series is, after add_submodule_odb, odb may have
> new temporary objects that only appear after the call. These temporary
> objects may lead to repo corruption (e.g. some new objects are created
> and point to these temporary objects). This series attempts to catch
> those cases. It would make it safer to dig deeper into submodule's odb,
> e.g. to implement unified git-diff.
>
> Previous approach [1] is record the odb source, then check if the
> source is from submodule's odb. But that means we rely on the
> lookup order in sha1_file.c. This approach instead allows the caller
> to select what odb sources it wants to look up from.
>
> The checks are also less drastic than before. Checks are now done at
> higher level, e.g. commit_tree(), instead of at write_sha1_file,
> because we do allow to write objects that point to nowhere.
>
> Another new thing from previous round is I completely forbid the use
> of add_submodule_odb in security sensitive commands like index-pack or
> rev-list. We could loosen up later if we need to.
>
> For fun, I set object_database_contaminated to 1 by default and ran
> the test suite. It passed :)

How does this interact with alternates set up by the user?  It's not
immediately obvious from the commit messages (hint hint) or the comments
near ODB_LOCAL etc.

-- 
Thomas Rast
trast@{inf,student}.ethz.ch

Re: [PATCH v2 0/8] Some object db protection when add_submodule_odb is used

[Duy Nguyen]

On Tue, Apr 30, 2013 at 3:43 PM, Thomas Rast <trast@inf.ethz.ch> wrote:
> Nguyễn Thái Ngọc Duy <pclouds@gmail.com> writes:
>
>> The idea behind this series is, after add_submodule_odb, odb may have
>> new temporary objects that only appear after the call. These temporary
>> objects may lead to repo corruption (e.g. some new objects are created
>> and point to these temporary objects). This series attempts to catch
>> those cases. It would make it safer to dig deeper into submodule's odb,
>> e.g. to implement unified git-diff.
>>
>> Previous approach [1] is record the odb source, then check if the
>> source is from submodule's odb. But that means we rely on the
>> lookup order in sha1_file.c. This approach instead allows the caller
>> to select what odb sources it wants to look up from.
>>
>> The checks are also less drastic than before. Checks are now done at
>> higher level, e.g. commit_tree(), instead of at write_sha1_file,
>> because we do allow to write objects that point to nowhere.
>>
>> Another new thing from previous round is I completely forbid the use
>> of add_submodule_odb in security sensitive commands like index-pack or
>> rev-list. We could loosen up later if we need to.
>>
>> For fun, I set object_database_contaminated to 1 by default and ran
>> the test suite. It passed :)
>
> How does this interact with alternates set up by the user?  It's not
> immediately obvious from the commit messages (hint hint) or the comments
> near ODB_LOCAL etc.

Room for improvement in the next reroll. Basically objects are put in
the following sources:

 - ODB_CACHED: builtin objects, always exist
 - ODB_LOCAL: objects in $GIT_DIR/objects
 - ODB_ALT: objects from $GIT_DIR/objects/info/alternates
 - and later ODB_EXTALT: objects added by add_submodule_odb()

Many functions now take "origin" as argument. If you pass ODB_ALT, it
will only lookup objects from $GIT_DIR/objects/info/alternates for
example, which might fail if the object does not exist there, or if
it's in a pack and some of the required bases are elsewhere.

By default, everything uses odb_default as the origin, which is
ODB_CACHED|ODB_LOCAL|ODB_ALT (for this patch), so nothing is really
changed regarding alternates setup.
--
Duy