[PATCH] readlink.2: Document using st_size to allocate the buffer

[PATCH] readlink.2: Document using st_size to allocate the buffer

[Guillem Jover]

Signed-off-by: Guillem Jover <guillem-+FW4gsLVM0RAfugRpC6u6w@public.gmane.org>
---
 man2/readlink.2 |   69 ++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 files changed, 68 insertions(+), 1 deletions(-)

diff --git a/man2/readlink.2 b/man2/readlink.2
index 1327e08..2601e22 100644
--- a/man2/readlink.2
+++ b/man2/readlink.2
@@ -35,7 +35,7 @@
 .\" Modified Tue Jul  9 23:55:17 1996 by aeb
 .\" Modified Fri Jan 24 00:26:00 1997 by aeb
 .\"
-.TH READLINK 2 2010-09-20 "Linux" "Linux Programmer's Manual"
+.TH READLINK 2 2011-04-20 "Linux" "Linux Programmer's Manual"
 .SH NAME
 readlink \- read value of a symbolic link
 .SH SYNOPSIS
@@ -69,6 +69,21 @@ does not append a null byte to
 It will truncate the contents (to a length of
 .I bufsiz
 characters), in case the buffer is too small to hold all of the contents.
+
+Using a statically sized buffer might not give enough room for the
+symbolic link contents, which might end up getting truncated. The
+needed size for the buffer can be obtained from the symbolic link's
+.I stat
+structure
+.I st_size
+field with
+.BR lstat (),
+but the number of written characters should be checked to make sure the
+symbolic link did not change between both calls, and no truncation
+happened. This also fixes a common portability problem when using
+.I PATH_MAX
+for the buffer size, as this is not guaranteed to be defined per POSIX
+if the system does not have such limit.
 .SH "RETURN VALUE"
 On success,
 .BR readlink ()
@@ -130,6 +145,58 @@ was declared as
 Nowadays, the return type is declared as
 .IR ssize_t ,
 as (newly) required in POSIX.1-2001.
+.SH EXAMPLE
+The following program allocates the space needed by
+.BR readlink ()
+dynamically from the information provided by
+.BR lstat (),
+making sure there's no race condition between both calls.
+.nf
+
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <unistd.h>
+
+int
+main(int argc, char *argv[])
+{
+    struct stat sb;
+    char *linkname;
+    ssize_t r;
+
+    if (argc != 2) {
+        fprintf(stderr, "Usage: %s <pathname>\\n", argv[0]);
+        exit(EXIT_FAILURE);
+    }
+
+    if (lstat(argv[1], &sb) == \-1) {
+        perror("lstat");
+        exit(EXIT_FAILURE);
+    }
+
+    linkname = malloc(sb.st_size + 1);
+    if (linkname == NULL) {
+        fprintf(stderr, "insufficient memory\\n");
+        exit(EXIT_FAILURE);
+    }
+
+    r = readlink(argv[1], linkname, sb.st_size + 1);
+    if (r < 0) {
+        perror("lstat");
+        exit(EXIT_FAILURE);
+    } else if (r != sb.st_size) {
+        fprintf(stderr, "symlink changed between lstat and readlink\\n");
+        exit(EXIT_FAILURE);
+    }
+    linkname[sb.st_size] = '\\0';
+
+    printf("'%s' points to '%s'\\n", argv[1], linkname);
+
+    exit(EXIT_SUCCESS);
+}
+.fi
 .SH "SEE ALSO"
 .BR lstat (2),
 .BR readlinkat (2),
-- 
1.7.4.4

--
To unsubscribe from this list: send the line "unsubscribe linux-man" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] readlink.2: Document using st_size to allocate the buffer

[Michael Kerrisk]

Hello Guillem,

