[PATCH 3/3] index-helper: take extra care with readlink

classic Classic list List threaded Threaded
3 messages Options
Reply | Threaded
Open this post in threaded view
|

[PATCH 3/3] index-helper: take extra care with readlink

Ramsay Jones-2

It took me a few minutes to convince myself that, if index-helper is
the only writer for the symlink, that the call to readlink would
result in a properly NULL terminated string. This relies on the
initialization of the address variable setting each byte of the
'struct sockaddr_un' to zero and the contents of the symlink being
no more than 'sizeof(address.sun_path) - 1' bytes long.

In order to make the call easier to reason about, introduce a wrapper
function, read_link(), which copes with overlong symlinks and ensures
correct NULL termination.

Signed-off-by: Ramsay Jones <[hidden email]>
---
 read-cache.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/read-cache.c b/read-cache.c
index c7053d8..39330a1 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -1722,12 +1722,23 @@ static void post_read_index_from(struct index_state *istate)
  tweak_untracked_cache(istate);
 }
 
+static int read_link(const char *path, char *buf, size_t bufsize)
+{
+ int len;
+
+ len = readlink(path, buf, bufsize);
+ if (len < 0 || len >= bufsize)
+ return -1;
+ buf[len] = '\0';
+ return 0;
+}
+
 int connect_to_index_helper(void)
 {
  struct sockaddr_un address = {0};
  int fd;
 
- if (readlink(git_path("index-helper.path"), address.sun_path,
+ if (read_link(git_path("index-helper.path"), address.sun_path,
      sizeof(address.sun_path)) < 0)
  return -1;
 
--
2.8.0
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 3/3] index-helper: take extra care with readlink

Torsten Bögershausen-2
On 04/11/2016 01:03 AM, Ramsay Jones wrote:
> It took me a few minutes to convince myself that, if index-helper is
> the only writer for the symlink, that the call to readlink would
> result in a properly NULL terminated string. This relies on the
>
Minor nit:
s/NULL/NUL/

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 3/3] index-helper: take extra care with readlink

David Turner
In reply to this post by Ramsay Jones-2
On Mon, 2016-04-11 at 00:03 +0100, Ramsay Jones wrote:

> It took me a few minutes to convince myself that, if index-helper is
> the only writer for the symlink, that the call to readlink would
> result in a properly NULL terminated string. This relies on the
> initialization of the address variable setting each byte of the
> 'struct sockaddr_un' to zero and the contents of the symlink being
> no more than 'sizeof(address.sun_path) - 1' bytes long.
>
> In order to make the call easier to reason about, introduce a wrapper
> function, read_link(), which copes with overlong symlinks and ensures
> correct NULL termination.
>
> Signed-off-by: Ramsay Jones <[hidden email]>
> ---
>  read-cache.c | 13 ++++++++++++-
>  1 file changed, 12 insertions(+), 1 deletion(-)
>
> diff --git a/read-cache.c b/read-cache.c
> index c7053d8..39330a1 100644
> --- a/read-cache.c
> +++ b/read-cache.c
> @@ -1722,12 +1722,23 @@ static void post_read_index_from(struct
> index_state *istate)
>   tweak_untracked_cache(istate);
>  }
>  
> +static int read_link(const char *path, char *buf, size_t bufsize)
> +{
> + int len;
> +
> + len = readlink(path, buf, bufsize);
> + if (len < 0 || len >= bufsize)
> + return -1;
> + buf[len] = '\0';
> + return 0;
> +}
> +
>  int connect_to_index_helper(void)
>  {
>   struct sockaddr_un address = {0};
>   int fd;
>  
> - if (readlink(git_path("index-helper.path"),
> address.sun_path,
> + if (read_link(git_path("index-helper.path"),
> address.sun_path,
>       sizeof(address.sun_path)) < 0)
>   return -1;
>  


Will squash.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html