{"thread":{"id":"41986","subject":"[PATCH 3/3] index-helper: take extra care with readlink","startedAt":"2016-04-10T23:03:39Z","lastAt":"2016-04-11T21:53:05Z","messageCount":3,"participants":["Ramsay Jones","Torsten Bögershausen","David Turner"],"isPatch":true,"patchVersion":1,"patchTotal":3},"messages":[{"id":"283116","messageId":"570ADBCB.4040503@ramsayjones.plus.com","threadId":"41986","inReplyTo":null,"subject":"[PATCH 3/3] index-helper: take extra care with readlink","fromName":"Ramsay Jones","fromEmail":"ramsay@ramsayjones.plus.com","sentAt":"2016-04-10T23:03:39Z","receivedAt":"2016-04-10T23:03:39Z","isPatch":true,"sender":{"key":"ramsay@ramsayjones.plus.com","avatar":"https://avatars.githubusercontent.com/u/33702710?v=4"},"body":"\nIt took me a few minutes to convince myself that, if index-helper is\nthe only writer for the symlink, that the call to readlink would\nresult in a properly NULL terminated string. This relies on the\ninitialization of the address variable setting each byte of the\n'struct sockaddr_un' to zero and the contents of the symlink being\nno more than 'sizeof(address.sun_path) - 1' bytes long.\n\nIn order to make the call easier to reason about, introduce a wrapper\nfunction, read_link(), which copes with overlong symlinks and ensures\ncorrect NULL termination.\n\nSigned-off-by: Ramsay Jones <ramsay@ramsayjones.plus.com>\n---\n read-cache.c | 13 ++++++++++++-\n 1 file changed, 12 insertions(+), 1 deletion(-)\n\ndiff --git a/read-cache.c b/read-cache.c\nindex c7053d8..39330a1 100644\n--- a/read-cache.c\n+++ b/read-cache.c\n@@ -1722,12 +1722,23 @@ static void post_read_index_from(struct index_state *istate)\n \ttweak_untracked_cache(istate);\n }\n \n+static int read_link(const char *path, char *buf, size_t bufsize)\n+{\n+\tint len;\n+\n+\tlen = readlink(path, buf, bufsize);\n+\tif (len < 0 || len >= bufsize)\n+\t\treturn -1;\n+\tbuf[len] = '\\0';\n+\treturn 0;\n+}\n+\n int connect_to_index_helper(void)\n {\n \tstruct sockaddr_un address = {0};\n \tint fd;\n \n-\tif (readlink(git_path(\"index-helper.path\"), address.sun_path,\n+\tif (read_link(git_path(\"index-helper.path\"), address.sun_path,\n \t\t     sizeof(address.sun_path)) < 0)\n \t\treturn -1;\n \n-- \n2.8.0\n"},{"id":"283126","messageId":"570B2554.1010105@web.de","threadId":"41986","inReplyTo":"570ADBCB.4040503@ramsayjones.plus.com","subject":"Re: [PATCH 3/3] index-helper: take extra care with readlink","fromName":"Torsten Bögershausen","fromEmail":"tboegi@web.de","sentAt":"2016-04-11T04:17:24Z","receivedAt":"2016-04-11T04:17:24Z","isPatch":true,"sender":{"key":"tboegi@web.de","avatar":"https://avatars.githubusercontent.com/u/7138363?v=4"},"body":"On 04/11/2016 01:03 AM, Ramsay Jones wrote:\n> It took me a few minutes to convince myself that, if index-helper is\n> the only writer for the symlink, that the call to readlink would\n> result in a properly NULL terminated string. This relies on the\n>\nMinor nit:\ns/NULL/NUL/\n"},{"id":"283158","messageId":"1460411585.5540.21.camel@twopensource.com","threadId":"41986","inReplyTo":"570ADBCB.4040503@ramsayjones.plus.com","subject":"Re: [PATCH 3/3] index-helper: take extra care with readlink","fromName":"David Turner","fromEmail":"dturner@twopensource.com","sentAt":"2016-04-11T21:53:05Z","receivedAt":"2016-04-11T21:53:05Z","isPatch":true,"sender":{"key":"novalis@novalis.org","avatar":"https://avatars.githubusercontent.com/u/77003?v=4"},"body":"On Mon, 2016-04-11 at 00:03 +0100, Ramsay Jones wrote:\n> It took me a few minutes to convince myself that, if index-helper is\n> the only writer for the symlink, that the call to readlink would\n> result in a properly NULL terminated string. This relies on the\n> initialization of the address variable setting each byte of the\n> 'struct sockaddr_un' to zero and the contents of the symlink being\n> no more than 'sizeof(address.sun_path) - 1' bytes long.\n> \n> In order to make the call easier to reason about, introduce a wrapper\n> function, read_link(), which copes with overlong symlinks and ensures\n> correct NULL termination.\n> \n> Signed-off-by: Ramsay Jones <ramsay@ramsayjones.plus.com>\n> ---\n>  read-cache.c | 13 ++++++++++++-\n>  1 file changed, 12 insertions(+), 1 deletion(-)\n> \n> diff --git a/read-cache.c b/read-cache.c\n> index c7053d8..39330a1 100644\n> --- a/read-cache.c\n> +++ b/read-cache.c\n> @@ -1722,12 +1722,23 @@ static void post_read_index_from(struct\n> index_state *istate)\n>  \ttweak_untracked_cache(istate);\n>  }\n>  \n> +static int read_link(const char *path, char *buf, size_t bufsize)\n> +{\n> +\tint len;\n> +\n> +\tlen = readlink(path, buf, bufsize);\n> +\tif (len < 0 || len >= bufsize)\n> +\t\treturn -1;\n> +\tbuf[len] = '\\0';\n> +\treturn 0;\n> +}\n> +\n>  int connect_to_index_helper(void)\n>  {\n>  \tstruct sockaddr_un address = {0};\n>  \tint fd;\n>  \n> -\tif (readlink(git_path(\"index-helper.path\"),\n> address.sun_path,\n> +\tif (read_link(git_path(\"index-helper.path\"),\n> address.sun_path,\n>  \t\t     sizeof(address.sun_path)) < 0)\n>  \t\treturn -1;\n>  \n\n\nWill squash.\n"}]}