On Thu, Apr 21, 2011 at 5:19 AM, Guillem Jover <guillem-+FW4gsLVM0RAfugRpC6u6w@public.gmane.org> wrote:
> Signed-off-by: Guillem Jover <guillem-+FW4gsLVM0RAfugRpC6u6w@public.gmane.org>
> ---
>  man2/readlink.2 |   69 ++++++++++++++++++++++++++++++++++++++++++++++++++++++-
>  1 files changed, 68 insertions(+), 1 deletions(-)
>
> diff --git a/man2/readlink.2 b/man2/readlink.2
> index 1327e08..2601e22 100644
> --- a/man2/readlink.2
> +++ b/man2/readlink.2
> @@ -35,7 +35,7 @@
>  .\" Modified Tue Jul  9 23:55:17 1996 by aeb
>  .\" Modified Fri Jan 24 00:26:00 1997 by aeb
>  .\"
> -.TH READLINK 2 2010-09-20 "Linux" "Linux Programmer's Manual"
> +.TH READLINK 2 2011-04-20 "Linux" "Linux Programmer's Manual"
>  .SH NAME
>  readlink \- read value of a symbolic link
>  .SH SYNOPSIS
> @@ -69,6 +69,21 @@ does not append a null byte to
>  It will truncate the contents (to a length of
>  .I bufsiz
>  characters), in case the buffer is too small to hold all of the contents.
> +
> +Using a statically sized buffer might not give enough room for the
> +symbolic link contents, which might end up getting truncated. The
> +needed size for the buffer can be obtained from the symbolic link's
> +.I stat
> +structure
> +.I st_size
> +field with
> +.BR lstat (),
> +but the number of written characters should be checked to make sure the
> +symbolic link did not change between both calls, and no truncation
> +happened. This also fixes a common portability problem when using
> +.I PATH_MAX
> +for the buffer size, as this is not guaranteed to be defined per POSIX
> +if the system does not have such limit.
>  .SH "RETURN VALUE"
>  On success,
>  .BR readlink ()
> @@ -130,6 +145,58 @@ was declared as
>  Nowadays, the return type is declared as
>  .IR ssize_t ,
>  as (newly) required in POSIX.1-2001.
> +.SH EXAMPLE
> +The following program allocates the space needed by
> +.BR readlink ()
> +dynamically from the information provided by
> +.BR lstat (),
> +making sure there's no race condition between both calls.
> +.nf
> +
> +#include <sys/types.h>
> +#include <sys/stat.h>
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <unistd.h>
> +
> +int
> +main(int argc, char *argv[])
> +{
> +    struct stat sb;
> +    char *linkname;
> +    ssize_t r;
> +
> +    if (argc != 2) {
> +        fprintf(stderr, "Usage: %s <pathname>\\n", argv[0]);
> +        exit(EXIT_FAILURE);
> +    }
> +
> +    if (lstat(argv[1], &sb) == \-1) {
> +        perror("lstat");
> +        exit(EXIT_FAILURE);
> +    }
> +
> +    linkname = malloc(sb.st_size + 1);
> +    if (linkname == NULL) {
> +        fprintf(stderr, "insufficient memory\\n");
> +        exit(EXIT_FAILURE);
> +    }
> +
> +    r = readlink(argv[1], linkname, sb.st_size + 1);
> +    if (r < 0) {
> +        perror("lstat");
> +        exit(EXIT_FAILURE);
> +    } else if (r != sb.st_size) {
> +        fprintf(stderr, "symlink changed between lstat and readlink\\n");
> +        exit(EXIT_FAILURE);
> +    }
> +    linkname[sb.st_size] = '\\0';
> +
> +    printf("'%s' points to '%s'\\n", argv[1], linkname);
> +
> +    exit(EXIT_SUCCESS);
> +}
> +.fi
>  .SH "SEE ALSO"
>  .BR lstat (2),
>  .BR readlinkat (2),
> --
> 1.7.4.4

That's a nice addition! Thanks!

I made some edits to your patch, and placed the added explanatory text
in NOTES, rather than DESCRIPTION. Modified patch below.

The change will be in 3.34.

Cheers,

Michael

diff --git a/man2/readlink.2 b/man2/readlink.2
index c9f6d92..051a870 100644
--- a/man2/readlink.2
+++ b/man2/readlink.2
@@ -1,4 +1,5 @@
 .\" Copyright (c) 1983, 1991 The Regents of the University of California.
+.\" Adn Copyright (C) 2011 Guillem Jover <guillem-+FW4gsLVM0RAfugRpC6u6w@public.gmane.org>
 .\" All rights reserved.
 .\"
 .\" Redistribution and use in source and binary forms, with or without
@@ -34,8 +35,10 @@
 .\" Modified Sat Jul 24 00:10:21 1993 by Rik Faith (faith-+5Oa3zvhR2o3uPMLIKxrzw@public.gmane.org)
 .\" Modified Tue Jul  9 23:55:17 1996 by aeb
 .\" Modified Fri Jan 24 00:26:00 1997 by aeb
+.\" 2011-09-20, Guillem Jover <guillem-+FW4gsLVM0RAfugRpC6u6w@public.gmane.org>:
+.\"     Added text on dynamically allocating buffer + example program
 .\"
-.TH READLINK 2 2010-09-20 "Linux" "Linux Programmer's Manual"
+.TH READLINK 2 2011-09-20 "Linux" "Linux Programmer's Manual"
 .SH NAME
 readlink \- read value of a symbolic link
 .SH SYNOPSIS
@@ -130,6 +133,82 @@ was declared as
 Nowadays, the return type is declared as
 .IR ssize_t ,
 as (newly) required in POSIX.1-2001.
