From 9055384710dd8963b125f4f87c24d8f67d9fa24f Mon Sep 17 00:00:00 2001 From: Jeff King Date: Wed, 31 Jul 2019 00:38:15 -0400 Subject: tree-walk: drop oid from traverse_info As the previous commit shows, the presence of an oid in each level of the traverse_info is confusing and ultimately not necessary. Let's drop it to make it clear that it will not always be set (as well as convince us that it's unused, and let the compiler catch any merges with other branches that do add new uses). Since the oid is part of name_entry, we'll actually stop embedding a name_entry entirely, and instead just separately hold the pathname, its length, and the mode. This makes the resulting code slightly more verbose as we have to pass those elements around individually. But it also makes it more clear what each code path is going to use (and in most of the paths, we really only care about the pathname itself). A few of these conversions are noisier than they need to be, as they also take the opportunity to rename "len" to "namelen" for clarity (especially where we also have "pathlen" or "ce_len" alongside). Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- tree-walk.h | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) (limited to 'tree-walk.h') diff --git a/tree-walk.h b/tree-walk.h index 161e2400f4..baa2aa62c7 100644 --- a/tree-walk.h +++ b/tree-walk.h @@ -56,7 +56,10 @@ enum get_oid_result get_tree_entry_follow_symlinks(struct object_id *tree_oid, c struct traverse_info { const char *traverse_path; struct traverse_info *prev; - struct name_entry name; + const char *name; + size_t namelen; + unsigned mode; + int pathlen; struct pathspec *pathspec; @@ -67,7 +70,8 @@ struct traverse_info { }; int get_tree_entry(const struct object_id *, const char *, struct object_id *, unsigned short *); -char *make_traverse_path(char *path, const struct traverse_info *info, const struct name_entry *n); +char *make_traverse_path(char *path, const struct traverse_info *info, + const char *name, size_t namelen); void setup_traverse_info(struct traverse_info *info, const char *base); static inline int traverse_path_len(const struct traverse_info *info, const struct name_entry *n) -- cgit v1.2.3 From 37806080d7be1ab5b2fa918f6a528652596ea2c1 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Wed, 31 Jul 2019 00:38:18 -0400 Subject: tree-walk: use size_t consistently We store and manipulate the cumulative traverse_info.pathlen as an "int", which can overflow when we are fed ridiculously long pathnames (e.g., ones at the edge of 2GB or 4GB, even if the individual tree entry names are smaller than that). The results can be confusing, though after some prodding I was not able to use this integer overflow to cause an under-allocated buffer. Let's consistently use size_t to generate and store these, and make sure our addition doesn't overflow. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- tree-walk.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'tree-walk.h') diff --git a/tree-walk.h b/tree-walk.h index baa2aa62c7..47bf85d282 100644 --- a/tree-walk.h +++ b/tree-walk.h @@ -60,7 +60,7 @@ struct traverse_info { size_t namelen; unsigned mode; - int pathlen; + size_t pathlen; struct pathspec *pathspec; unsigned long df_conflicts; @@ -74,9 +74,9 @@ char *make_traverse_path(char *path, const struct traverse_info *info, const char *name, size_t namelen); void setup_traverse_info(struct traverse_info *info, const char *base); -static inline int traverse_path_len(const struct traverse_info *info, const struct name_entry *n) +static inline size_t traverse_path_len(const struct traverse_info *info, const struct name_entry *n) { - return info->pathlen + tree_entry_len(n); + return st_add(info->pathlen, tree_entry_len(n)); } /* in general, positive means "kind of interesting" */ -- cgit v1.2.3 From b3b3cbcbf246b1051ad453bc02e24a89573e2911 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Wed, 31 Jul 2019 00:38:20 -0400 Subject: tree-walk: accept a raw length for traverse_path_len() We take a "struct name_entry", but only care about the length of the path name. Let's just take that length directly, making it easier to use the function from callers that sometimes do not have a name_entry at all. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- tree-walk.h | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) (limited to 'tree-walk.h') diff --git a/tree-walk.h b/tree-walk.h index 47bf85d282..a25c751c1e 100644 --- a/tree-walk.h +++ b/tree-walk.h @@ -74,9 +74,10 @@ char *make_traverse_path(char *path, const struct traverse_info *info, const char *name, size_t namelen); void setup_traverse_info(struct traverse_info *info, const char *base); -static inline size_t traverse_path_len(const struct traverse_info *info, const struct name_entry *n) +static inline size_t traverse_path_len(const struct traverse_info *info, + size_t namelen) { - return st_add(info->pathlen, tree_entry_len(n)); + return st_add(info->pathlen, namelen); } /* in general, positive means "kind of interesting" */ -- cgit v1.2.3 From c43ab062598d0299ea6e0d115a6018189a7793bf Mon Sep 17 00:00:00 2001 From: Jeff King Date: Wed, 31 Jul 2019 00:38:23 -0400 Subject: tree-walk: add a strbuf wrapper for make_traverse_path() All but one of the callers of make_traverse_path() allocate a new heap buffer to store the path. Let's give them an easy way to write to a strbuf, which saves them from computing the length themselves (which is especially tricky when they want to add to the path). It will also make it easier for us to change the make_traverse_path() interface in a future patch to improve its bounds-checking. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- tree-walk.h | 3 +++ 1 file changed, 3 insertions(+) (limited to 'tree-walk.h') diff --git a/tree-walk.h b/tree-walk.h index a25c751c1e..994c14a499 100644 --- a/tree-walk.h +++ b/tree-walk.h @@ -72,6 +72,9 @@ struct traverse_info { int get_tree_entry(const struct object_id *, const char *, struct object_id *, unsigned short *); char *make_traverse_path(char *path, const struct traverse_info *info, const char *name, size_t namelen); +void strbuf_make_traverse_path(struct strbuf *out, + const struct traverse_info *info, + const char *name, size_t namelen); void setup_traverse_info(struct traverse_info *info, const char *base); static inline size_t traverse_path_len(const struct traverse_info *info, -- cgit v1.2.3 From 5aa02f98685d78666293149087d3f69b97528cfb Mon Sep 17 00:00:00 2001 From: Jeff King Date: Wed, 31 Jul 2019 00:38:25 -0400 Subject: tree-walk: harden make_traverse_path() length computations The make_traverse_path() function isn't very careful about checking its output buffer boundaries. In fact, it doesn't even _know_ the size of the buffer it's writing to, and just assumes that the caller used traverse_path_len() correctly. And even then we assume that our traverse_info.pathlen components are all correct, and just blindly write into the buffer. Let's improve this situation a bit: - have the caller pass in their allocated buffer length, which we'll check against our own computations - check for integer underflow as we do our backwards-insertion of pathnames into the buffer - check that we do not run out items in our list to traverse before we've filled the expected number of bytes None of these should be triggerable in practice (especially since our switch to size_t everywhere in a previous commit), but it doesn't hurt to check our assumptions. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- tree-walk.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'tree-walk.h') diff --git a/tree-walk.h b/tree-walk.h index 994c14a499..a3ad54e6ce 100644 --- a/tree-walk.h +++ b/tree-walk.h @@ -70,7 +70,7 @@ struct traverse_info { }; int get_tree_entry(const struct object_id *, const char *, struct object_id *, unsigned short *); -char *make_traverse_path(char *path, const struct traverse_info *info, +char *make_traverse_path(char *path, size_t pathlen, const struct traverse_info *info, const char *name, size_t namelen); void strbuf_make_traverse_path(struct strbuf *out, const struct traverse_info *info, -- cgit v1.2.3