[PATCH] mkostemp: fix implementation
Anthony G. Basile
basile at opensource.dyc.edu
Sun Dec 7 01:52:09 UTC 2014
On 10/27/14 16:13, basile at opensource.dyc.edu wrote:
> From: "Anthony G. Basile" <blueness at gentoo.org>
>
> mkostemp(char *template, int flags) generates a unique temporary
> filename from a template. The flags parameter accepts three of
> the same flags as open(2): O_APPEND, O_CLOEXEC, and O_SYNC. The
> current implementation of mkostemp(3) does not respect the flags
> and in fact confuses the flags with the file mode which should
> always be S_IRUSR | S_IWUSR. This patch corrects this issue.
Can someone please review this. Thanks.
>
> Signed-off-by: Anthony G. Basile <blueness at gentoo.org>
> ---
> libc/inet/getaddrinfo.c | 2 +-
> libc/misc/internals/tempname.c | 6 +++---
> libc/misc/internals/tempname.h | 2 +-
> libc/stdio/tempnam.c | 2 +-
> libc/stdio/tmpfile.c | 2 +-
> libc/stdio/tmpnam.c | 2 +-
> libc/stdio/tmpnam_r.c | 2 +-
> libc/stdlib/mkdtemp.c | 2 +-
> libc/stdlib/mkostemp.c | 4 +++-
> libc/stdlib/mkostemp64.c | 2 +-
> libc/stdlib/mkstemp.c | 2 +-
> libc/stdlib/mkstemp64.c | 2 +-
> libc/stdlib/mktemp.c | 2 +-
> libpthread/nptl/sem_open.c | 2 +-
> test/.gitignore | 2 ++
> test/stdlib/test-mkostemp-O_CLOEXEC.c | 40 +++++++++++++++++++++++++++++++++++
> test/stdlib/test-mkostemp-child.c | 22 +++++++++++++++++++
> 17 files changed, 82 insertions(+), 16 deletions(-)
> create mode 100644 test/stdlib/test-mkostemp-O_CLOEXEC.c
> create mode 100644 test/stdlib/test-mkostemp-child.c
>
> diff --git a/libc/inet/getaddrinfo.c b/libc/inet/getaddrinfo.c
> index b61d69c..168adb1 100644
> --- a/libc/inet/getaddrinfo.c
> +++ b/libc/inet/getaddrinfo.c
> @@ -308,7 +308,7 @@ gaih_local(const char *name, const struct gaih_service *service,
> char *buf = ((struct sockaddr_un *)ai->ai_addr)->sun_path;
>
> if (__path_search(buf, L_tmpnam, NULL, NULL, 0) != 0
> - || __gen_tempname(buf, __GT_NOCREATE, 0) != 0
> + || __gen_tempname(buf, __GT_NOCREATE, 0, 0) != 0
> ) {
> return -EAI_SYSTEM;
> }
> diff --git a/libc/misc/internals/tempname.c b/libc/misc/internals/tempname.c
> index 18fd823..edcc31c 100644
> --- a/libc/misc/internals/tempname.c
> +++ b/libc/misc/internals/tempname.c
> @@ -177,7 +177,7 @@ static void brain_damaged_fillrand(unsigned char *buf, unsigned int len)
> __GT_DIR: create a directory with given mode.
>
> */
> -int attribute_hidden __gen_tempname (char *tmpl, int kind, mode_t mode)
> +int attribute_hidden __gen_tempname (char *tmpl, int kind, int flags, mode_t mode)
> {
> char *XXXXXX;
> unsigned int i;
> @@ -219,11 +219,11 @@ int attribute_hidden __gen_tempname (char *tmpl, int kind, mode_t mode)
> fd = 0;
> }
> case __GT_FILE:
> - fd = open (tmpl, O_RDWR | O_CREAT | O_EXCL, mode);
> + fd = open (tmpl, O_RDWR | O_CREAT | O_EXCL | flags, mode);
> break;
> #if defined __UCLIBC_HAS_LFS__
> case __GT_BIGFILE:
> - fd = open64 (tmpl, O_RDWR | O_CREAT | O_EXCL, mode);
> + fd = open64 (tmpl, O_RDWR | O_CREAT | O_EXCL | flags, mode);
> break;
> #endif
> case __GT_DIR:
> diff --git a/libc/misc/internals/tempname.h b/libc/misc/internals/tempname.h
> index e75b632..edfe26d 100644
> --- a/libc/misc/internals/tempname.h
> +++ b/libc/misc/internals/tempname.h
> @@ -10,7 +10,7 @@ extern int ___path_search (char *tmpl, size_t tmpl_len, const char *dir,
> const char *pfx /*, int try_tmpdir */) attribute_hidden;
> #define __path_search(tmpl, tmpl_len, dir, pfx, try_tmpdir) ___path_search(tmpl, tmpl_len, dir, pfx)
>
> -extern int __gen_tempname (char *__tmpl, int __kind, mode_t mode) attribute_hidden;
> +extern int __gen_tempname (char *__tmpl, int __kind, int flags, mode_t mode) attribute_hidden;
>
> /* The __kind argument to __gen_tempname may be one of: */
> #define __GT_FILE 0 /* create a file */
> diff --git a/libc/stdio/tempnam.c b/libc/stdio/tempnam.c
> index 232ed02..74bb26e 100644
> --- a/libc/stdio/tempnam.c
> +++ b/libc/stdio/tempnam.c
> @@ -35,7 +35,7 @@ tempnam (const char *dir, const char *pfx)
> if (__path_search (buf, FILENAME_MAX, dir, pfx, 1))
> return NULL;
>
> - if (__gen_tempname (buf, __GT_NOCREATE, 0))
> + if (__gen_tempname (buf, __GT_NOCREATE, 0, 0))
> return NULL;
>
> return strdup (buf);
> diff --git a/libc/stdio/tmpfile.c b/libc/stdio/tmpfile.c
> index a9bf474..83c85b5 100644
> --- a/libc/stdio/tmpfile.c
> +++ b/libc/stdio/tmpfile.c
> @@ -35,7 +35,7 @@ FILE * tmpfile (void)
>
> if (__path_search (buf, FILENAME_MAX, NULL, "tmpf", 0))
> return NULL;
> - fd = __gen_tempname (buf, __GT_FILE, S_IRUSR | S_IWUSR);
> + fd = __gen_tempname (buf, __GT_FILE, 0, S_IRUSR | S_IWUSR);
> if (fd < 0)
> return NULL;
>
> diff --git a/libc/stdio/tmpnam.c b/libc/stdio/tmpnam.c
> index 88b9bff..ffed862 100644
> --- a/libc/stdio/tmpnam.c
> +++ b/libc/stdio/tmpnam.c
> @@ -40,7 +40,7 @@ tmpnam (char *s)
> 0))
> return NULL;
>
> - if (__builtin_expect (__gen_tempname (tmpbuf, __GT_NOCREATE, 0), 0))
> + if (__builtin_expect (__gen_tempname (tmpbuf, __GT_NOCREATE, 0, 0), 0))
> return NULL;
>
> if (s == NULL)
> diff --git a/libc/stdio/tmpnam_r.c b/libc/stdio/tmpnam_r.c
> index 8cde84d..bfd60a4 100644
> --- a/libc/stdio/tmpnam_r.c
> +++ b/libc/stdio/tmpnam_r.c
> @@ -27,7 +27,7 @@ char * tmpnam_r (char *s)
>
> if (__path_search (s, L_tmpnam, NULL, NULL, 0))
> return NULL;
> - if (__gen_tempname (s, __GT_NOCREATE, 0))
> + if (__gen_tempname (s, __GT_NOCREATE, 0, 0))
> return NULL;
>
> return s;
> diff --git a/libc/stdlib/mkdtemp.c b/libc/stdlib/mkdtemp.c
> index da7598a..e6d4a36 100644
> --- a/libc/stdlib/mkdtemp.c
> +++ b/libc/stdlib/mkdtemp.c
> @@ -29,7 +29,7 @@
> (This function comes from OpenBSD.) */
> char * mkdtemp (char *template)
> {
> - if (__gen_tempname (template, __GT_DIR, S_IRUSR | S_IWUSR | S_IXUSR))
> + if (__gen_tempname (template, __GT_DIR, 0, S_IRUSR | S_IWUSR | S_IXUSR))
> return NULL;
> else
> return template;
> diff --git a/libc/stdlib/mkostemp.c b/libc/stdlib/mkostemp.c
> index 0369235..912be30 100644
> --- a/libc/stdlib/mkostemp.c
> +++ b/libc/stdlib/mkostemp.c
> @@ -17,6 +17,7 @@
>
> #include <stdio.h>
> #include <stdlib.h>
> +#include <fcntl.h>
> #include "../misc/internals/tempname.h"
>
> /* Generate a unique temporary file name from TEMPLATE.
> @@ -26,5 +27,6 @@
> int
> mkostemp (char *template, int flags)
> {
> - return __gen_tempname (template, __GT_FILE, flags);
> + flags -= flags & O_ACCMODE; /* Remove O_RDONLY, O_WRONLY, and O_RDWR. */
> + return __gen_tempname (template, __GT_FILE, flags, S_IRUSR | S_IWUSR);
> }
> diff --git a/libc/stdlib/mkostemp64.c b/libc/stdlib/mkostemp64.c
> index d21def5..c6d6d84 100644
> --- a/libc/stdlib/mkostemp64.c
> +++ b/libc/stdlib/mkostemp64.c
> @@ -27,5 +27,5 @@
> int
> mkostemp64 (char *template, int flags)
> {
> - return __gen_tempname (template, __GT_BIGFILE, flags | O_LARGEFILE);
> + return __gen_tempname (template, __GT_BIGFILE, flags | O_LARGEFILE, S_IRUSR | S_IWUSR | S_IXUSR);
> }
> diff --git a/libc/stdlib/mkstemp.c b/libc/stdlib/mkstemp.c
> index 61c7175..a3a1595 100644
> --- a/libc/stdlib/mkstemp.c
> +++ b/libc/stdlib/mkstemp.c
> @@ -26,5 +26,5 @@
> Then open the file and return a fd. */
> int mkstemp (char *template)
> {
> - return __gen_tempname (template, __GT_FILE, S_IRUSR | S_IWUSR);
> + return __gen_tempname (template, __GT_FILE, 0, S_IRUSR | S_IWUSR);
> }
> diff --git a/libc/stdlib/mkstemp64.c b/libc/stdlib/mkstemp64.c
> index e29be2d..6f2ee3e 100644
> --- a/libc/stdlib/mkstemp64.c
> +++ b/libc/stdlib/mkstemp64.c
> @@ -26,5 +26,5 @@
> Then open the file and return a fd. */
> int mkstemp64 (char *template)
> {
> - return __gen_tempname (template, __GT_BIGFILE, S_IRUSR | S_IWUSR);
> + return __gen_tempname (template, __GT_BIGFILE, 0, S_IRUSR | S_IWUSR);
> }
> diff --git a/libc/stdlib/mktemp.c b/libc/stdlib/mktemp.c
> index edd001d..1ff93da 100644
> --- a/libc/stdlib/mktemp.c
> +++ b/libc/stdlib/mktemp.c
> @@ -24,7 +24,7 @@
> * they are replaced with a string that makes the filename unique. */
> char *mktemp(char *template)
> {
> - if (__gen_tempname (template, __GT_NOCREATE, 0) < 0)
> + if (__gen_tempname (template, __GT_NOCREATE, 0, 0) < 0)
> /* We return the null string if we can't find a unique file name. */
> template[0] = '\0';
>
> diff --git a/libpthread/nptl/sem_open.c b/libpthread/nptl/sem_open.c
> index 1b36164..3a72079 100644
> --- a/libpthread/nptl/sem_open.c
> +++ b/libpthread/nptl/sem_open.c
> @@ -336,7 +336,7 @@ sem_open (const char *name, int oflag, ...)
> mempcpy (mempcpy (tmpfname, mountpoint.dir, mountpoint.dirlen),
> "XXXXXX", 7);
>
> - fd = __gen_tempname (tmpfname, __GT_FILE, mode);
> + fd = __gen_tempname (tmpfname, __GT_FILE, 0, mode);
> if (fd == -1)
> return SEM_FAILED;
>
> diff --git a/test/.gitignore b/test/.gitignore
> index 8f32031..5944f0a 100644
> --- a/test/.gitignore
> +++ b/test/.gitignore
> @@ -274,6 +274,8 @@ stdlib/testarc4random
> stdlib/testatexit
> stdlib/test-canon
> stdlib/test-canon2
> +stdlib/test-mkostemp-O_CLOEXEC
> +stdlib/test-mkostemp-child
> stdlib/teston_exit
> stdlib/teststrtol
> stdlib/teststrtoq
> diff --git a/test/stdlib/test-mkostemp-O_CLOEXEC.c b/test/stdlib/test-mkostemp-O_CLOEXEC.c
> new file mode 100644
> index 0000000..5652086
> --- /dev/null
> +++ b/test/stdlib/test-mkostemp-O_CLOEXEC.c
> @@ -0,0 +1,40 @@
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <string.h>
> +#include <unistd.h>
> +#include <sys/types.h>
> +#include <sys/stat.h>
> +#include <fcntl.h>
> +#include <sys/wait.h>
> +#include <errno.h>
> +
> +int main(int argc, char *argv[]) {
> + int fd, status;
> + char buff[5];
> + char template[] = "/tmp/test-mkostemp.XXXXXX";
> +
> + fd = mkostemp(template, O_CLOEXEC);
> + unlink(template);
> +
> + snprintf(buff, 5, "%d", fd);
> +
> + if(!fork())
> + if(execl("./test-mkostemp-child", "test-mkostemp-child", buff, NULL) == -1)
> + exit(EXIT_FAILURE);
> +
> + wait(&status);
> +
> + memset(buff, 0, 5);
> + lseek(fd, 0, SEEK_SET);
> + errno = 0;
> + if(read(fd, buff, 5) == -1)
> + exit(EXIT_FAILURE);
> +
> + if(!strncmp(buff, "test", 5))
> + exit(EXIT_FAILURE);
> + else
> + exit(EXIT_SUCCESS);
> +
> + close(fd);
> + exit(EXIT_SUCCESS);
> +}
> diff --git a/test/stdlib/test-mkostemp-child.c b/test/stdlib/test-mkostemp-child.c
> new file mode 100644
> index 0000000..708bd80
> --- /dev/null
> +++ b/test/stdlib/test-mkostemp-child.c
> @@ -0,0 +1,22 @@
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <unistd.h>
> +
> +int main(int argc, char *argv[]) {
> + int fd;
> +
> + /* This file gets built and run as a test, but its
> + * really just a helper for test-mkostemp-O_CLOEXEC.c.
> + * So, we'll always return succcess.
> + */
> + if(argc != 2)
> + exit(EXIT_SUCCESS);
> +
> + sscanf(argv[1], "%d", &fd);
> +
> + if(write(fd, "test\0", 5) == -1)
> + ; /* Don't Panic! Failure is okay here. */
> +
> + close(fd);
> + exit(EXIT_SUCCESS);
> +}
>
--
Anthony G. Basile, Ph. D.
Chair of Information Technology
D'Youville College
Buffalo, NY 14201
(716) 829-8197
More information about the uClibc
mailing list