+
+Using a statically sized buffer might not provide enough room for the
+symbolic link contents.
+The required size for the buffer can be obtained from the
+.I stat.st_size
+value returned by a call to
+.BR lstat (2)
+on the link.
+However, the number of bytes written by
+.BR readlink ()
+should be checked to make sure that the size of the
+symbolic link did not increase between the calls.
+Dynamically allocating the buffer for
+.BR readlink ()
+also addresses a common portability problem when using
+.I PATH_MAX
+for the buffer size,
+as this constant is not guaranteed to be defined per POSIX
+if the system does not have such limit.
+.SH EXAMPLE
+The following program allocates the buffer needed by
+.BR readlink ()
+dynamically from the information provided by
+.BR lstat (),
+making sure there's no race condition between the calls.
+.nf
+
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <unistd.h>
+
+int
+main(int argc, char *argv[])
+{
+    struct stat sb;
+    char *linkname;
+    ssize_t r;
+
+    if (argc != 2) {
+        fprintf(stderr, "Usage: %s <pathname>\\n", argv[0]);
+        exit(EXIT_FAILURE);
+    }
+
+    if (lstat(argv[1], &sb) == \-1) {
+        perror("lstat");
+        exit(EXIT_FAILURE);
+    }
+
+    linkname = malloc(sb.st_size + 1);
+    if (linkname == NULL) {
+        fprintf(stderr, "insufficient memory\\n");
+        exit(EXIT_FAILURE);
+    }
+
+    r = readlink(argv[1], linkname, sb.st_size + 1);
+
+    if (r < 0) {
+        perror("lstat");
+        exit(EXIT_FAILURE);
+    }
+
+    if (r != sb.st_size) {
+        fprintf(stderr, "symlink increased in size "
+                        "between lstat() and readlink()\\n");
+        exit(EXIT_FAILURE);
+    }
+
+    linkname[sb.st_size] = '\\0';
+
+    printf("'%s' points to '%s'\\n", argv[1], linkname);
+
+    exit(EXIT_SUCCESS);
+}
+.fi
 .SH "SEE ALSO"
 .BR readlink (1),
 .BR lstat (2),


-- 
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Author of "The Linux Programming Interface"; http://man7.org/tlpi/
--
To unsubscribe from this list: send the line "unsubscribe linux-man" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] readlink.2: Document using st_size to allocate the buffer

[Guillem Jover]

Hi!

On Tue, 2011-09-20 at 08:51:41 +0200, Michael Kerrisk wrote:
> I made some edits to your patch, and placed the added explanatory text
> in NOTES, rather than DESCRIPTION. Modified patch below.
> 
> The change will be in 3.34.

Thanks! Looks good.

> diff --git a/man2/readlink.2 b/man2/readlink.2
> index c9f6d92..051a870 100644
> --- a/man2/readlink.2
> +++ b/man2/readlink.2
> @@ -1,4 +1,5 @@
>  .\" Copyright (c) 1983, 1991 The Regents of the University of California.
> +.\" Adn Copyright (C) 2011 Guillem Jover <guillem-+FW4gsLVM0RAfugRpC6u6w@public.gmane.org>

Small typo: Adn → And.

> +    r = readlink(argv[1], linkname, sb.st_size + 1);
> +
> +    if (r < 0) {
> +        perror("lstat");
> +        exit(EXIT_FAILURE);
> +    }
> +
> +    if (r != sb.st_size) {
> +        fprintf(stderr, "symlink increased in size "
> +                        "between lstat() and readlink()\\n");
> +        exit(EXIT_FAILURE);
> +    }

You might want to change the check condition to “r > sb.st_size” to
match the new error string.

thanks,
guillem
--
To unsubscribe from this list: send the line "unsubscribe linux-man" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] readlink.2: Document using st_size to allocate the buffer

[Michael Kerrisk]

Hi Guillem

On Sun, Sep 25, 2011 at 6:05 AM, Guillem Jover <guillem-+FW4gsLVM0RAfugRpC6u6w@public.gmane.org> wrote:
> Hi!
>
> On Tue, 2011-09-20 at 08:51:41 +0200, Michael Kerrisk wrote:
>> I made some edits to your patch, and placed the added explanatory text
>> in NOTES, rather than DESCRIPTION. Modified patch below.
>>
>> The change will be in 3.34.
>
> Thanks! Looks good.
>
>> diff --git a/man2/readlink.2 b/man2/readlink.2
>> index c9f6d92..051a870 100644
>> --- a/man2/readlink.2
>> +++ b/man2/readlink.2
>> @@ -1,4 +1,5 @@
>>  .\" Copyright (c) 1983, 1991 The Regents of the University of California.
>> +.\" Adn Copyright (C) 2011 Guillem Jover <guillem-+FW4gsLVM0RAfugRpC6u6w@public.gmane.org>
>
> Small typo: Adn → And.
>
>> +    r = readlink(argv[1], linkname, sb.st_size + 1);
>> +
>> +    if (r < 0) {
>> +        perror("lstat");
>> +        exit(EXIT_FAILURE);
>> +    }
>> +
>> +    if (r != sb.st_size) {
>> +        fprintf(stderr, "symlink increased in size "
>> +                        "between lstat() and readlink()\\n");
>> +        exit(EXIT_FAILURE);
>> +    }
>
> You might want to change the check condition to “r > sb.st_size” to
> match the new error string.

Thanks for checking that. Fixed now in git.

Cheers,

Michael
-- 
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Author of "The Linux Programming Interface"; http://man7.org/tlpi/
--
To unsubscribe from this list: send the line "unsubscribe linux-man" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html