mailing list of musl libc
 help / color / mirror / code / Atom feed
* [PATCH] Add support for mkostemp, mkstemps and mkostemps
@ 2013-01-28  5:06 Anthony G. Basile
  2013-01-28  8:47 ` John Spencer
                   ` (3 more replies)
  0 siblings, 4 replies; 27+ messages in thread
From: Anthony G. Basile @ 2013-01-28  5:06 UTC (permalink / raw)
  To: musl; +Cc: Anthony G. Basile, Anthony G. Basile

From: "Anthony G. Basile" <basile@opensource.dyc.edu>

Signed-off-by: Anthony G. Basile <blueness@gentoo.org>
---
 include/stdlib.h     |  6 ++++++
 src/temp/mkostemp.c  | 18 ++++++++++++++++++
 src/temp/mkostemps.c | 18 ++++++++++++++++++
 src/temp/mkstemp.c   | 15 ++-------------
 src/temp/mkstemps.c  | 18 ++++++++++++++++++
 src/temp/mktemp.c    |  7 +++----
 src/temp/randname.c  | 22 ++++++++++++++++++++++
 src/temp/tempfile.c  | 42 ++++++++++++++++++++++++++++++++++++++++++
 8 files changed, 129 insertions(+), 17 deletions(-)
 create mode 100644 src/temp/mkostemp.c
 create mode 100644 src/temp/mkostemps.c
 create mode 100644 src/temp/mkstemps.c
 create mode 100644 src/temp/randname.c
 create mode 100644 src/temp/tempfile.c

diff --git a/include/stdlib.h b/include/stdlib.h
index 671d188..4210f40 100644
--- a/include/stdlib.h
+++ b/include/stdlib.h
@@ -95,6 +95,9 @@ int posix_memalign (void **, size_t, size_t);
 int setenv (const char *, const char *, int);
 int unsetenv (const char *);
 int mkstemp (char *);
+int mkostemp (char *, int);
+int mkstemps (char *, int);
+int mkostemps (char *, int, int);
 char *mkdtemp (char *);
 int getsubopt (char **, char *const *, char **);
 int rand_r (unsigned *);
@@ -150,6 +153,9 @@ char *gcvt(double, int, char *);
 
 #if defined(_LARGEFILE64_SOURCE) || defined(_GNU_SOURCE)
 #define mkstemp64 mkstemp
+#define mkostemp64 mkostemp
+#define mkstemps64 mkstemps
+#define mkostemps64 mkostemps
 #endif
 
 #ifdef __cplusplus
diff --git a/src/temp/mkostemp.c b/src/temp/mkostemp.c
new file mode 100644
index 0000000..750d880
--- /dev/null
+++ b/src/temp/mkostemp.c
@@ -0,0 +1,18 @@
+#define _GNU_SOURCE
+#include <string.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <fcntl.h>
+#include <unistd.h>
+#include <limits.h>
+#include <errno.h>
+#include "libc.h"
+
+int __open_tempfile (char *, int, int);
+
+int __mkostemp(char *template, int flags)
+{
+	return __open_tempfile (template, 0, flags);
+}
+
+weak_alias(__mkostemp, mkostemp);
diff --git a/src/temp/mkostemps.c b/src/temp/mkostemps.c
new file mode 100644
index 0000000..8c810ce
--- /dev/null
+++ b/src/temp/mkostemps.c
@@ -0,0 +1,18 @@
+#define _GNU_SOURCE
+#include <string.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <fcntl.h>
+#include <unistd.h>
+#include <limits.h>
+#include <errno.h>
+#include "libc.h"
+
+int __open_tempfile (char *, int, int);
+
+int __mkostemps(char *template, int len, int flags)
+{
+	return __open_tempfile (template, len, flags);
+}
+
+weak_alias(__mkostemps, mkostemps);
diff --git a/src/temp/mkstemp.c b/src/temp/mkstemp.c
index a390d42..ccaf3c6 100644
--- a/src/temp/mkstemp.c
+++ b/src/temp/mkstemp.c
@@ -7,22 +7,11 @@
 #include <errno.h>
 #include "libc.h"
 
-char *__mktemp(char *);
+int __open_tempfile (char *, int, int);
 
 int mkstemp(char *template)
 {
-	int fd, retries = 100, t0 = *template;
-	while (retries--) {
-		if (!*__mktemp(template)) return -1;
-		if ((fd = open(template, O_RDWR | O_CREAT | O_EXCL, 0600))>=0)
-			return fd;
-		if (errno != EEXIST) return -1;
-		/* this is safe because mktemp verified
-		 * that we have a valid template string */
-		template[0] = t0;
-		strcpy(template+strlen(template)-6, "XXXXXX");
-	}
-	return -1;
+	return __open_tempfile (template, 0, O_RDWR);
 }
 
 LFS64(mkstemp);
diff --git a/src/temp/mkstemps.c b/src/temp/mkstemps.c
new file mode 100644
index 0000000..53fea07
--- /dev/null
+++ b/src/temp/mkstemps.c
@@ -0,0 +1,18 @@
+#define _GNU_SOURCE
+#include <string.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <fcntl.h>
+#include <unistd.h>
+#include <limits.h>
+#include <errno.h>
+#include "libc.h"
+
+int __open_tempfile (char *, int, int);
+
+int __mkstemps(char *template, int len)
+{
+	return __open_tempfile (template, len, O_RDWR);
+}
+
+weak_alias(__mkstemps, mkstemps);
diff --git a/src/temp/mktemp.c b/src/temp/mktemp.c
index c0e06f5..de1afb4 100644
--- a/src/temp/mktemp.c
+++ b/src/temp/mktemp.c
@@ -8,6 +8,8 @@
 #include <stdint.h>
 #include "libc.h"
 
+char *__randname(char *);
+
 char *__mktemp(char *template)
 {
 	struct timespec ts;
@@ -21,10 +23,7 @@ char *__mktemp(char *template)
 		return template;
 	}
 	while (retries--) {
-		clock_gettime(CLOCK_REALTIME, &ts);
-		r = ts.tv_nsec + (uintptr_t)&ts / 16 + (uintptr_t)template;
-		for (i=1; i<=6; i++, r>>=4)
-			template[l-i] = 'A'+(r&15);
+		__randname(template);
 		if (access(template, F_OK) < 0) return template;
 	}
 	*template = 0;
diff --git a/src/temp/randname.c b/src/temp/randname.c
new file mode 100644
index 0000000..4d3476f
--- /dev/null
+++ b/src/temp/randname.c
@@ -0,0 +1,22 @@
+#include <string.h>
+#include <unistd.h>
+#include <errno.h>
+#include <time.h>
+#include <stdint.h>
+#include "libc.h"
+
+char *__randname(char *template)
+{
+	struct timespec ts;
+	size_t i, l = strlen(template);
+	unsigned long r;
+
+	/* This assumes that a check for the template
+	   size has alrady been made */
+	clock_gettime(CLOCK_REALTIME, &ts);
+	r = ts.tv_nsec + (uintptr_t)&ts / 16 + (uintptr_t)template;
+	for (i=1; i<=6; i++, r>>=4)
+		template[l-i] = 'A'+(r&15);
+
+	return template;
+}
diff --git a/src/temp/tempfile.c b/src/temp/tempfile.c
new file mode 100644
index 0000000..93808a6
--- /dev/null
+++ b/src/temp/tempfile.c
@@ -0,0 +1,42 @@
+#include <string.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <fcntl.h>
+#include <unistd.h>
+#include <limits.h>
+#include <errno.h>
+#include "libc.h"
+
+char *__randname(char *);
+
+int __open_tempfile (char *template, int len, int flags)
+{
+	if (len < 0) return EINVAL;
+
+	int l = strlen(template)-len;
+	if (l < 6 || strncmp(template+l-6, "XXXXXX",6)) {
+		errno = EINVAL;
+		*template = 0;
+		return -1;
+	}
+
+	/* Null terminate the template before the suffix,
+	   and save the char for adding back the suffix */
+	char suffix = template[l];
+	template[l] = '\0';
+
+	int fd, retries = 100, t0 = *template;
+	while (retries--) {
+		if (!*__randname(template)) return -1;
+		/* Add back the suffix */
+		template[l] = suffix;
+		if ((fd = open(template, flags | O_CREAT | O_EXCL, 0600))>=0)
+			return fd;
+		if (errno != EEXIST) return -1;
+		/* this is safe because mktemp verified
+		 * that we have a valid template string */
+		template[0] = t0;
+		strcpy(template+l-6, "XXXXXX");
+	}
+	return -1;
+}
-- 
1.7.12.4



^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH] Add support for mkostemp, mkstemps and mkostemps
  2013-01-28  5:06 [PATCH] Add support for mkostemp, mkstemps and mkostemps Anthony G. Basile
@ 2013-01-28  8:47 ` John Spencer
  2013-01-28  9:37 ` Szabolcs Nagy
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 27+ messages in thread
From: John Spencer @ 2013-01-28  8:47 UTC (permalink / raw)
  To: musl

On 01/28/2013 06:06 AM, Anthony G. Basile wrote:

looks better; however i'd prefer if the __randname would get the strlen 
passed (or even the string pointer directly to the 6 bytes in question) 
from the caller to save some cpu cycles (and code bytes).

btw, are you sure that the declarations in stdlib.h are inside a 
_GNU_SOURCE block ? haven't checked but it doesnt look like it in your 
patch.

another thing: the strcpy of XXXXXX in __open_tempfile doesnt seem to 
make much sense to me - why should there be a need to restore the 
original template ?
(at least it is not needed if you pass the pointer to those 6 bytes 
directly to __randname)
> From: "Anthony G. Basile"<basile@opensource.dyc.edu>
>
> Signed-off-by: Anthony G. Basile<blueness@gentoo.org>
> ---
>   include/stdlib.h     |  6 ++++++
>   src/temp/mkostemp.c  | 18 ++++++++++++++++++
>   src/temp/mkostemps.c | 18 ++++++++++++++++++
>   src/temp/mkstemp.c   | 15 ++-------------
>   src/temp/mkstemps.c  | 18 ++++++++++++++++++
>   src/temp/mktemp.c    |  7 +++----
>   src/temp/randname.c  | 22 ++++++++++++++++++++++
>   src/temp/tempfile.c  | 42 ++++++++++++++++++++++++++++++++++++++++++
>   8 files changed, 129 insertions(+), 17 deletions(-)
>   create mode 100644 src/temp/mkostemp.c
>   create mode 100644 src/temp/mkostemps.c
>   create mode 100644 src/temp/mkstemps.c
>   create mode 100644 src/temp/randname.c
>   create mode 100644 src/temp/tempfile.c
>
> diff --git a/include/stdlib.h b/include/stdlib.h
> index 671d188..4210f40 100644
> --- a/include/stdlib.h
> +++ b/include/stdlib.h
> @@ -95,6 +95,9 @@ int posix_memalign (void **, size_t, size_t);
>   int setenv (const char *, const char *, int);
>   int unsetenv (const char *);
>   int mkstemp (char *);
> +int mkostemp (char *, int);
> +int mkstemps (char *, int);
> +int mkostemps (char *, int, int);
>   char *mkdtemp (char *);
>   int getsubopt (char **, char *const *, char **);
>   int rand_r (unsigned *);
> @@ -150,6 +153,9 @@ char *gcvt(double, int, char *);
>
>   #if defined(_LARGEFILE64_SOURCE) || defined(_GNU_SOURCE)
>   #define mkstemp64 mkstemp
> +#define mkostemp64 mkostemp
> +#define mkstemps64 mkstemps
> +#define mkostemps64 mkostemps
>   #endif
>
>   #ifdef __cplusplus
> diff --git a/src/temp/mkostemp.c b/src/temp/mkostemp.c
> new file mode 100644
> index 0000000..750d880
> --- /dev/null
> +++ b/src/temp/mkostemp.c
> @@ -0,0 +1,18 @@
> +#define _GNU_SOURCE
> +#include<string.h>
> +#include<stdio.h>
> +#include<stdlib.h>
> +#include<fcntl.h>
> +#include<unistd.h>
> +#include<limits.h>
> +#include<errno.h>
> +#include "libc.h"
> +
> +int __open_tempfile (char *, int, int);
> +
> +int __mkostemp(char *template, int flags)
> +{
> +	return __open_tempfile (template, 0, flags);
> +}
> +
> +weak_alias(__mkostemp, mkostemp);
> diff --git a/src/temp/mkostemps.c b/src/temp/mkostemps.c
> new file mode 100644
> index 0000000..8c810ce
> --- /dev/null
> +++ b/src/temp/mkostemps.c
> @@ -0,0 +1,18 @@
> +#define _GNU_SOURCE
> +#include<string.h>
> +#include<stdio.h>
> +#include<stdlib.h>
> +#include<fcntl.h>
> +#include<unistd.h>
> +#include<limits.h>
> +#include<errno.h>
> +#include "libc.h"
> +
> +int __open_tempfile (char *, int, int);
> +
> +int __mkostemps(char *template, int len, int flags)
> +{
> +	return __open_tempfile (template, len, flags);
> +}
> +
> +weak_alias(__mkostemps, mkostemps);
> diff --git a/src/temp/mkstemp.c b/src/temp/mkstemp.c
> index a390d42..ccaf3c6 100644
> --- a/src/temp/mkstemp.c
> +++ b/src/temp/mkstemp.c
> @@ -7,22 +7,11 @@
>   #include<errno.h>
>   #include "libc.h"
>
> -char *__mktemp(char *);
> +int __open_tempfile (char *, int, int);
>
>   int mkstemp(char *template)
>   {
> -	int fd, retries = 100, t0 = *template;
> -	while (retries--) {
> -		if (!*__mktemp(template)) return -1;
> -		if ((fd = open(template, O_RDWR | O_CREAT | O_EXCL, 0600))>=0)
> -			return fd;
> -		if (errno != EEXIST) return -1;
> -		/* this is safe because mktemp verified
> -		 * that we have a valid template string */
> -		template[0] = t0;
> -		strcpy(template+strlen(template)-6, "XXXXXX");
> -	}
> -	return -1;
> +	return __open_tempfile (template, 0, O_RDWR);
>   }
>
>   LFS64(mkstemp);
> diff --git a/src/temp/mkstemps.c b/src/temp/mkstemps.c
> new file mode 100644
> index 0000000..53fea07
> --- /dev/null
> +++ b/src/temp/mkstemps.c
> @@ -0,0 +1,18 @@
> +#define _GNU_SOURCE
> +#include<string.h>
> +#include<stdio.h>
> +#include<stdlib.h>
> +#include<fcntl.h>
> +#include<unistd.h>
> +#include<limits.h>
> +#include<errno.h>
> +#include "libc.h"
> +
> +int __open_tempfile (char *, int, int);
> +
> +int __mkstemps(char *template, int len)
> +{
> +	return __open_tempfile (template, len, O_RDWR);
> +}
> +
> +weak_alias(__mkstemps, mkstemps);
> diff --git a/src/temp/mktemp.c b/src/temp/mktemp.c
> index c0e06f5..de1afb4 100644
> --- a/src/temp/mktemp.c
> +++ b/src/temp/mktemp.c
> @@ -8,6 +8,8 @@
>   #include<stdint.h>
>   #include "libc.h"
>
> +char *__randname(char *);
> +
>   char *__mktemp(char *template)
>   {
>   	struct timespec ts;
> @@ -21,10 +23,7 @@ char *__mktemp(char *template)
>   		return template;
>   	}
>   	while (retries--) {
> -		clock_gettime(CLOCK_REALTIME,&ts);
> -		r = ts.tv_nsec + (uintptr_t)&ts / 16 + (uintptr_t)template;
> -		for (i=1; i<=6; i++, r>>=4)
> -			template[l-i] = 'A'+(r&15);
> +		__randname(template);
>   		if (access(template, F_OK)<  0) return template;
>   	}
>   	*template = 0;
> diff --git a/src/temp/randname.c b/src/temp/randname.c
> new file mode 100644
> index 0000000..4d3476f
> --- /dev/null
> +++ b/src/temp/randname.c
> @@ -0,0 +1,22 @@
> +#include<string.h>
> +#include<unistd.h>
> +#include<errno.h>
> +#include<time.h>
> +#include<stdint.h>
> +#include "libc.h"
> +
> +char *__randname(char *template)
> +{
> +	struct timespec ts;
> +	size_t i, l = strlen(template);
> +	unsigned long r;
> +
> +	/* This assumes that a check for the template
> +	   size has alrady been made */
> +	clock_gettime(CLOCK_REALTIME,&ts);
> +	r = ts.tv_nsec + (uintptr_t)&ts / 16 + (uintptr_t)template;
> +	for (i=1; i<=6; i++, r>>=4)
> +		template[l-i] = 'A'+(r&15);
> +
> +	return template;
> +}
> diff --git a/src/temp/tempfile.c b/src/temp/tempfile.c
> new file mode 100644
> index 0000000..93808a6
> --- /dev/null
> +++ b/src/temp/tempfile.c
> @@ -0,0 +1,42 @@
> +#include<string.h>
> +#include<stdio.h>
> +#include<stdlib.h>
> +#include<fcntl.h>
> +#include<unistd.h>
> +#include<limits.h>
> +#include<errno.h>
> +#include "libc.h"
> +
> +char *__randname(char *);
> +
> +int __open_tempfile (char *template, int len, int flags)
> +{
> +	if (len<  0) return EINVAL;
> +
> +	int l = strlen(template)-len;
> +	if (l<  6 || strncmp(template+l-6, "XXXXXX",6)) {
> +		errno = EINVAL;
> +		*template = 0;
> +		return -1;
> +	}
> +
> +	/* Null terminate the template before the suffix,
> +	   and save the char for adding back the suffix */
> +	char suffix = template[l];
> +	template[l] = '\0';
> +
> +	int fd, retries = 100, t0 = *template;
> +	while (retries--) {
> +		if (!*__randname(template)) return -1;
> +		/* Add back the suffix */
> +		template[l] = suffix;
> +		if ((fd = open(template, flags | O_CREAT | O_EXCL, 0600))>=0)
> +			return fd;
> +		if (errno != EEXIST) return -1;
> +		/* this is safe because mktemp verified
> +		 * that we have a valid template string */
> +		template[0] = t0;
> +		strcpy(template+l-6, "XXXXXX");
> +	}
> +	return -1;
> +}



^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH] Add support for mkostemp, mkstemps and mkostemps
  2013-01-28  5:06 [PATCH] Add support for mkostemp, mkstemps and mkostemps Anthony G. Basile
  2013-01-28  8:47 ` John Spencer
@ 2013-01-28  9:37 ` Szabolcs Nagy
  2013-01-28 17:33   ` Szabolcs Nagy
  2013-01-29 23:16   ` Anthony G. Basile
  2013-01-28 16:33 ` Rich Felker
  2013-02-02  3:48 ` Rich Felker
  3 siblings, 2 replies; 27+ messages in thread
From: Szabolcs Nagy @ 2013-01-28  9:37 UTC (permalink / raw)
  To: musl

* Anthony G. Basile <basile@opensource.dyc.edu> [2013-01-28 00:06:23 -0500]:
> +#include <string.h>
> +#include <unistd.h>
> +#include <errno.h>
> +#include <time.h>
> +#include <stdint.h>
> +#include "libc.h"

libc.h, errno.h, unistd.h are not used

> +
> +char *__randname(char *template)
> +{
> +	struct timespec ts;
> +	size_t i, l = strlen(template);
> +	unsigned long r;

can be unsigned char r;

> +
> +	/* This assumes that a check for the template
> +	   size has alrady been made */
> +	clock_gettime(CLOCK_REALTIME, &ts);
> +	r = ts.tv_nsec + (uintptr_t)&ts / 16 + (uintptr_t)template;
> +	for (i=1; i<=6; i++, r>>=4)
> +		template[l-i] = 'A'+(r&15);

it seems to use only 4bit entropy based on clock (and fixed addresses)

and (uintptr_t)template does not give much entropy if it's malloced

and if the clock source is bad for some reason (eg cpu clock mod 16ns
is even) then this might not even give different result for each retry
(may be it helps if retry count is included)

> +
> +	return template;
> +}
> diff --git a/src/temp/tempfile.c b/src/temp/tempfile.c

i'd use different name, eg __tempfile.c to signify that it's internal

> new file mode 100644
> index 0000000..93808a6
> --- /dev/null
> +++ b/src/temp/tempfile.c
> @@ -0,0 +1,42 @@
> +#include <string.h>
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <fcntl.h>
> +#include <unistd.h>
> +#include <limits.h>
> +#include <errno.h>
> +#include "libc.h"

stdlib.h, limits.h are not used

libc.h is not used
(in the other files it is needed for weak_alias)

> +
> +char *__randname(char *);
> +
> +int __open_tempfile (char *template, int len, int flags)
> +{
> +	if (len < 0) return EINVAL;
> +
> +	int l = strlen(template)-len;

int vs size_t problem

> +	if (l < 6 || strncmp(template+l-6, "XXXXXX",6)) {
> +		errno = EINVAL;
> +		*template = 0;
> +		return -1;
> +	}
> +
> +	/* Null terminate the template before the suffix,
> +	   and save the char for adding back the suffix */
> +	char suffix = template[l];
> +	template[l] = '\0';
> +

i'd do __randname(template, length-suffix)
so pass the template len as an argument

then no suffix saving is needed

> +	int fd, retries = 100, t0 = *template;

100 retries does not make sense if only 4bit entropy is used
(there are 16 different names for the given addresses)

> +	while (retries--) {
> +		if (!*__randname(template)) return -1;
> +		/* Add back the suffix */
> +		template[l] = suffix;
> +		if ((fd = open(template, flags | O_CREAT | O_EXCL, 0600))>=0)
> +			return fd;
> +		if (errno != EEXIST) return -1;
> +		/* this is safe because mktemp verified
> +		 * that we have a valid template string */
> +		template[0] = t0;

i dont understand why template[0] gets clobbered

> +		strcpy(template+l-6, "XXXXXX");

i'd use memcpy or memset to avoid suffix saving

> +	}
> +	return -1;
> +}
> -- 
> 1.7.12.4


^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH] Add support for mkostemp, mkstemps and mkostemps
  2013-01-28  5:06 [PATCH] Add support for mkostemp, mkstemps and mkostemps Anthony G. Basile
  2013-01-28  8:47 ` John Spencer
  2013-01-28  9:37 ` Szabolcs Nagy
@ 2013-01-28 16:33 ` Rich Felker
  2013-02-02  3:48 ` Rich Felker
  3 siblings, 0 replies; 27+ messages in thread
From: Rich Felker @ 2013-01-28 16:33 UTC (permalink / raw)
  To: musl

On Mon, Jan 28, 2013 at 12:06:23AM -0500, Anthony G. Basile wrote:
> From: "Anthony G. Basile" <basile@opensource.dyc.edu>
> 
> Signed-off-by: Anthony G. Basile <blueness@gentoo.org>
> ---
>  include/stdlib.h     |  6 ++++++
>  src/temp/mkostemp.c  | 18 ++++++++++++++++++
>  src/temp/mkostemps.c | 18 ++++++++++++++++++
>  src/temp/mkstemp.c   | 15 ++-------------
>  src/temp/mkstemps.c  | 18 ++++++++++++++++++
>  src/temp/mktemp.c    |  7 +++----
>  src/temp/randname.c  | 22 ++++++++++++++++++++++
>  src/temp/tempfile.c  | 42 ++++++++++++++++++++++++++++++++++++++++++
>  8 files changed, 129 insertions(+), 17 deletions(-)
>  create mode 100644 src/temp/mkostemp.c
>  create mode 100644 src/temp/mkostemps.c
>  create mode 100644 src/temp/mkstemps.c
>  create mode 100644 src/temp/randname.c
>  create mode 100644 src/temp/tempfile.c
> 
> diff --git a/include/stdlib.h b/include/stdlib.h
> index 671d188..4210f40 100644
> --- a/include/stdlib.h
> +++ b/include/stdlib.h
> @@ -95,6 +95,9 @@ int posix_memalign (void **, size_t, size_t);
>  int setenv (const char *, const char *, int);
>  int unsetenv (const char *);
>  int mkstemp (char *);
> +int mkostemp (char *, int);
> +int mkstemps (char *, int);
> +int mkostemps (char *, int, int);
>  char *mkdtemp (char *);
>  int getsubopt (char **, char *const *, char **);
>  int rand_r (unsigned *);
> @@ -150,6 +153,9 @@ char *gcvt(double, int, char *);
>  
>  #if defined(_LARGEFILE64_SOURCE) || defined(_GNU_SOURCE)
>  #define mkstemp64 mkstemp
> +#define mkostemp64 mkostemp
> +#define mkstemps64 mkstemps
> +#define mkostemps64 mkostemps
>  #endif
>  
>  #ifdef __cplusplus
> diff --git a/src/temp/mkostemp.c b/src/temp/mkostemp.c
> new file mode 100644
> index 0000000..750d880
> --- /dev/null
> +++ b/src/temp/mkostemp.c
> @@ -0,0 +1,18 @@
> +#define _GNU_SOURCE
> +#include <string.h>
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <fcntl.h>
> +#include <unistd.h>
> +#include <limits.h>
> +#include <errno.h>
> +#include "libc.h"
> +
> +int __open_tempfile (char *, int, int);
> +
> +int __mkostemp(char *template, int flags)
> +{
> +	return __open_tempfile (template, 0, flags);
> +}
> +
> +weak_alias(__mkostemp, mkostemp);
> diff --git a/src/temp/mkostemps.c b/src/temp/mkostemps.c
> new file mode 100644
> index 0000000..8c810ce
> --- /dev/null
> +++ b/src/temp/mkostemps.c
> @@ -0,0 +1,18 @@
> +#define _GNU_SOURCE
> +#include <string.h>
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <fcntl.h>
> +#include <unistd.h>
> +#include <limits.h>
> +#include <errno.h>
> +#include "libc.h"
> +
> +int __open_tempfile (char *, int, int);
> +
> +int __mkostemps(char *template, int len, int flags)
> +{
> +	return __open_tempfile (template, len, flags);
> +}
> +
> +weak_alias(__mkostemps, mkostemps);

Sorry, I think I wasn't clear about my comments on the alias. I was
just saying the actual function can be named __mkostemps (rather than
__open_tempfile or whatever) with weak_alias(__mkostemps, mkostemps);
so that it also provides the publicly visible mkostemps. That avoids
having an extra wrapper layer.

> diff --git a/src/temp/mkstemps.c b/src/temp/mkstemps.c
> new file mode 100644
> index 0000000..53fea07
> --- /dev/null
> +++ b/src/temp/mkstemps.c
> @@ -0,0 +1,18 @@
> +#define _GNU_SOURCE
> +#include <string.h>
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <fcntl.h>
> +#include <unistd.h>
> +#include <limits.h>
> +#include <errno.h>
> +#include "libc.h"

I don't think all these headers are needed for just a one-line wrapper
function. :)

> +
> +int __open_tempfile (char *, int, int);
> +
> +int __mkstemps(char *template, int len)
> +{
> +	return __open_tempfile (template, len, O_RDWR);
> +}
> +
> +weak_alias(__mkstemps, mkstemps);

These aliases aren't needed since mkstemps, et.c aren't used
internally. The __-prefixed version of a function, with the "real"
name being a weak alias for it, is needed only when you want to be
able to call a nonstandard function from within the standard library,
or if it's being defined in the same source file as a standard
function.

> diff --git a/src/temp/mktemp.c b/src/temp/mktemp.c
> index c0e06f5..de1afb4 100644
> --- a/src/temp/mktemp.c
> +++ b/src/temp/mktemp.c
> @@ -8,6 +8,8 @@
>  #include <stdint.h>
>  #include "libc.h"
>  
> +char *__randname(char *);
> +
>  char *__mktemp(char *template)
>  {
>  	struct timespec ts;
> @@ -21,10 +23,7 @@ char *__mktemp(char *template)
>  		return template;
>  	}
>  	while (retries--) {
> -		clock_gettime(CLOCK_REALTIME, &ts);
> -		r = ts.tv_nsec + (uintptr_t)&ts / 16 + (uintptr_t)template;
> -		for (i=1; i<=6; i++, r>>=4)
> -			template[l-i] = 'A'+(r&15);
> +		__randname(template);
>  		if (access(template, F_OK) < 0) return template;
>  	}
>  	*template = 0;
> diff --git a/src/temp/randname.c b/src/temp/randname.c
> new file mode 100644
> index 0000000..4d3476f
> --- /dev/null
> +++ b/src/temp/randname.c
> @@ -0,0 +1,22 @@
> +#include <string.h>
> +#include <unistd.h>
> +#include <errno.h>
> +#include <time.h>
> +#include <stdint.h>
> +#include "libc.h"
> +
> +char *__randname(char *template)
> +{
> +	struct timespec ts;
> +	size_t i, l = strlen(template);
> +	unsigned long r;
> +
> +	/* This assumes that a check for the template
> +	   size has alrady been made */
> +	clock_gettime(CLOCK_REALTIME, &ts);
> +	r = ts.tv_nsec + (uintptr_t)&ts / 16 + (uintptr_t)template;
> +	for (i=1; i<=6; i++, r>>=4)
> +		template[l-i] = 'A'+(r&15);
> +
> +	return template;
> +}
> diff --git a/src/temp/tempfile.c b/src/temp/tempfile.c
> new file mode 100644
> index 0000000..93808a6
> --- /dev/null
> +++ b/src/temp/tempfile.c
> @@ -0,0 +1,42 @@
> +#include <string.h>
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <fcntl.h>
> +#include <unistd.h>
> +#include <limits.h>
> +#include <errno.h>
> +#include "libc.h"
> +
> +char *__randname(char *);
> +
> +int __open_tempfile (char *template, int len, int flags)
> +{
> +	if (len < 0) return EINVAL;
> +
> +	int l = strlen(template)-len;
> +	if (l < 6 || strncmp(template+l-6, "XXXXXX",6)) {
> +		errno = EINVAL;
> +		*template = 0;
> +		return -1;
> +	}
> +
> +	/* Null terminate the template before the suffix,
> +	   and save the char for adding back the suffix */
> +	char suffix = template[l];
> +	template[l] = '\0';
> +
> +	int fd, retries = 100, t0 = *template;
> +	while (retries--) {
> +		if (!*__randname(template)) return -1;
> +		/* Add back the suffix */
> +		template[l] = suffix;
> +		if ((fd = open(template, flags | O_CREAT | O_EXCL, 0600))>=0)
> +			return fd;
> +		if (errno != EEXIST) return -1;
> +		/* this is safe because mktemp verified
> +		 * that we have a valid template string */
> +		template[0] = t0;
> +		strcpy(template+l-6, "XXXXXX");

Since your __randname is not checking for XXXXXX like __mktemp was
doing, this strcpy is probably not needed. I'm not sure whether it
would be cleaner to keep this strcpy and put the template check back
in __randname (so that mktemp and __mkostemps don't duplicate the
check) or just get rid of the strcpy here. What do you think?

Rich


^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH] Add support for mkostemp, mkstemps and mkostemps
  2013-01-28  9:37 ` Szabolcs Nagy
@ 2013-01-28 17:33   ` Szabolcs Nagy
  2013-01-29 23:16   ` Anthony G. Basile
  1 sibling, 0 replies; 27+ messages in thread
From: Szabolcs Nagy @ 2013-01-28 17:33 UTC (permalink / raw)
  To: musl

* Szabolcs Nagy <nsz@port70.net> [2013-01-28 10:37:55 +0100]:
> * Anthony G. Basile <basile@opensource.dyc.edu> [2013-01-28 00:06:23 -0500]:
> > +	unsigned long r;
> 
> can be unsigned char r;
> 
> > +	r = ts.tv_nsec + (uintptr_t)&ts / 16 + (uintptr_t)template;
> > +	for (i=1; i<=6; i++, r>>=4)
> > +		template[l-i] = 'A'+(r&15);
> 
> it seems to use only 4bit entropy based on clock (and fixed addresses)
> 

these comments were wrong, i did not notice r>>=4

> > +	int l = strlen(template)-len;
> 
> int vs size_t problem
> 

this is a bug

> i'd do __randname(template, length-suffix)
> 

i think __randname(char *p) api suggestion of john spencer
is good (where p points to the XXXXXX part)


^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH] Add support for mkostemp, mkstemps and mkostemps
  2013-01-28  9:37 ` Szabolcs Nagy
  2013-01-28 17:33   ` Szabolcs Nagy
@ 2013-01-29 23:16   ` Anthony G. Basile
  2013-01-30  7:21     ` Rich Felker
  1 sibling, 1 reply; 27+ messages in thread
From: Anthony G. Basile @ 2013-01-29 23:16 UTC (permalink / raw)
  To: musl

Hi Szabolcs,

Thanks for the feedback.  All these improvements are easy to implement, 
but the random name generator definitely needs a better algorithm.  I 
just adopted what was already there, but its not good enough.

Here's a simple test program which demonstrates the problem:

#include <stdio.h>
#include <stdlib.h>
#include <unistd.h>
#include <string.h>

int main()
{
	int i, fd;
	char *t = (char *)malloc(7);

	for(i = 0; i < 10; i++)
	{
		strcpy(t, "XXXXXX");
		fd = mkstemp(t);
		printf("%s\n", t);
		close(fd);
		unlink(t);
	}

	return 0;
}


On a glibc system, we get something like:

4FeUYd
gZd1x7
wkq860
y2QfGU
e9rnfO
crdvOH
0P9CnB
m0cLWu
eVuTvo
cxT14h

On uclibc we get

WJMzFT
nvSCqr
fneEMi
DTWxB1
SH4n1C
TwLMQ9
LVyEHe
EihiL4
uaqxr4
xmqe7O


On musl we get

GIJNPM
GJDGKC
GJJBDH
GJOFHL
GKFHON
GKLIPB
GLCDJK
GLHONA
GMABAH
GMGPLG


Let me play around with some different algorithms and resubmit this. 
I'll look at what uclibc and glibc do and try to slim them down and 
speed them up.



On 01/28/2013 04:37 AM, Szabolcs Nagy wrote:
> * Anthony G. Basile <basile@opensource.dyc.edu> [2013-01-28 00:06:23 -0500]:
>> +#include <string.h>
>> +#include <unistd.h>
>> +#include <errno.h>
>> +#include <time.h>
>> +#include <stdint.h>
>> +#include "libc.h"
>
> libc.h, errno.h, unistd.h are not used
>
>> +
>> +char *__randname(char *template)
>> +{
>> +	struct timespec ts;
>> +	size_t i, l = strlen(template);
>> +	unsigned long r;
>
> can be unsigned char r;
>
>> +
>> +	/* This assumes that a check for the template
>> +	   size has alrady been made */
>> +	clock_gettime(CLOCK_REALTIME, &ts);
>> +	r = ts.tv_nsec + (uintptr_t)&ts / 16 + (uintptr_t)template;
>> +	for (i=1; i<=6; i++, r>>=4)
>> +		template[l-i] = 'A'+(r&15);
>
> it seems to use only 4bit entropy based on clock (and fixed addresses)
>
> and (uintptr_t)template does not give much entropy if it's malloced
>
> and if the clock source is bad for some reason (eg cpu clock mod 16ns
> is even) then this might not even give different result for each retry
> (may be it helps if retry count is included)
>
>> +
>> +	return template;
>> +}
>> diff --git a/src/temp/tempfile.c b/src/temp/tempfile.c
>
> i'd use different name, eg __tempfile.c to signify that it's internal
>
>> new file mode 100644
>> index 0000000..93808a6
>> --- /dev/null
>> +++ b/src/temp/tempfile.c
>> @@ -0,0 +1,42 @@
>> +#include <string.h>
>> +#include <stdio.h>
>> +#include <stdlib.h>
>> +#include <fcntl.h>
>> +#include <unistd.h>
>> +#include <limits.h>
>> +#include <errno.h>
>> +#include "libc.h"
>
> stdlib.h, limits.h are not used
>
> libc.h is not used
> (in the other files it is needed for weak_alias)
>
>> +
>> +char *__randname(char *);
>> +
>> +int __open_tempfile (char *template, int len, int flags)
>> +{
>> +	if (len < 0) return EINVAL;
>> +
>> +	int l = strlen(template)-len;
>
> int vs size_t problem
>
>> +	if (l < 6 || strncmp(template+l-6, "XXXXXX",6)) {
>> +		errno = EINVAL;
>> +		*template = 0;
>> +		return -1;
>> +	}
>> +
>> +	/* Null terminate the template before the suffix,
>> +	   and save the char for adding back the suffix */
>> +	char suffix = template[l];
>> +	template[l] = '\0';
>> +
>
> i'd do __randname(template, length-suffix)
> so pass the template len as an argument
>
> then no suffix saving is needed
>
>> +	int fd, retries = 100, t0 = *template;
>
> 100 retries does not make sense if only 4bit entropy is used
> (there are 16 different names for the given addresses)
>
>> +	while (retries--) {
>> +		if (!*__randname(template)) return -1;
>> +		/* Add back the suffix */
>> +		template[l] = suffix;
>> +		if ((fd = open(template, flags | O_CREAT | O_EXCL, 0600))>=0)
>> +			return fd;
>> +		if (errno != EEXIST) return -1;
>> +		/* this is safe because mktemp verified
>> +		 * that we have a valid template string */
>> +		template[0] = t0;
>
> i dont understand why template[0] gets clobbered
>
>> +		strcpy(template+l-6, "XXXXXX");
>
> i'd use memcpy or memset to avoid suffix saving
>
>> +	}
>> +	return -1;
>> +}
>> --
>> 1.7.12.4


-- 
Anthony G. Basile, Ph. D.
Chair of Information Technology
D'Youville College
Buffalo, NY 14201
(716) 829-8197


^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH] Add support for mkostemp, mkstemps and mkostemps
  2013-01-29 23:16   ` Anthony G. Basile
@ 2013-01-30  7:21     ` Rich Felker
  2013-01-30  7:59       ` Hardy Falk
  0 siblings, 1 reply; 27+ messages in thread
From: Rich Felker @ 2013-01-30  7:21 UTC (permalink / raw)
  To: musl

On Tue, Jan 29, 2013 at 06:16:11PM -0500, Anthony G. Basile wrote:
> Hi Szabolcs,
> 
> Thanks for the feedback.  All these improvements are easy to
> implement, but the random name generator definitely needs a better
> algorithm.  I just adopted what was already there, but its not good
> enough.
> 
> Here's a simple test program which demonstrates the problem:
> 
> #include <stdio.h>
> #include <stdlib.h>
> #include <unistd.h>
> #include <string.h>
> 
> int main()
> {
> 	int i, fd;
> 	char *t = (char *)malloc(7);
> 
> 	for(i = 0; i < 10; i++)
> 	{
> 		strcpy(t, "XXXXXX");
> 		fd = mkstemp(t);
> 		printf("%s\n", t);
> 		close(fd);
> 		unlink(t);
> 	}
> 
> 	return 0;
> }
> 
> 
> On a glibc system, we get something like:
> 
> 4FeUYd
> gZd1x7
> wkq860
> y2QfGU
> e9rnfO
> crdvOH
> 0P9CnB
> m0cLWu
> eVuTvo
> cxT14h
> 
> On uclibc we get
> 
> WJMzFT
> nvSCqr
> fneEMi
> DTWxB1
> SH4n1C
> TwLMQ9
> LVyEHe
> EihiL4
> uaqxr4
> xmqe7O
> 
> 
> On musl we get
> 
> GIJNPM
> GJDGKC
> GJJBDH
> GJOFHL
> GKFHON
> GKLIPB
> GLCDJK
> GLHONA
> GMABAH
> GMGPLG

Is your concern denial of service by creating all possible temp names
for a given prefix?

With 6 characters and 4 bits per character, there are 2^24
possibilities. That's definitely high enough to avoid problems from
unintended collisions, but I think it's in the realm of "possible" for
an attacker to create them all (it would fill up quite an enormous
amount of directory table space, however, probably over a gig, and
stress the filesystem pretty bad).

One trivial way to get an extra bit per letter would be to add the
lower 4 bits to 'A' and then or the 5th bit onto the result (which
would lowercase it). This would up the number of possibilities to
2^30, which is getting pretty high.

Of course it would be nicer to get up to 6 bits per character
(base64), or even more like 6.5, using modulo rather than &. However,
using non-alphanumeric characters has some tradeoffs; one has to ask
whether the added security against temp name exhaustion DoS is worth
the risk of broken programs passing filenames generated by mkstemp on
system(), popen(), etc. unquoted, which would be dangerous if they
happened to contain characters special to the shell.

> Let me play around with some different algorithms and resubmit this.
> I'll look at what uclibc and glibc do and try to slim them down and
> speed them up.

I don't think performance is an issue unless you're talking about
extra syscalls. The bulk of the time spent in this function is
syscalls, so even making the userspace part of the code 100x faster is
not going to be noticable. I would aim just for small size and
simplicity.

Rich


^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH] Add support for mkostemp, mkstemps and mkostemps
  2013-01-30  7:21     ` Rich Felker
@ 2013-01-30  7:59       ` Hardy Falk
  2013-01-30 13:45         ` Szabolcs Nagy
  0 siblings, 1 reply; 27+ messages in thread
From: Hardy Falk @ 2013-01-30  7:59 UTC (permalink / raw)
  To: musl

Am 30.01.2013 08:21, schrieb Rich Felker:
> On Tue, Jan 29, 2013 at 06:16:11PM -0500, Anthony G. Basile wrote:
>> >Hi Szabolcs,
>> >
>> >Thanks for the feedback.  All these improvements are easy to
>> >implement, but the random name generator definitely needs a better
>> >algorithm.  I just adopted what was already there, but its not good
>> >enough.
>> >
You should try "shr3"  by George Marsaglia (rip)
https://groups.google.com/forum/?fromgroups=#!msg/sci.math/k3kVM8KwR-s/jxPdZl8XWZkJ 
<http://www.google.de/url?sa=t&rct=j&q=marsaglia&source=web&cd=3&ved=0CEAQFjAC&url=http%3A%2F%2Fde.wikipedia.org%2Fwiki%2FGeorge_Marsaglia&ei=99EIUabzCofLtQaI_YG4DA&usg=AFQjCNEImofmguJZAhNFnvO522Qrs4iniw&bvm=bv.41642243,d.Yms>
Thank you for your great work!
<http://www.google.de/url?sa=t&rct=j&q=marsaglia&source=web&cd=3&ved=0CEAQFjAC&url=http%3A%2F%2Fde.wikipedia.org%2Fwiki%2FGeorge_Marsaglia&ei=99EIUabzCofLtQaI_YG4DA&usg=AFQjCNEImofmguJZAhNFnvO522Qrs4iniw&bvm=bv.41642243,d.Yms> 



^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH] Add support for mkostemp, mkstemps and mkostemps
  2013-01-30  7:59       ` Hardy Falk
@ 2013-01-30 13:45         ` Szabolcs Nagy
  2013-01-30 16:51           ` Rich Felker
  0 siblings, 1 reply; 27+ messages in thread
From: Szabolcs Nagy @ 2013-01-30 13:45 UTC (permalink / raw)
  To: musl

* Hardy Falk <hardy.falk@qomboo.com> [2013-01-30 08:59:51 +0100]:
> Am 30.01.2013 08:21, schrieb Rich Felker:
> >On Tue, Jan 29, 2013 at 06:16:11PM -0500, Anthony G. Basile wrote:
> >>>implement, but the random name generator definitely needs a better
> >>>algorithm.  I just adopted what was already there, but its not good
> >>>enough.
> >>>
> You should try "shr3"  by George Marsaglia (rip)

no, we dont need a prng there, we get new entropy
at each try from the clock source

the statistical quality may be improved a bit with
different hashing of the time and addresses, but
it is reasonable now

for retries the iteration count and the previous
rand could be used as well, but that's a rare case

more significant improvement can be done by larger
set of names and better entropy source

the current 24bit should be good enough for most
practical use (you can generate a few thousand
names before a collision happens, assuming uniform
distribution) but that may be worth increasing

the entropy source is mostly problematic on embedded
systems with bad clock, but there is probably no
good source at all there

> https://groups.google.com/forum/?fromgroups=#!msg/sci.math/k3kVM8KwR-s/jxPdZl8XWZkJ

i could not open this
another evil from google..


^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH] Add support for mkostemp, mkstemps and mkostemps
  2013-01-30 13:45         ` Szabolcs Nagy
@ 2013-01-30 16:51           ` Rich Felker
  2013-01-30 19:12             ` Szabolcs Nagy
  0 siblings, 1 reply; 27+ messages in thread
From: Rich Felker @ 2013-01-30 16:51 UTC (permalink / raw)
  To: musl

On Wed, Jan 30, 2013 at 02:45:37PM +0100, Szabolcs Nagy wrote:
> * Hardy Falk <hardy.falk@qomboo.com> [2013-01-30 08:59:51 +0100]:
> > Am 30.01.2013 08:21, schrieb Rich Felker:
> > >On Tue, Jan 29, 2013 at 06:16:11PM -0500, Anthony G. Basile wrote:
> > >>>implement, but the random name generator definitely needs a better
> > >>>algorithm.  I just adopted what was already there, but its not good
> > >>>enough.
> > >>>
> > You should try "shr3"  by George Marsaglia (rip)
> 
> no, we dont need a prng there, we get new entropy
> at each try from the clock source
> 
> the statistical quality may be improved a bit with
> different hashing of the time and addresses, but
> it is reasonable now
> 
> for retries the iteration count and the previous
> rand could be used as well, but that's a rare case

Right now, and attacker who just wants to drastically slow down a
program can create every name in a large time window around the
current time. Better use of the stack address in generating the
filenames could prevent knowing the set of output filenames for a
range of times without knowing the stack address in the program being
attacked. In fact, I'm a little bit worried that the current approach
discloses too much information about the stack address to an attacker.
If nothing else, I think some shuffling should be done so that the
(typically more valuable) high bits of the stack address are matched
with the low (least predictable) bits of the clock.

Obviously cryptographic-quality hashes would be a solution, but I
don't think anybody wants that much bloat in mkstemp, etc...

> more significant improvement can be done by larger
> set of names and better entropy source

The theoretical bound on the space (6 X's to work with) is slightly
under 48 bits (no nulls or slashes), but that assumes you're okay with
inserting control characters, shell-special characters, malformed
utf-8, etc. I would say most of these are very bad.

Other implementations probably use 36 bits or slightly less (base64
perhaps modified base64).

I could see it being feasible to increase this slightly and maybe even
get up to ~40 bit space using part of the UTF-8 space, but I'm not
sure users would like all sorts of random characters in filenames. It
would definitely bog down your file manager loading all those fonts to
view /tmp.. :-)

> the entropy source is mostly problematic on embedded
> systems with bad clock, but there is probably no
> good source at all there

Are you sure this is an issue? IMO it's the kernel's responsibility to
give a good clock value however it can. IIRC even mips has a cpu
counter or something that could be used to compensate for bad clock
hardware, so it seems like a kernel failing if clock_gettime has bad
resolution.

Rich


^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH] Add support for mkostemp, mkstemps and mkostemps
  2013-01-30 16:51           ` Rich Felker
@ 2013-01-30 19:12             ` Szabolcs Nagy
  2013-01-30 19:22               ` Rich Felker
  0 siblings, 1 reply; 27+ messages in thread
From: Szabolcs Nagy @ 2013-01-30 19:12 UTC (permalink / raw)
  To: musl

* Rich Felker <dalias@aerifal.cx> [2013-01-30 11:51:27 -0500]:
> current time. Better use of the stack address in generating the
> filenames could prevent knowing the set of output filenames for a
> range of times without knowing the stack address in the program being
> attacked. In fact, I'm a little bit worried that the current approach
> discloses too much information about the stack address to an attacker.
> If nothing else, I think some shuffling should be done so that the
> (typically more valuable) high bits of the stack address are matched
> with the low (least predictable) bits of the clock.

void __randname(char *p)
{
	struct timespec ts;
	unsigned long r;
	int i;

	clock_gettime(CLOCK_REALTIME, &ts);
	r = ts.tv_nsec*65537 ^ (uintptr_t)&ts / 16 + (uintptr_t)p;
	for (i=0; i<6; i++, r>>=5)
		p[i] = 'A'+(r&15)+(r&16)*2;
}

this uses 30bits of r and mixes the random low bits of nsec
into the high bits

using ^ as i guess that way it's harder to do useful arithmetics
with known r values

> > more significant improvement can be done by larger
> > set of names and better entropy source
> 
> Other implementations probably use 36 bits or slightly less (base64
> perhaps modified base64).
> 
> I could see it being feasible to increase this slightly and maybe even

<= 36bits is probably ok

> > the entropy source is mostly problematic on embedded
> > systems with bad clock, but there is probably no
> > good source at all there
> 
> Are you sure this is an issue? IMO it's the kernel's responsibility to

it was just a guess, iirc there are devices with
low resolution clock (lower than nanoseconds) which
can mean short period of the last few bits of nsec

but i dont know how this works

> give a good clock value however it can. IIRC even mips has a cpu
> counter or something that could be used to compensate for bad clock
> hardware, so it seems like a kernel failing if clock_gettime has bad
> resolution.
> 
> Rich


^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH] Add support for mkostemp, mkstemps and mkostemps
  2013-01-30 19:12             ` Szabolcs Nagy
@ 2013-01-30 19:22               ` Rich Felker
  2013-01-31  0:28                 ` Szabolcs Nagy
  0 siblings, 1 reply; 27+ messages in thread
From: Rich Felker @ 2013-01-30 19:22 UTC (permalink / raw)
  To: musl

On Wed, Jan 30, 2013 at 08:12:58PM +0100, Szabolcs Nagy wrote:
> * Rich Felker <dalias@aerifal.cx> [2013-01-30 11:51:27 -0500]:
> > current time. Better use of the stack address in generating the
> > filenames could prevent knowing the set of output filenames for a
> > range of times without knowing the stack address in the program being
> > attacked. In fact, I'm a little bit worried that the current approach
> > discloses too much information about the stack address to an attacker.
> > If nothing else, I think some shuffling should be done so that the
> > (typically more valuable) high bits of the stack address are matched
> > with the low (least predictable) bits of the clock.
> 
> void __randname(char *p)
> {
> 	struct timespec ts;
> 	unsigned long r;
> 	int i;
> 
> 	clock_gettime(CLOCK_REALTIME, &ts);
> 	r = ts.tv_nsec*65537 ^ (uintptr_t)&ts / 16 + (uintptr_t)p;
> 	for (i=0; i<6; i++, r>>=5)
> 		p[i] = 'A'+(r&15)+(r&16)*2;
> }
> 
> this uses 30bits of r and mixes the random low bits of nsec
> into the high bits

Keep in mind it might be bits 8-15 that are most valuable with ASLR
(assuming the randomization only adjusts by small amounts and not so
much to waste lots of address space). I think this needs a little bit
more consideration.

> > > more significant improvement can be done by larger
> > > set of names and better entropy source
> > 
> > Other implementations probably use 36 bits or slightly less (base64
> > perhaps modified base64).
> > 
> > I could see it being feasible to increase this slightly and maybe even
> 
> <= 36bits is probably ok

You mean >=36? Or..?

Rich


^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH] Add support for mkostemp, mkstemps and mkostemps
  2013-01-30 19:22               ` Rich Felker
@ 2013-01-31  0:28                 ` Szabolcs Nagy
  0 siblings, 0 replies; 27+ messages in thread
From: Szabolcs Nagy @ 2013-01-31  0:28 UTC (permalink / raw)
  To: musl

* Rich Felker <dalias@aerifal.cx> [2013-01-30 14:22:33 -0500]:
> On Wed, Jan 30, 2013 at 08:12:58PM +0100, Szabolcs Nagy wrote:
> > void __randname(char *p)
> > {
> > 	struct timespec ts;
> > 	unsigned long r;
> > 	int i;
> > 
> > 	clock_gettime(CLOCK_REALTIME, &ts);
> > 	r = ts.tv_nsec*65537 ^ (uintptr_t)&ts / 16 + (uintptr_t)p;
> > 	for (i=0; i<6; i++, r>>=5)
> > 		p[i] = 'A'+(r&15)+(r&16)*2;
> > }
> > 
> > this uses 30bits of r and mixes the random low bits of nsec
> > into the high bits
> 
> Keep in mind it might be bits 8-15 that are most valuable with ASLR
> (assuming the randomization only adjusts by small amounts and not so
> much to waste lots of address space). I think this needs a little bit
> more consideration.

hm then use *69069 (a favourite lcg multiplier of the
above mentioned george marsaglia according to knuth,
[taocp vol2 p108] so it probably mixes the low bits
of nsec well)

> > > Other implementations probably use 36 bits or slightly less (base64
> > > perhaps modified base64).
> > > 
> > > I could see it being feasible to increase this slightly and maybe even
> > 
> > <= 36bits is probably ok
> 
> You mean >=36? Or..?

i wanted to say that using <=64 chars in the filename is reasonable
(i thought there were only 62 robust chars: lower,upper,digits
so going above 36 does not make sense and we have 32bit input anyway)

but maybe #%+-._~ are ok as well (these are the ascii chars that
are not escaped by bash for whatever reason), except #+-.~ should
not be first char

in urls only -._ are the always safe extra chars

in regex #%-_~ are safe from the above set

but if we go above 62chars then more than 32bit input is needed


another proposal:

uint64_t r=1;
uint64_t a[]={stackptr, inputptr, sec, ns, pid, retrycount}; // entropy sources
for (i = 0; i < 6; i++) {
	r += a[i];
	r *= 6364136223846793005ull;
	r ^= r>>24;
}
for (i = 0; i < 6; i++, r /= 63)
	p[i] = "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789_"[r%63];


^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH] Add support for mkostemp, mkstemps and mkostemps
  2013-01-28  5:06 [PATCH] Add support for mkostemp, mkstemps and mkostemps Anthony G. Basile
                   ` (2 preceding siblings ...)
  2013-01-28 16:33 ` Rich Felker
@ 2013-02-02  3:48 ` Rich Felker
  2013-02-02 18:46   ` Anthony G. Basile
  3 siblings, 1 reply; 27+ messages in thread
From: Rich Felker @ 2013-02-02  3:48 UTC (permalink / raw)
  To: musl

Hi again,

Now that the release is done, I'd like to get back to integrating
things like this. Have you made any changes based on the reviews?
Overall this looks good and I'd like to commit a patch within the next
few days. If you don't have time to work on it more, let me know and I
can prepare a final patch based on what you sent.

Rich

On Mon, Jan 28, 2013 at 12:06:23AM -0500, Anthony G. Basile wrote:
> From: "Anthony G. Basile" <basile@opensource.dyc.edu>
> 
> Signed-off-by: Anthony G. Basile <blueness@gentoo.org>
> ---
>  include/stdlib.h     |  6 ++++++
>  src/temp/mkostemp.c  | 18 ++++++++++++++++++
>  src/temp/mkostemps.c | 18 ++++++++++++++++++
>  src/temp/mkstemp.c   | 15 ++-------------
>  src/temp/mkstemps.c  | 18 ++++++++++++++++++
>  src/temp/mktemp.c    |  7 +++----
>  src/temp/randname.c  | 22 ++++++++++++++++++++++
>  src/temp/tempfile.c  | 42 ++++++++++++++++++++++++++++++++++++++++++
>  8 files changed, 129 insertions(+), 17 deletions(-)
>  create mode 100644 src/temp/mkostemp.c
>  create mode 100644 src/temp/mkostemps.c
>  create mode 100644 src/temp/mkstemps.c
>  create mode 100644 src/temp/randname.c
>  create mode 100644 src/temp/tempfile.c
> 
> diff --git a/include/stdlib.h b/include/stdlib.h
> index 671d188..4210f40 100644
> --- a/include/stdlib.h
> +++ b/include/stdlib.h
> @@ -95,6 +95,9 @@ int posix_memalign (void **, size_t, size_t);
>  int setenv (const char *, const char *, int);
>  int unsetenv (const char *);
>  int mkstemp (char *);
> +int mkostemp (char *, int);
> +int mkstemps (char *, int);
> +int mkostemps (char *, int, int);
>  char *mkdtemp (char *);
>  int getsubopt (char **, char *const *, char **);
>  int rand_r (unsigned *);
> @@ -150,6 +153,9 @@ char *gcvt(double, int, char *);
>  
>  #if defined(_LARGEFILE64_SOURCE) || defined(_GNU_SOURCE)
>  #define mkstemp64 mkstemp
> +#define mkostemp64 mkostemp
> +#define mkstemps64 mkstemps
> +#define mkostemps64 mkostemps
>  #endif
>  
>  #ifdef __cplusplus
> diff --git a/src/temp/mkostemp.c b/src/temp/mkostemp.c
> new file mode 100644
> index 0000000..750d880
> --- /dev/null
> +++ b/src/temp/mkostemp.c
> @@ -0,0 +1,18 @@
> +#define _GNU_SOURCE
> +#include <string.h>
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <fcntl.h>
> +#include <unistd.h>
> +#include <limits.h>
> +#include <errno.h>
> +#include "libc.h"
> +
> +int __open_tempfile (char *, int, int);
> +
> +int __mkostemp(char *template, int flags)
> +{
> +	return __open_tempfile (template, 0, flags);
> +}
> +
> +weak_alias(__mkostemp, mkostemp);
> diff --git a/src/temp/mkostemps.c b/src/temp/mkostemps.c
> new file mode 100644
> index 0000000..8c810ce
> --- /dev/null
> +++ b/src/temp/mkostemps.c
> @@ -0,0 +1,18 @@
> +#define _GNU_SOURCE
> +#include <string.h>
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <fcntl.h>
> +#include <unistd.h>
> +#include <limits.h>
> +#include <errno.h>
> +#include "libc.h"
> +
> +int __open_tempfile (char *, int, int);
> +
> +int __mkostemps(char *template, int len, int flags)
> +{
> +	return __open_tempfile (template, len, flags);
> +}
> +
> +weak_alias(__mkostemps, mkostemps);
> diff --git a/src/temp/mkstemp.c b/src/temp/mkstemp.c
> index a390d42..ccaf3c6 100644
> --- a/src/temp/mkstemp.c
> +++ b/src/temp/mkstemp.c
> @@ -7,22 +7,11 @@
>  #include <errno.h>
>  #include "libc.h"
>  
> -char *__mktemp(char *);
> +int __open_tempfile (char *, int, int);
>  
>  int mkstemp(char *template)
>  {
> -	int fd, retries = 100, t0 = *template;
> -	while (retries--) {
> -		if (!*__mktemp(template)) return -1;
> -		if ((fd = open(template, O_RDWR | O_CREAT | O_EXCL, 0600))>=0)
> -			return fd;
> -		if (errno != EEXIST) return -1;
> -		/* this is safe because mktemp verified
> -		 * that we have a valid template string */
> -		template[0] = t0;
> -		strcpy(template+strlen(template)-6, "XXXXXX");
> -	}
> -	return -1;
> +	return __open_tempfile (template, 0, O_RDWR);
>  }
>  
>  LFS64(mkstemp);
> diff --git a/src/temp/mkstemps.c b/src/temp/mkstemps.c
> new file mode 100644
> index 0000000..53fea07
> --- /dev/null
> +++ b/src/temp/mkstemps.c
> @@ -0,0 +1,18 @@
> +#define _GNU_SOURCE
> +#include <string.h>
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <fcntl.h>
> +#include <unistd.h>
> +#include <limits.h>
> +#include <errno.h>
> +#include "libc.h"
> +
> +int __open_tempfile (char *, int, int);
> +
> +int __mkstemps(char *template, int len)
> +{
> +	return __open_tempfile (template, len, O_RDWR);
> +}
> +
> +weak_alias(__mkstemps, mkstemps);
> diff --git a/src/temp/mktemp.c b/src/temp/mktemp.c
> index c0e06f5..de1afb4 100644
> --- a/src/temp/mktemp.c
> +++ b/src/temp/mktemp.c
> @@ -8,6 +8,8 @@
>  #include <stdint.h>
>  #include "libc.h"
>  
> +char *__randname(char *);
> +
>  char *__mktemp(char *template)
>  {
>  	struct timespec ts;
> @@ -21,10 +23,7 @@ char *__mktemp(char *template)
>  		return template;
>  	}
>  	while (retries--) {
> -		clock_gettime(CLOCK_REALTIME, &ts);
> -		r = ts.tv_nsec + (uintptr_t)&ts / 16 + (uintptr_t)template;
> -		for (i=1; i<=6; i++, r>>=4)
> -			template[l-i] = 'A'+(r&15);
> +		__randname(template);
>  		if (access(template, F_OK) < 0) return template;
>  	}
>  	*template = 0;
> diff --git a/src/temp/randname.c b/src/temp/randname.c
> new file mode 100644
> index 0000000..4d3476f
> --- /dev/null
> +++ b/src/temp/randname.c
> @@ -0,0 +1,22 @@
> +#include <string.h>
> +#include <unistd.h>
> +#include <errno.h>
> +#include <time.h>
> +#include <stdint.h>
> +#include "libc.h"
> +
> +char *__randname(char *template)
> +{
> +	struct timespec ts;
> +	size_t i, l = strlen(template);
> +	unsigned long r;
> +
> +	/* This assumes that a check for the template
> +	   size has alrady been made */
> +	clock_gettime(CLOCK_REALTIME, &ts);
> +	r = ts.tv_nsec + (uintptr_t)&ts / 16 + (uintptr_t)template;
> +	for (i=1; i<=6; i++, r>>=4)
> +		template[l-i] = 'A'+(r&15);
> +
> +	return template;
> +}
> diff --git a/src/temp/tempfile.c b/src/temp/tempfile.c
> new file mode 100644
> index 0000000..93808a6
> --- /dev/null
> +++ b/src/temp/tempfile.c
> @@ -0,0 +1,42 @@
> +#include <string.h>
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <fcntl.h>
> +#include <unistd.h>
> +#include <limits.h>
> +#include <errno.h>
> +#include "libc.h"
> +
> +char *__randname(char *);
> +
> +int __open_tempfile (char *template, int len, int flags)
> +{
> +	if (len < 0) return EINVAL;
> +
> +	int l = strlen(template)-len;
> +	if (l < 6 || strncmp(template+l-6, "XXXXXX",6)) {
> +		errno = EINVAL;
> +		*template = 0;
> +		return -1;
> +	}
> +
> +	/* Null terminate the template before the suffix,
> +	   and save the char for adding back the suffix */
> +	char suffix = template[l];
> +	template[l] = '\0';
> +
> +	int fd, retries = 100, t0 = *template;
> +	while (retries--) {
> +		if (!*__randname(template)) return -1;
> +		/* Add back the suffix */
> +		template[l] = suffix;
> +		if ((fd = open(template, flags | O_CREAT | O_EXCL, 0600))>=0)
> +			return fd;
> +		if (errno != EEXIST) return -1;
> +		/* this is safe because mktemp verified
> +		 * that we have a valid template string */
> +		template[0] = t0;
> +		strcpy(template+l-6, "XXXXXX");
> +	}
> +	return -1;
> +}
> -- 
> 1.7.12.4


^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH] Add support for mkostemp, mkstemps and mkostemps
  2013-02-02  3:48 ` Rich Felker
@ 2013-02-02 18:46   ` Anthony G. Basile
  0 siblings, 0 replies; 27+ messages in thread
From: Anthony G. Basile @ 2013-02-02 18:46 UTC (permalink / raw)
  To: musl

Hi Rich,

Revised patch submitted.  The only thing I'm not certain of is the 
entropy for the random name.  I considered what everyone said, and 
looked at how uclibc/glibc do it and came up with something that is 1) 
fast, 2) small footprint, but whether there's enough randomness is 
debatable.


On 02/01/2013 10:48 PM, Rich Felker wrote:
> Hi again,
>
> Now that the release is done, I'd like to get back to integrating
> things like this. Have you made any changes based on the reviews?
> Overall this looks good and I'd like to commit a patch within the next
> few days. If you don't have time to work on it more, let me know and I
> can prepare a final patch based on what you sent.
>
> Rich
>
> On Mon, Jan 28, 2013 at 12:06:23AM -0500, Anthony G. Basile wrote:
>> From: "Anthony G. Basile" <basile@opensource.dyc.edu>
>>
>> Signed-off-by: Anthony G. Basile <blueness@gentoo.org>
>> ---
>>   include/stdlib.h     |  6 ++++++
>>   src/temp/mkostemp.c  | 18 ++++++++++++++++++
>>   src/temp/mkostemps.c | 18 ++++++++++++++++++
>>   src/temp/mkstemp.c   | 15 ++-------------
>>   src/temp/mkstemps.c  | 18 ++++++++++++++++++
>>   src/temp/mktemp.c    |  7 +++----
>>   src/temp/randname.c  | 22 ++++++++++++++++++++++
>>   src/temp/tempfile.c  | 42 ++++++++++++++++++++++++++++++++++++++++++
>>   8 files changed, 129 insertions(+), 17 deletions(-)
>>   create mode 100644 src/temp/mkostemp.c
>>   create mode 100644 src/temp/mkostemps.c
>>   create mode 100644 src/temp/mkstemps.c
>>   create mode 100644 src/temp/randname.c
>>   create mode 100644 src/temp/tempfile.c
>>
>> diff --git a/include/stdlib.h b/include/stdlib.h
>> index 671d188..4210f40 100644
>> --- a/include/stdlib.h
>> +++ b/include/stdlib.h
>> @@ -95,6 +95,9 @@ int posix_memalign (void **, size_t, size_t);
>>   int setenv (const char *, const char *, int);
>>   int unsetenv (const char *);
>>   int mkstemp (char *);
>> +int mkostemp (char *, int);
>> +int mkstemps (char *, int);
>> +int mkostemps (char *, int, int);
>>   char *mkdtemp (char *);
>>   int getsubopt (char **, char *const *, char **);
>>   int rand_r (unsigned *);
>> @@ -150,6 +153,9 @@ char *gcvt(double, int, char *);
>>
>>   #if defined(_LARGEFILE64_SOURCE) || defined(_GNU_SOURCE)
>>   #define mkstemp64 mkstemp
>> +#define mkostemp64 mkostemp
>> +#define mkstemps64 mkstemps
>> +#define mkostemps64 mkostemps
>>   #endif
>>
>>   #ifdef __cplusplus
>> diff --git a/src/temp/mkostemp.c b/src/temp/mkostemp.c
>> new file mode 100644
>> index 0000000..750d880
>> --- /dev/null
>> +++ b/src/temp/mkostemp.c
>> @@ -0,0 +1,18 @@
>> +#define _GNU_SOURCE
>> +#include <string.h>
>> +#include <stdio.h>
>> +#include <stdlib.h>
>> +#include <fcntl.h>
>> +#include <unistd.h>
>> +#include <limits.h>
>> +#include <errno.h>
>> +#include "libc.h"
>> +
>> +int __open_tempfile (char *, int, int);
>> +
>> +int __mkostemp(char *template, int flags)
>> +{
>> +	return __open_tempfile (template, 0, flags);
>> +}
>> +
>> +weak_alias(__mkostemp, mkostemp);
>> diff --git a/src/temp/mkostemps.c b/src/temp/mkostemps.c
>> new file mode 100644
>> index 0000000..8c810ce
>> --- /dev/null
>> +++ b/src/temp/mkostemps.c
>> @@ -0,0 +1,18 @@
>> +#define _GNU_SOURCE
>> +#include <string.h>
>> +#include <stdio.h>
>> +#include <stdlib.h>
>> +#include <fcntl.h>
>> +#include <unistd.h>
>> +#include <limits.h>
>> +#include <errno.h>
>> +#include "libc.h"
>> +
>> +int __open_tempfile (char *, int, int);
>> +
>> +int __mkostemps(char *template, int len, int flags)
>> +{
>> +	return __open_tempfile (template, len, flags);
>> +}
>> +
>> +weak_alias(__mkostemps, mkostemps);
>> diff --git a/src/temp/mkstemp.c b/src/temp/mkstemp.c
>> index a390d42..ccaf3c6 100644
>> --- a/src/temp/mkstemp.c
>> +++ b/src/temp/mkstemp.c
>> @@ -7,22 +7,11 @@
>>   #include <errno.h>
>>   #include "libc.h"
>>
>> -char *__mktemp(char *);
>> +int __open_tempfile (char *, int, int);
>>
>>   int mkstemp(char *template)
>>   {
>> -	int fd, retries = 100, t0 = *template;
>> -	while (retries--) {
>> -		if (!*__mktemp(template)) return -1;
>> -		if ((fd = open(template, O_RDWR | O_CREAT | O_EXCL, 0600))>=0)
>> -			return fd;
>> -		if (errno != EEXIST) return -1;
>> -		/* this is safe because mktemp verified
>> -		 * that we have a valid template string */
>> -		template[0] = t0;
>> -		strcpy(template+strlen(template)-6, "XXXXXX");
>> -	}
>> -	return -1;
>> +	return __open_tempfile (template, 0, O_RDWR);
>>   }
>>
>>   LFS64(mkstemp);
>> diff --git a/src/temp/mkstemps.c b/src/temp/mkstemps.c
>> new file mode 100644
>> index 0000000..53fea07
>> --- /dev/null
>> +++ b/src/temp/mkstemps.c
>> @@ -0,0 +1,18 @@
>> +#define _GNU_SOURCE
>> +#include <string.h>
>> +#include <stdio.h>
>> +#include <stdlib.h>
>> +#include <fcntl.h>
>> +#include <unistd.h>
>> +#include <limits.h>
>> +#include <errno.h>
>> +#include "libc.h"
>> +
>> +int __open_tempfile (char *, int, int);
>> +
>> +int __mkstemps(char *template, int len)
>> +{
>> +	return __open_tempfile (template, len, O_RDWR);
>> +}
>> +
>> +weak_alias(__mkstemps, mkstemps);
>> diff --git a/src/temp/mktemp.c b/src/temp/mktemp.c
>> index c0e06f5..de1afb4 100644
>> --- a/src/temp/mktemp.c
>> +++ b/src/temp/mktemp.c
>> @@ -8,6 +8,8 @@
>>   #include <stdint.h>
>>   #include "libc.h"
>>
>> +char *__randname(char *);
>> +
>>   char *__mktemp(char *template)
>>   {
>>   	struct timespec ts;
>> @@ -21,10 +23,7 @@ char *__mktemp(char *template)
>>   		return template;
>>   	}
>>   	while (retries--) {
>> -		clock_gettime(CLOCK_REALTIME, &ts);
>> -		r = ts.tv_nsec + (uintptr_t)&ts / 16 + (uintptr_t)template;
>> -		for (i=1; i<=6; i++, r>>=4)
>> -			template[l-i] = 'A'+(r&15);
>> +		__randname(template);
>>   		if (access(template, F_OK) < 0) return template;
>>   	}
>>   	*template = 0;
>> diff --git a/src/temp/randname.c b/src/temp/randname.c
>> new file mode 100644
>> index 0000000..4d3476f
>> --- /dev/null
>> +++ b/src/temp/randname.c
>> @@ -0,0 +1,22 @@
>> +#include <string.h>
>> +#include <unistd.h>
>> +#include <errno.h>
>> +#include <time.h>
>> +#include <stdint.h>
>> +#include "libc.h"
>> +
>> +char *__randname(char *template)
>> +{
>> +	struct timespec ts;
>> +	size_t i, l = strlen(template);
>> +	unsigned long r;
>> +
>> +	/* This assumes that a check for the template
>> +	   size has alrady been made */
>> +	clock_gettime(CLOCK_REALTIME, &ts);
>> +	r = ts.tv_nsec + (uintptr_t)&ts / 16 + (uintptr_t)template;
>> +	for (i=1; i<=6; i++, r>>=4)
>> +		template[l-i] = 'A'+(r&15);
>> +
>> +	return template;
>> +}
>> diff --git a/src/temp/tempfile.c b/src/temp/tempfile.c
>> new file mode 100644
>> index 0000000..93808a6
>> --- /dev/null
>> +++ b/src/temp/tempfile.c
>> @@ -0,0 +1,42 @@
>> +#include <string.h>
>> +#include <stdio.h>
>> +#include <stdlib.h>
>> +#include <fcntl.h>
>> +#include <unistd.h>
>> +#include <limits.h>
>> +#include <errno.h>
>> +#include "libc.h"
>> +
>> +char *__randname(char *);
>> +
>> +int __open_tempfile (char *template, int len, int flags)
>> +{
>> +	if (len < 0) return EINVAL;
>> +
>> +	int l = strlen(template)-len;
>> +	if (l < 6 || strncmp(template+l-6, "XXXXXX",6)) {
>> +		errno = EINVAL;
>> +		*template = 0;
>> +		return -1;
>> +	}
>> +
>> +	/* Null terminate the template before the suffix,
>> +	   and save the char for adding back the suffix */
>> +	char suffix = template[l];
>> +	template[l] = '\0';
>> +
>> +	int fd, retries = 100, t0 = *template;
>> +	while (retries--) {
>> +		if (!*__randname(template)) return -1;
>> +		/* Add back the suffix */
>> +		template[l] = suffix;
>> +		if ((fd = open(template, flags | O_CREAT | O_EXCL, 0600))>=0)
>> +			return fd;
>> +		if (errno != EEXIST) return -1;
>> +		/* this is safe because mktemp verified
>> +		 * that we have a valid template string */
>> +		template[0] = t0;
>> +		strcpy(template+l-6, "XXXXXX");
>> +	}
>> +	return -1;
>> +}
>> --
>> 1.7.12.4


-- 
Anthony G. Basile, Ph. D.
Chair of Information Technology
D'Youville College
Buffalo, NY 14201
(716) 829-8197


^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH] Add support for mkostemp, mkstemps and mkostemps
  2013-02-02 21:15 Anthony G. Basile
  2013-02-02 21:17 ` Anthony G. Basile
@ 2013-02-03  3:30 ` Rich Felker
  1 sibling, 0 replies; 27+ messages in thread
From: Rich Felker @ 2013-02-03  3:30 UTC (permalink / raw)
  To: musl

[-- Attachment #1: Type: text/plain, Size: 1791 bytes --]

I'm attaching my "proposed" version, which isn't quite in the state I
want it to be committed, but it addresses a few issues, some of which
I tried to explain before but which are difficult without just showing
it...

Some notes:

- The public header in which a public function is defined should
  always be included in the file with the definition, to ensure the
  prototype matches if nothing else.

- Nonstandard functions should be protected in the headers, not
  exposed in the default namespace. Technically mkostemp is
  nonstandard, but it's accepted for inclusion in Issue 8, and so far
  the policy for such functions is to treat them as if they're already
  in the standard.

- Using the LFS64 macro when _GNU_SOURCE is defined breaks due to the
  #defines in the system headers. This is a bug in musl, but I worked
  around it by using _BSD_SOURCE instead which doesn't expose the *64
  junk.

- O_RDWR needs to be builtin to the tempfile-creation core (now
  __mkostemps), not passed in by the caller. The flags argument is
  only for additional flags, and the proposed text in POSIX is clear
  that passing anything but a small limited set of flags yields
  unspecified behavior.

- I'm using __clock_gettime rather than clock_gettime so that these
  functions can hopefully eventually be shared with the code for
  tmpfile and tmpnam, which are in ISO C and can't depend on POSIX
  namespace. However the issue of using open still remains; it should
  perhaps be changed to make the syscall directly like tmpfile does
  now. This can be changed later; it's not a big deal.

I have not addressed any of the randomness considerations; I just
copied the code for that from your latest patch. Comments on the ml
should still be taken into consideration for finalizing that.

Rich

[-- Attachment #2: mkostemps.diff --]
[-- Type: text/plain, Size: 5017 bytes --]

diff --git a/include/stdlib.h b/include/stdlib.h
index 671d188..0bcc9f4 100644
--- a/include/stdlib.h
+++ b/include/stdlib.h
@@ -95,6 +95,7 @@ int posix_memalign (void **, size_t, size_t);
 int setenv (const char *, const char *, int);
 int unsetenv (const char *);
 int mkstemp (char *);
+int mkostemp (char *, int);
 char *mkdtemp (char *);
 int getsubopt (char **, char *const *, char **);
 int rand_r (unsigned *);
@@ -134,6 +135,8 @@ void lcong48 (unsigned short [7]);
 #if defined(_GNU_SOURCE) || defined(_BSD_SOURCE)
 #include <alloca.h>
 char *mktemp (char *);
+int mkstemps (char *, int);
+int mkostemps (char *, int, int);
 void *valloc (size_t);
 void *memalign(size_t, size_t);
 #define WCOREDUMP(s) ((s) & 0x80)
@@ -150,6 +153,11 @@ char *gcvt(double, int, char *);
 
 #if defined(_LARGEFILE64_SOURCE) || defined(_GNU_SOURCE)
 #define mkstemp64 mkstemp
+#define mkostemp64 mkostemp
+#if defined(_GNU_SOURCE) || defined(_BSD_SOURCE)
+#define mkstemps64 mkstemps
+#define mkostemps64 mkostemps
+#endif
 #endif
 
 #ifdef __cplusplus
diff --git a/src/temp/__randname.c b/src/temp/__randname.c
new file mode 100644
index 0000000..b097576
--- /dev/null
+++ b/src/temp/__randname.c
@@ -0,0 +1,21 @@
+#include <string.h>
+#include <time.h>
+#include <stdint.h>
+
+int __clock_gettime(clockid_t, struct timespec *);
+
+/* This assumes that a check for the
+   template size has alrady been made */
+char *__randname(char *template)
+{
+	int i;
+	struct timespec ts;
+	unsigned long r;
+
+	__clock_gettime(CLOCK_REALTIME, &ts);
+	r = ts.tv_nsec*65537 ^ (uintptr_t)&ts / 16 + (uintptr_t)template;
+	for (i=0; i<6; i++, r>>=5)
+		template[i] = 'A'+(r&15)+(r&16)*2;
+
+	return template;
+}
diff --git a/src/temp/mkostemp.c b/src/temp/mkostemp.c
new file mode 100644
index 0000000..e73e22a
--- /dev/null
+++ b/src/temp/mkostemp.c
@@ -0,0 +1,12 @@
+#define _BSD_SOURCE
+#include <stdlib.h>
+#include "libc.h"
+
+int __mkostemps(char *, int, int);
+
+int mkostemp(char *template, int flags)
+{
+	return __mkostemps(template, 0, flags);
+}
+
+LFS64(mkostemp);
diff --git a/src/temp/mkostemps.c b/src/temp/mkostemps.c
new file mode 100644
index 0000000..804a547
--- /dev/null
+++ b/src/temp/mkostemps.c
@@ -0,0 +1,32 @@
+#define _BSD_SOURCE
+#include <string.h>
+#include <fcntl.h>
+#include <unistd.h>
+#include <errno.h>
+#include "libc.h"
+
+char *__randname(char *);
+
+int __mkostemps(char *template, int len, int flags)
+{
+	if (len < 0) return EINVAL;
+
+	size_t l = strlen(template)-len;
+	if (l < 6 || strncmp(template+l-6, "XXXXXX", 6)) {
+		errno = EINVAL;
+		*template = 0;
+		return -1;
+	}
+
+	int fd, retries = 100;
+	while (retries--) {
+		__randname(template+l-6);
+		if ((fd = open(template, flags | O_RDWR | O_CREAT | O_EXCL, 0600))>=0)
+			return fd;
+		if (errno != EEXIST) return -1;
+	}
+	return -1;
+}
+
+weak_alias(__mkostemps, mkostemps);
+weak_alias(__mkostemps, mkostemps64);
diff --git a/src/temp/mkstemp.c b/src/temp/mkstemp.c
index a390d42..85764af 100644
--- a/src/temp/mkstemp.c
+++ b/src/temp/mkstemp.c
@@ -1,28 +1,11 @@
-#include <string.h>
-#include <stdio.h>
 #include <stdlib.h>
-#include <fcntl.h>
-#include <unistd.h>
-#include <limits.h>
-#include <errno.h>
 #include "libc.h"
 
-char *__mktemp(char *);
+int __mkostemps(char *, int, int);
 
 int mkstemp(char *template)
 {
-	int fd, retries = 100, t0 = *template;
-	while (retries--) {
-		if (!*__mktemp(template)) return -1;
-		if ((fd = open(template, O_RDWR | O_CREAT | O_EXCL, 0600))>=0)
-			return fd;
-		if (errno != EEXIST) return -1;
-		/* this is safe because mktemp verified
-		 * that we have a valid template string */
-		template[0] = t0;
-		strcpy(template+strlen(template)-6, "XXXXXX");
-	}
-	return -1;
+	return __mkostemps(template, 0, 0);
 }
 
 LFS64(mkstemp);
diff --git a/src/temp/mkstemps.c b/src/temp/mkstemps.c
new file mode 100644
index 0000000..fda710b
--- /dev/null
+++ b/src/temp/mkstemps.c
@@ -0,0 +1,12 @@
+#define _BSD_SOURCE
+#include <stdlib.h>
+#include "libc.h"
+
+int __mkostemps(char *, int, int);
+
+int mkstemps(char *template, int len)
+{
+	return __mkostemps(template, len, 0);
+}
+
+LFS64(mkstemps);
diff --git a/src/temp/mktemp.c b/src/temp/mktemp.c
index c0e06f5..ed2c103 100644
--- a/src/temp/mktemp.c
+++ b/src/temp/mktemp.c
@@ -1,17 +1,14 @@
 #include <string.h>
-#include <stdio.h>
-#include <stdlib.h>
 #include <fcntl.h>
 #include <unistd.h>
 #include <errno.h>
-#include <time.h>
-#include <stdint.h>
 #include "libc.h"
 
+char *__randname(char *);
+
 char *__mktemp(char *template)
 {
-	struct timespec ts;
-	size_t i, l = strlen(template);
+	size_t l = strlen(template);
 	int retries = 10000;
 	unsigned long r;
 
@@ -21,10 +18,7 @@ char *__mktemp(char *template)
 		return template;
 	}
 	while (retries--) {
-		clock_gettime(CLOCK_REALTIME, &ts);
-		r = ts.tv_nsec + (uintptr_t)&ts / 16 + (uintptr_t)template;
-		for (i=1; i<=6; i++, r>>=4)
-			template[l-i] = 'A'+(r&15);
+		__randname(template+l-6);
 		if (access(template, F_OK) < 0) return template;
 	}
 	*template = 0;

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH] Add support for mkostemp, mkstemps and mkostemps
  2013-02-02 20:38   ` Anthony G. Basile
@ 2013-02-02 22:22     ` Szabolcs Nagy
  0 siblings, 0 replies; 27+ messages in thread
From: Szabolcs Nagy @ 2013-02-02 22:22 UTC (permalink / raw)
  To: musl

* Anthony G. Basile <basile@opensource.dyc.edu> [2013-02-02 15:38:43 -0500]:
> 2. This is from uclibc.  Clearly, static is critical here, but still
> they never initialize a value of 'value' on first entry into the
> function, so that memory is dirty to start.  Mine is worse, but I
> wonder if this is still a bug there.
> 
> static void brain_damaged_fillrand(unsigned char *buf, unsigned int len)
> {
> 	...
>         static uint64_t value;
>         gettimeofday(&tv, NULL);
>         value += ((uint64_t) tv.tv_usec << 16) ^ tv.tv_sec ^ getpid();
> 	...

static variables are always initialized to 0

they use static so every call to that function
uses some entropy from the previous call
(in multithreaded code it may not work that way)


> 3. I retested your address approach.  I like it but it only maps to
> upper and lower case letters, no numbers which uclibc and glibc do.

yes i used 6*5 bit for the names, which makes sense: r is 32bits usually
(ie. that code can generate 2^30 different names)

uclibc/glibc can generate 62^6 names (about 2^36) which is a bit more
but by not much

> clock_gettime(CLOCK_REALTIME, &ts);
> r = ts.tv_nsec*65537 ^ (uintptr_t)&ts / 16 + (uintptr_t)template;
> for (i=0; i<6; i++, r>>=5)
> 	template[i] = 'A'+(r&15)+(r&16)*2;
> 


^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH] Add support for mkostemp, mkstemps and mkostemps
  2013-02-02 19:51 ` John Spencer
  2013-02-02 19:59   ` John Spencer
@ 2013-02-02 22:11   ` Szabolcs Nagy
  1 sibling, 0 replies; 27+ messages in thread
From: Szabolcs Nagy @ 2013-02-02 22:11 UTC (permalink / raw)
  To: musl

* John Spencer <maillist-musl@barfooze.de> [2013-02-02 20:51:12 +0100]:
> +	unsigned long r = ts.tv_nsec + (uintptr_t)&ts / 16 + (uintptr_t)x6;

i recommend a multiplier here, eg. with

r = nsec*1664525 + A;

if only the last few bits of nsec are uniform random then
the top bits of A are not immediately known from r

..only the top bits of A*4276115653 can be known
(1664525*4276115653 == 1 mod 2^32)

and that's probably less useful for an attacker


^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH] Add support for mkostemp, mkstemps and mkostemps
  2013-02-02 21:15 Anthony G. Basile
@ 2013-02-02 21:17 ` Anthony G. Basile
  2013-02-03  3:30 ` Rich Felker
  1 sibling, 0 replies; 27+ messages in thread
From: Anthony G. Basile @ 2013-02-02 21:17 UTC (permalink / raw)
  To: musl

Okay a few points about my last patch.

1. I removed lots of unnecessary headers.

2. I tested using:

	strcpy(t, "aaaXXXXXX");
	mktemp(t);
	printf("mktemp = %s\n", t);

	strcpy(t, "aaaXXXXXX");
	fd = mkstemp(t);
	printf("mkstemp = %s\n", t);
	report_close(fd);
	//unlink(t);

	strcpy(t, "aaaXXXXXX");
	fd = mkostemp(t, O_WRONLY|O_CLOEXEC);
	printf("mkostemp = %s\n", t);
	report_close(fd);
	//unlink(t);

	strcpy(t, "aaaXXXXXXzzz");
	fd = mkstemps(t, 3);
	printf("mkstemps = %s\n", t);
	report_close(fd);
	//unlink(t);

	strcpy(t, "aaaXXXXXXzzz");
	fd = mkostemps(t, 3, O_WRONLY|O_CLOEXEC);
	printf("mkostemps = %s\n", t);
	report_close(fd);
	//unlink(t);


3. I went with Szabolcs' function to generate entropy:

	for (i=0; i<6; i++, r>>=5)
		template[i] = 'A'+(r&15)+(r&16)*2;

I think we can commit this (baring any other problems) and think about 
improving it in the future.  As I said before, uclibc/glibc give 
filenames which include:

static const char letters[] =
"abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789";

while 'A'+(r&15)+(r&16)*2 gives A-P a-p. So the space of names for the 
former is 62^6 while for the latter is 32^6.



-- 
Anthony G. Basile, Ph. D.
Chair of Information Technology
D'Youville College
Buffalo, NY 14201
(716) 829-8197


^ permalink raw reply	[flat|nested] 27+ messages in thread

* [PATCH] Add support for mkostemp, mkstemps and mkostemps
@ 2013-02-02 21:15 Anthony G. Basile
  2013-02-02 21:17 ` Anthony G. Basile
  2013-02-03  3:30 ` Rich Felker
  0 siblings, 2 replies; 27+ messages in thread
From: Anthony G. Basile @ 2013-02-02 21:15 UTC (permalink / raw)
  To: musl; +Cc: Anthony G. Basile

From: "Anthony G. Basile" <blueness@gentoo.org>

Signed-off-by: Anthony G. Basile <blueness@gentoo.org>
---
 include/stdlib.h           |  6 ++++++
 src/temp/__open_tempfile.c | 27 +++++++++++++++++++++++++++
 src/temp/__randname.c      | 19 +++++++++++++++++++
 src/temp/mkostemp.c        | 11 +++++++++++
 src/temp/mkostemps.c       | 11 +++++++++++
 src/temp/mkstemp.c         | 21 ++-------------------
 src/temp/mkstemps.c        | 12 ++++++++++++
 src/temp/mktemp.c          | 14 ++++----------
 8 files changed, 92 insertions(+), 29 deletions(-)
 create mode 100644 src/temp/__open_tempfile.c
 create mode 100644 src/temp/__randname.c
 create mode 100644 src/temp/mkostemp.c
 create mode 100644 src/temp/mkostemps.c
 create mode 100644 src/temp/mkstemps.c

diff --git a/include/stdlib.h b/include/stdlib.h
index 671d188..4210f40 100644
--- a/include/stdlib.h
+++ b/include/stdlib.h
@@ -95,6 +95,9 @@ int posix_memalign (void **, size_t, size_t);
 int setenv (const char *, const char *, int);
 int unsetenv (const char *);
 int mkstemp (char *);
+int mkostemp (char *, int);
+int mkstemps (char *, int);
+int mkostemps (char *, int, int);
 char *mkdtemp (char *);
 int getsubopt (char **, char *const *, char **);
 int rand_r (unsigned *);
@@ -150,6 +153,9 @@ char *gcvt(double, int, char *);
 
 #if defined(_LARGEFILE64_SOURCE) || defined(_GNU_SOURCE)
 #define mkstemp64 mkstemp
+#define mkostemp64 mkostemp
+#define mkstemps64 mkstemps
+#define mkostemps64 mkostemps
 #endif
 
 #ifdef __cplusplus
diff --git a/src/temp/__open_tempfile.c b/src/temp/__open_tempfile.c
new file mode 100644
index 0000000..797d3b3
--- /dev/null
+++ b/src/temp/__open_tempfile.c
@@ -0,0 +1,27 @@
+#include <string.h>
+#include <fcntl.h>
+#include <unistd.h>
+#include <errno.h>
+
+char *__randname(char *);
+
+int __open_tempfile (char *template, int len, int flags)
+{
+	if (len < 0) return EINVAL;
+
+	size_t l = strlen(template)-len;
+	if (l < 6 || strncmp(template+l-6, "XXXXXX", 6)) {
+		errno = EINVAL;
+		*template = 0;
+		return -1;
+	}
+
+	int fd, retries = 100;
+	while (retries--) {
+		__randname(template+l-6);
+		if ((fd = open(template, flags | O_CREAT | O_EXCL, 0600))>=0)
+			return fd;
+		if (errno != EEXIST) return -1;
+	}
+	return -1;
+}
diff --git a/src/temp/__randname.c b/src/temp/__randname.c
new file mode 100644
index 0000000..0dd5725
--- /dev/null
+++ b/src/temp/__randname.c
@@ -0,0 +1,19 @@
+#include <string.h>
+#include <time.h>
+#include <stdint.h>
+
+/* This assumes that a check for the
+   template size has alrady been made */
+char *__randname(char *template)
+{
+	int i;
+	struct timespec ts;
+	unsigned long r;
+
+	clock_gettime(CLOCK_REALTIME, &ts);
+	r = ts.tv_nsec*65537 ^ (uintptr_t)&ts / 16 + (uintptr_t)template;
+	for (i=0; i<6; i++, r>>=5)
+		template[i] = 'A'+(r&15)+(r&16)*2;
+
+	return template;
+}
diff --git a/src/temp/mkostemp.c b/src/temp/mkostemp.c
new file mode 100644
index 0000000..92a7bc4
--- /dev/null
+++ b/src/temp/mkostemp.c
@@ -0,0 +1,11 @@
+#define _GNU_SOURCE
+#include "libc.h"
+
+int __open_tempfile (char *, int, int);
+
+int __mkostemp(char *template, int flags)
+{
+	return __open_tempfile (template, 0, flags);
+}
+
+weak_alias(__mkostemp, mkostemp);
diff --git a/src/temp/mkostemps.c b/src/temp/mkostemps.c
new file mode 100644
index 0000000..6c60323
--- /dev/null
+++ b/src/temp/mkostemps.c
@@ -0,0 +1,11 @@
+#define _GNU_SOURCE
+#include "libc.h"
+
+int __open_tempfile (char *, int, int);
+
+int __mkostemps(char *template, int len, int flags)
+{
+	return __open_tempfile (template, len, flags);
+}
+
+weak_alias(__mkostemps, mkostemps);
diff --git a/src/temp/mkstemp.c b/src/temp/mkstemp.c
index a390d42..72ab7d1 100644
--- a/src/temp/mkstemp.c
+++ b/src/temp/mkstemp.c
@@ -1,28 +1,11 @@
-#include <string.h>
-#include <stdio.h>
-#include <stdlib.h>
 #include <fcntl.h>
-#include <unistd.h>
-#include <limits.h>
-#include <errno.h>
 #include "libc.h"
 
-char *__mktemp(char *);
+int __open_tempfile (char *, int, int);
 
 int mkstemp(char *template)
 {
-	int fd, retries = 100, t0 = *template;
-	while (retries--) {
-		if (!*__mktemp(template)) return -1;
-		if ((fd = open(template, O_RDWR | O_CREAT | O_EXCL, 0600))>=0)
-			return fd;
-		if (errno != EEXIST) return -1;
-		/* this is safe because mktemp verified
-		 * that we have a valid template string */
-		template[0] = t0;
-		strcpy(template+strlen(template)-6, "XXXXXX");
-	}
-	return -1;
+	return __open_tempfile (template, 0, O_RDWR);
 }
 
 LFS64(mkstemp);
diff --git a/src/temp/mkstemps.c b/src/temp/mkstemps.c
new file mode 100644
index 0000000..fcfc010
--- /dev/null
+++ b/src/temp/mkstemps.c
@@ -0,0 +1,12 @@
+#define _GNU_SOURCE
+#include <fcntl.h>
+#include "libc.h"
+
+int __open_tempfile (char *, int, int);
+
+int __mkstemps(char *template, int len)
+{
+	return __open_tempfile (template, len, O_RDWR);
+}
+
+weak_alias(__mkstemps, mkstemps);
diff --git a/src/temp/mktemp.c b/src/temp/mktemp.c
index c0e06f5..ed2c103 100644
--- a/src/temp/mktemp.c
+++ b/src/temp/mktemp.c
@@ -1,17 +1,14 @@
 #include <string.h>
-#include <stdio.h>
-#include <stdlib.h>
 #include <fcntl.h>
 #include <unistd.h>
 #include <errno.h>
-#include <time.h>
-#include <stdint.h>
 #include "libc.h"
 
+char *__randname(char *);
+
 char *__mktemp(char *template)
 {
-	struct timespec ts;
-	size_t i, l = strlen(template);
+	size_t l = strlen(template);
 	int retries = 10000;
 	unsigned long r;
 
@@ -21,10 +18,7 @@ char *__mktemp(char *template)
 		return template;
 	}
 	while (retries--) {
-		clock_gettime(CLOCK_REALTIME, &ts);
-		r = ts.tv_nsec + (uintptr_t)&ts / 16 + (uintptr_t)template;
-		for (i=1; i<=6; i++, r>>=4)
-			template[l-i] = 'A'+(r&15);
+		__randname(template+l-6);
 		if (access(template, F_OK) < 0) return template;
 	}
 	*template = 0;
-- 
1.7.12.4



^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH] Add support for mkostemp, mkstemps and mkostemps
  2013-02-02 20:14 ` Szabolcs Nagy
@ 2013-02-02 20:38   ` Anthony G. Basile
  2013-02-02 22:22     ` Szabolcs Nagy
  0 siblings, 1 reply; 27+ messages in thread
From: Anthony G. Basile @ 2013-02-02 20:38 UTC (permalink / raw)
  To: musl

Okay let me try those fixes.  Some points:

1. I forgot to remove a group of headers when I refactorized.  So 
mkostemp.c mkostemps.c mkstemp.c and mkstemps.c only need

#include <fcntl.h>
#include "libc.h"

or

#define _GNU_SOURCE
#include "libc.h"


2. This is from uclibc.  Clearly, static is critical here, but still 
they never initialize a value of 'value' on first entry into the 
function, so that memory is dirty to start.  Mine is worse, but I wonder 
if this is still a bug there.

static void brain_damaged_fillrand(unsigned char *buf, unsigned int len)
{
	...
         static uint64_t value;
         gettimeofday(&tv, NULL);
         value += ((uint64_t) tv.tv_usec << 16) ^ tv.tv_sec ^ getpid();
	...


3. I retested your address approach.  I like it but it only maps to 
upper and lower case letters, no numbers which uclibc and glibc do.

clock_gettime(CLOCK_REALTIME, &ts);
r = ts.tv_nsec*65537 ^ (uintptr_t)&ts / 16 + (uintptr_t)template;
for (i=0; i<6; i++, r>>=5)
	template[i] = 'A'+(r&15)+(r&16)*2;




On 02/02/2013 03:14 PM, Szabolcs Nagy wrote:
> * Anthony G. Basile <basile@opensource.dyc.edu> [2013-02-02 13:45:31 -0500]:
>> +	/* Null terminate the template before the suffix,
>> +	   and save the char for adding back the suffix */
>> +	char suffix = template[l];
>> +	template[l] = '\0';
>
> if you set only the XXXXXX part in __randname, then the \0 is unnecessary
>
>> +	int fd, retries = 100;
>> +	while (retries--) {
>> +		if (!*__randname(template)) return -1;
>
> __randname cannot fail, so the check is unnecessary
>
>> +/* This assumes that a check for the
>> +   template size has alrady been made */
>> +char *__randname(char *template)
>> +{
>> +	struct timespec ts;
>> +	size_t i, l = strlen(template);
>> +
>> +	/* r is intentially uninialized and 'dirty' */
>> +	unsigned long r;
>> +
>
> it's undefined behaviour so the compiler is allowed to
> completely remove the code of this function
>
> if you seen this kind of code somewhere, that's a critical
> bug that should be reported
>
> the original address based entropy source was ok
>
>> +	clock_gettime(CLOCK_REALTIME, &ts);
>> +	r += ((uint64_t) ts.tv_nsec << 16) ^ ts.tv_sec;
>> +	for (i=1; i<=6; i++, r>>=6)
>> +		template[l-i] = __map_letter(r);
>> +
>> +	return template;
>> +}


-- 
Anthony G. Basile, Ph. D.
Chair of Information Technology
D'Youville College
Buffalo, NY 14201
(716) 829-8197


^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH] Add support for mkostemp, mkstemps and mkostemps
  2013-02-02 18:45 Anthony G. Basile
  2013-02-02 19:51 ` John Spencer
@ 2013-02-02 20:14 ` Szabolcs Nagy
  2013-02-02 20:38   ` Anthony G. Basile
  1 sibling, 1 reply; 27+ messages in thread
From: Szabolcs Nagy @ 2013-02-02 20:14 UTC (permalink / raw)
  To: musl

* Anthony G. Basile <basile@opensource.dyc.edu> [2013-02-02 13:45:31 -0500]:
> +	/* Null terminate the template before the suffix,
> +	   and save the char for adding back the suffix */
> +	char suffix = template[l];
> +	template[l] = '\0';

if you set only the XXXXXX part in __randname, then the \0 is unnecessary

> +	int fd, retries = 100;
> +	while (retries--) {
> +		if (!*__randname(template)) return -1;

__randname cannot fail, so the check is unnecessary

> +/* This assumes that a check for the
> +   template size has alrady been made */
> +char *__randname(char *template)
> +{
> +	struct timespec ts;
> +	size_t i, l = strlen(template);
> +
> +	/* r is intentially uninialized and 'dirty' */
> +	unsigned long r;
> +

it's undefined behaviour so the compiler is allowed to
completely remove the code of this function

if you seen this kind of code somewhere, that's a critical
bug that should be reported

the original address based entropy source was ok

> +	clock_gettime(CLOCK_REALTIME, &ts);
> +	r += ((uint64_t) ts.tv_nsec << 16) ^ ts.tv_sec;
> +	for (i=1; i<=6; i++, r>>=6)
> +		template[l-i] = __map_letter(r);
> +
> +	return template;
> +}


^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH] Add support for mkostemp, mkstemps and mkostemps
  2013-02-02 19:51 ` John Spencer
@ 2013-02-02 19:59   ` John Spencer
  2013-02-02 22:11   ` Szabolcs Nagy
  1 sibling, 0 replies; 27+ messages in thread
From: John Spencer @ 2013-02-02 19:59 UTC (permalink / raw)
  To: musl

[-- Attachment #1: Type: text/plain, Size: 115 bytes --]

On 02/02/2013 08:51 PM, John Spencer wrote:
> here's my version:
>
>
oops, forgot to pass O_RDWR, here's a fixup



[-- Attachment #2: 0001-fix-O_RDWR-not-being-passed-to-mkostemps.patch --]
[-- Type: text/x-patch, Size: 1147 bytes --]

From ebdecbd45156c5525fe2483982c77fab484bb946 Mon Sep 17 00:00:00 2001
From: rofl0r <retnyg@gmx.net>
Date: Sat, 2 Feb 2013 20:58:32 +0100
Subject: [PATCH] fix O_RDWR not being passed to mkostemps

---
 src/temp/mkstemp.c  |    3 ++-
 src/temp/mkstemps.c |    3 ++-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/src/temp/mkstemp.c b/src/temp/mkstemp.c
index 07ec4d7..17bfcbb 100644
--- a/src/temp/mkstemp.c
+++ b/src/temp/mkstemp.c
@@ -1,10 +1,11 @@
 #include "libc.h"
+#include <fcntl.h>
 
 int __mkostemps (char *template, int suffixlen, int flags);
 
 int mkstemp(char *template)
 {
-	return __mkostemps(template, 0, 0);
+	return __mkostemps(template, 0, O_RDWR);
 }
 
 LFS64(mkstemp);
diff --git a/src/temp/mkstemps.c b/src/temp/mkstemps.c
index 6d16d9c..caad441 100644
--- a/src/temp/mkstemps.c
+++ b/src/temp/mkstemps.c
@@ -1,10 +1,11 @@
+#include <fcntl.h>
 #include "libc.h"
 
 int __mkostemps (char *template, int suffixlen, int flags);
 
 int mkstemps (char *template, int suffixlen)
 {
-	return __mkostemps(template, suffixlen, 0);
+	return __mkostemps(template, suffixlen, O_RDWR);
 }
 
 LFS64(mkstemps);
-- 
1.7.3.4


^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH] Add support for mkostemp, mkstemps and mkostemps
  2013-02-02 18:45 Anthony G. Basile
@ 2013-02-02 19:51 ` John Spencer
  2013-02-02 19:59   ` John Spencer
  2013-02-02 22:11   ` Szabolcs Nagy
  2013-02-02 20:14 ` Szabolcs Nagy
  1 sibling, 2 replies; 27+ messages in thread
From: John Spencer @ 2013-02-02 19:51 UTC (permalink / raw)
  To: musl

[-- Attachment #1: Type: text/plain, Size: 21 bytes --]

here's my version:



[-- Attachment #2: 0001-refactor-mktemp-code-and-add-mkstemps-mkostemp-s.patch --]
[-- Type: text/x-patch, Size: 4864 bytes --]

From 59fa03a4a72bac8119fa77f6450cd4fae4446911 Mon Sep 17 00:00:00 2001
From: rofl0r <retnyg@gmx.net>
Date: Sat, 2 Feb 2013 20:48:26 +0100
Subject: [PATCH] refactor mktemp code and add mkstemps, mkostemp(s)

loosely based on patch by Anthony G. Basile
---
 include/stdlib.h     |    7 +++++++
 src/temp/mkostemp.c  |   10 ++++++++++
 src/temp/mkostemps.c |   29 +++++++++++++++++++++++++++++
 src/temp/mkstemp.c   |   22 ++--------------------
 src/temp/mkstemps.c  |   10 ++++++++++
 src/temp/mktemp.c    |   21 +++++++++++++--------
 6 files changed, 71 insertions(+), 28 deletions(-)
 create mode 100644 src/temp/mkostemp.c
 create mode 100644 src/temp/mkostemps.c
 create mode 100644 src/temp/mkstemps.c

diff --git a/include/stdlib.h b/include/stdlib.h
index 671d188..a2aafde 100644
--- a/include/stdlib.h
+++ b/include/stdlib.h
@@ -134,6 +134,10 @@ void lcong48 (unsigned short [7]);
 #if defined(_GNU_SOURCE) || defined(_BSD_SOURCE)
 #include <alloca.h>
 char *mktemp (char *);
+int mkostemp (char *, int);
+int mkstemps (char *, int);
+int mkostemps (char *, int, int);
+
 void *valloc (size_t);
 void *memalign(size_t, size_t);
 #define WCOREDUMP(s) ((s) & 0x80)
@@ -150,6 +154,9 @@ char *gcvt(double, int, char *);
 
 #if defined(_LARGEFILE64_SOURCE) || defined(_GNU_SOURCE)
 #define mkstemp64 mkstemp
+#define mkostemp64 mkostemp
+#define mkstemps64 mkstemps
+#define mkostemps64 mkostemps
 #endif
 
 #ifdef __cplusplus
diff --git a/src/temp/mkostemp.c b/src/temp/mkostemp.c
new file mode 100644
index 0000000..f3e45cb
--- /dev/null
+++ b/src/temp/mkostemp.c
@@ -0,0 +1,10 @@
+#include "libc.h"
+
+int __mkostemps (char *template, int suffixlen, int flags);
+
+int mkostemp(char *template, int flags)
+{
+	return __mkostemps(template, 0, flags);
+}
+
+LFS64(mkostemp);
diff --git a/src/temp/mkostemps.c b/src/temp/mkostemps.c
new file mode 100644
index 0000000..d389ec8
--- /dev/null
+++ b/src/temp/mkostemps.c
@@ -0,0 +1,29 @@
+#include <fcntl.h>
+#include <string.h>
+#include <errno.h>
+#include "libc.h"
+
+void __randname(char *);
+
+int __mkostemps (char *template, int suffixlen, int flags)
+{
+	size_t l = strlen(template);
+	char *x6 = template + l - suffixlen;
+	if(l - suffixlen < 6 || memcmp(x6, "XXXXXX", 6)) {
+		errno = EINVAL;
+		*template = 0;
+		return -1;
+	}
+
+	int fd, retries = 100;
+	while (retries--) {
+		__randname(x6);
+		if ((fd = open(template, flags | O_CREAT | O_EXCL, 0600))>=0)
+			return fd;
+		if (errno != EEXIST) return -1;
+	}
+	return -1;
+}
+
+weak_alias(__mkostemps, mkostemps);
+LFS64(mkostemps);
diff --git a/src/temp/mkstemp.c b/src/temp/mkstemp.c
index a390d42..07ec4d7 100644
--- a/src/temp/mkstemp.c
+++ b/src/temp/mkstemp.c
@@ -1,28 +1,10 @@
-#include <string.h>
-#include <stdio.h>
-#include <stdlib.h>
-#include <fcntl.h>
-#include <unistd.h>
-#include <limits.h>
-#include <errno.h>
 #include "libc.h"
 
-char *__mktemp(char *);
+int __mkostemps (char *template, int suffixlen, int flags);
 
 int mkstemp(char *template)
 {
-	int fd, retries = 100, t0 = *template;
-	while (retries--) {
-		if (!*__mktemp(template)) return -1;
-		if ((fd = open(template, O_RDWR | O_CREAT | O_EXCL, 0600))>=0)
-			return fd;
-		if (errno != EEXIST) return -1;
-		/* this is safe because mktemp verified
-		 * that we have a valid template string */
-		template[0] = t0;
-		strcpy(template+strlen(template)-6, "XXXXXX");
-	}
-	return -1;
+	return __mkostemps(template, 0, 0);
 }
 
 LFS64(mkstemp);
diff --git a/src/temp/mkstemps.c b/src/temp/mkstemps.c
new file mode 100644
index 0000000..6d16d9c
--- /dev/null
+++ b/src/temp/mkstemps.c
@@ -0,0 +1,10 @@
+#include "libc.h"
+
+int __mkostemps (char *template, int suffixlen, int flags);
+
+int mkstemps (char *template, int suffixlen)
+{
+	return __mkostemps(template, suffixlen, 0);
+}
+
+LFS64(mkstemps);
diff --git a/src/temp/mktemp.c b/src/temp/mktemp.c
index c0e06f5..a22f69c 100644
--- a/src/temp/mktemp.c
+++ b/src/temp/mktemp.c
@@ -8,23 +8,28 @@
 #include <stdint.h>
 #include "libc.h"
 
-char *__mktemp(char *template)
+void __randname(char *x6)
 {
+	size_t i = 0;
 	struct timespec ts;
-	size_t i, l = strlen(template);
+	clock_gettime(CLOCK_REALTIME, &ts);
+	unsigned long r = ts.tv_nsec + (uintptr_t)&ts / 16 + (uintptr_t)x6;
+	for (; i<6; i++, r>>=4) x6[i] = 'A'+(r&15);
+}
+
+char *__mktemp(char *template)
+{
 	int retries = 10000;
-	unsigned long r;
+	size_t l = strlen(template);
+	char *x6 = template +l -6;
 
-	if (l < 6 || strcmp(template+l-6, "XXXXXX")) {
+	if(l < 6 || memcmp(x6, "XXXXXX", 6)) {
 		errno = EINVAL;
 		*template = 0;
 		return template;
 	}
 	while (retries--) {
-		clock_gettime(CLOCK_REALTIME, &ts);
-		r = ts.tv_nsec + (uintptr_t)&ts / 16 + (uintptr_t)template;
-		for (i=1; i<=6; i++, r>>=4)
-			template[l-i] = 'A'+(r&15);
+		__randname(x6);
 		if (access(template, F_OK) < 0) return template;
 	}
 	*template = 0;
-- 
1.7.3.4


^ permalink raw reply	[flat|nested] 27+ messages in thread

* [PATCH] Add support for mkostemp, mkstemps and mkostemps
@ 2013-02-02 18:45 Anthony G. Basile
  2013-02-02 19:51 ` John Spencer
  2013-02-02 20:14 ` Szabolcs Nagy
  0 siblings, 2 replies; 27+ messages in thread
From: Anthony G. Basile @ 2013-02-02 18:45 UTC (permalink / raw)
  To: musl; +Cc: Anthony G. Basile

From: "Anthony G. Basile" <blueness@gentoo.org>

Signed-off-by: Anthony G. Basile <blueness@gentoo.org>

1
---
 include/stdlib.h           |  6 ++++++
 src/temp/__open_tempfile.c | 35 +++++++++++++++++++++++++++++++++++
 src/temp/__randname.c      | 35 +++++++++++++++++++++++++++++++++++
 src/temp/mkostemp.c        | 18 ++++++++++++++++++
 src/temp/mkostemps.c       | 18 ++++++++++++++++++
 src/temp/mkstemp.c         | 15 ++-------------
 src/temp/mkstemps.c        | 18 ++++++++++++++++++
 src/temp/mktemp.c          |  7 +++----
 8 files changed, 135 insertions(+), 17 deletions(-)
 create mode 100644 src/temp/__open_tempfile.c
 create mode 100644 src/temp/__randname.c
 create mode 100644 src/temp/mkostemp.c
 create mode 100644 src/temp/mkostemps.c
 create mode 100644 src/temp/mkstemps.c

diff --git a/include/stdlib.h b/include/stdlib.h
index 671d188..4210f40 100644
--- a/include/stdlib.h
+++ b/include/stdlib.h
@@ -95,6 +95,9 @@ int posix_memalign (void **, size_t, size_t);
 int setenv (const char *, const char *, int);
 int unsetenv (const char *);
 int mkstemp (char *);
+int mkostemp (char *, int);
+int mkstemps (char *, int);
+int mkostemps (char *, int, int);
 char *mkdtemp (char *);
 int getsubopt (char **, char *const *, char **);
 int rand_r (unsigned *);
@@ -150,6 +153,9 @@ char *gcvt(double, int, char *);
 
 #if defined(_LARGEFILE64_SOURCE) || defined(_GNU_SOURCE)
 #define mkstemp64 mkstemp
+#define mkostemp64 mkostemp
+#define mkstemps64 mkstemps
+#define mkostemps64 mkostemps
 #endif
 
 #ifdef __cplusplus
diff --git a/src/temp/__open_tempfile.c b/src/temp/__open_tempfile.c
new file mode 100644
index 0000000..7797e01
--- /dev/null
+++ b/src/temp/__open_tempfile.c
@@ -0,0 +1,35 @@
+#include <string.h>
+#include <stdlib.h>
+#include <fcntl.h>
+#include <unistd.h>
+#include <errno.h>
+
+char *__randname(char *);
+
+int __open_tempfile (char *template, int len, int flags)
+{
+	if (len < 0) return EINVAL;
+
+	size_t l = strlen(template)-len;
+	if (l < 6 || strncmp(template+l-6, "XXXXXX", 6)) {
+		errno = EINVAL;
+		*template = 0;
+		return -1;
+	}
+
+	/* Null terminate the template before the suffix,
+	   and save the char for adding back the suffix */
+	char suffix = template[l];
+	template[l] = '\0';
+
+	int fd, retries = 100;
+	while (retries--) {
+		if (!*__randname(template)) return -1;
+		/* Add back the suffix */
+		template[l] = suffix;
+		if ((fd = open(template, flags | O_CREAT | O_EXCL, 0600))>=0)
+			return fd;
+		if (errno != EEXIST) return -1;
+	}
+	return -1;
+}
diff --git a/src/temp/__randname.c b/src/temp/__randname.c
new file mode 100644
index 0000000..ac48545
--- /dev/null
+++ b/src/temp/__randname.c
@@ -0,0 +1,35 @@
+#include <string.h>
+#include <unistd.h>
+#include <time.h>
+#include <stdint.h>
+
+/* We prefer this to a static const char[] array for a smaller footprint */
+
+char __map_letter(unsigned long x)
+{
+	x %= 62;
+	if (x<10)
+		return '0'+x;
+	else if (x<36)
+		return 'a'+(x-10);
+	else
+		return 'A'+(x-36);
+}
+
+/* This assumes that a check for the
+   template size has alrady been made */
+char *__randname(char *template)
+{
+	struct timespec ts;
+	size_t i, l = strlen(template);
+
+	/* r is intentially uninialized and 'dirty' */
+	unsigned long r;
+
+	clock_gettime(CLOCK_REALTIME, &ts);
+	r += ((uint64_t) ts.tv_nsec << 16) ^ ts.tv_sec;
+	for (i=1; i<=6; i++, r>>=6)
+		template[l-i] = __map_letter(r);
+
+	return template;
+}
diff --git a/src/temp/mkostemp.c b/src/temp/mkostemp.c
new file mode 100644
index 0000000..750d880
--- /dev/null
+++ b/src/temp/mkostemp.c
@@ -0,0 +1,18 @@
+#define _GNU_SOURCE
+#include <string.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <fcntl.h>
+#include <unistd.h>
+#include <limits.h>
+#include <errno.h>
+#include "libc.h"
+
+int __open_tempfile (char *, int, int);
+
+int __mkostemp(char *template, int flags)
+{
+	return __open_tempfile (template, 0, flags);
+}
+
+weak_alias(__mkostemp, mkostemp);
diff --git a/src/temp/mkostemps.c b/src/temp/mkostemps.c
new file mode 100644
index 0000000..8c810ce
--- /dev/null
+++ b/src/temp/mkostemps.c
@@ -0,0 +1,18 @@
+#define _GNU_SOURCE
+#include <string.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <fcntl.h>
+#include <unistd.h>
+#include <limits.h>
+#include <errno.h>
+#include "libc.h"
+
+int __open_tempfile (char *, int, int);
+
+int __mkostemps(char *template, int len, int flags)
+{
+	return __open_tempfile (template, len, flags);
+}
+
+weak_alias(__mkostemps, mkostemps);
diff --git a/src/temp/mkstemp.c b/src/temp/mkstemp.c
index a390d42..ccaf3c6 100644
--- a/src/temp/mkstemp.c
+++ b/src/temp/mkstemp.c
@@ -7,22 +7,11 @@
 #include <errno.h>
 #include "libc.h"
 
-char *__mktemp(char *);
+int __open_tempfile (char *, int, int);
 
 int mkstemp(char *template)
 {
-	int fd, retries = 100, t0 = *template;
-	while (retries--) {
-		if (!*__mktemp(template)) return -1;
-		if ((fd = open(template, O_RDWR | O_CREAT | O_EXCL, 0600))>=0)
-			return fd;
-		if (errno != EEXIST) return -1;
-		/* this is safe because mktemp verified
-		 * that we have a valid template string */
-		template[0] = t0;
-		strcpy(template+strlen(template)-6, "XXXXXX");
-	}
-	return -1;
+	return __open_tempfile (template, 0, O_RDWR);
 }
 
 LFS64(mkstemp);
diff --git a/src/temp/mkstemps.c b/src/temp/mkstemps.c
new file mode 100644
index 0000000..53fea07
--- /dev/null
+++ b/src/temp/mkstemps.c
@@ -0,0 +1,18 @@
+#define _GNU_SOURCE
+#include <string.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <fcntl.h>
+#include <unistd.h>
+#include <limits.h>
+#include <errno.h>
+#include "libc.h"
+
+int __open_tempfile (char *, int, int);
+
+int __mkstemps(char *template, int len)
+{
+	return __open_tempfile (template, len, O_RDWR);
+}
+
+weak_alias(__mkstemps, mkstemps);
diff --git a/src/temp/mktemp.c b/src/temp/mktemp.c
index c0e06f5..de1afb4 100644
--- a/src/temp/mktemp.c
+++ b/src/temp/mktemp.c
@@ -8,6 +8,8 @@
 #include <stdint.h>
 #include "libc.h"
 
+char *__randname(char *);
+
 char *__mktemp(char *template)
 {
 	struct timespec ts;
@@ -21,10 +23,7 @@ char *__mktemp(char *template)
 		return template;
 	}
 	while (retries--) {
-		clock_gettime(CLOCK_REALTIME, &ts);
-		r = ts.tv_nsec + (uintptr_t)&ts / 16 + (uintptr_t)template;
-		for (i=1; i<=6; i++, r>>=4)
-			template[l-i] = 'A'+(r&15);
+		__randname(template);
 		if (access(template, F_OK) < 0) return template;
 	}
 	*template = 0;
-- 
1.7.12.4



^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH] Add support for mkostemp, mkstemps and mkostemps
  2013-01-28  2:36 Anthony G. Basile
@ 2013-01-28  5:01 ` Anthony G. Basile
  0 siblings, 0 replies; 27+ messages in thread
From: Anthony G. Basile @ 2013-01-28  5:01 UTC (permalink / raw)
  To: musl

Okay after some chat on IRC, my next attemp will change the following:

1) Use weak_alias for mkostemp, mkostemps and mkstemps

2) Don't use malloc to copy the suffix (which I forgot to free anyhow), 
but simply save the one initial char so that we can cut the XXXXXX from 
the suffix and restore it.

3) Refactorize mktemp so that the generation of the random chars to 
replace XXXXXX is separated from the while(retries--) { access(...); }  
Use __randname for mkstemp and friends which don't need the access check 
with O_EXCL.

4) Rename __gen_tempname to __open_tempfile which more clearly says what 
it does.

Okay ... patch coming in my next email to the list.


On 01/27/2013 09:36 PM, Anthony G. Basile wrote:
> From: "Anthony G. Basile" <basile@opensource.dyc.edu>
>
> Signed-off-by: Anthony G. Basile <blueness@gentoo.org>
> ---
>   include/stdlib.h     |  6 ++++++
>   src/temp/mkostemp.c  | 16 ++++++++++++++++
>   src/temp/mkostemps.c | 16 ++++++++++++++++
>   src/temp/mkstemp.c   | 15 ++-------------
>   src/temp/mkstemps.c  | 16 ++++++++++++++++
>   src/temp/tempname.c  | 42 ++++++++++++++++++++++++++++++++++++++++++
>   6 files changed, 98 insertions(+), 13 deletions(-)
>   create mode 100644 src/temp/mkostemp.c
>   create mode 100644 src/temp/mkostemps.c
>   create mode 100644 src/temp/mkstemps.c
>   create mode 100644 src/temp/tempname.c
>
> diff --git a/include/stdlib.h b/include/stdlib.h
> index 671d188..4210f40 100644
> --- a/include/stdlib.h
> +++ b/include/stdlib.h
> @@ -95,6 +95,9 @@ int posix_memalign (void **, size_t, size_t);
>   int setenv (const char *, const char *, int);
>   int unsetenv (const char *);
>   int mkstemp (char *);
> +int mkostemp (char *, int);
> +int mkstemps (char *, int);
> +int mkostemps (char *, int, int);
>   char *mkdtemp (char *);
>   int getsubopt (char **, char *const *, char **);
>   int rand_r (unsigned *);
> @@ -150,6 +153,9 @@ char *gcvt(double, int, char *);
>   
>   #if defined(_LARGEFILE64_SOURCE) || defined(_GNU_SOURCE)
>   #define mkstemp64 mkstemp
> +#define mkostemp64 mkostemp
> +#define mkstemps64 mkstemps
> +#define mkostemps64 mkostemps
>   #endif
>   
>   #ifdef __cplusplus
> diff --git a/src/temp/mkostemp.c b/src/temp/mkostemp.c
> new file mode 100644
> index 0000000..4fd374c
> --- /dev/null
> +++ b/src/temp/mkostemp.c
> @@ -0,0 +1,16 @@
> +#define _GNU_SOURCE
> +#include <string.h>
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <fcntl.h>
> +#include <unistd.h>
> +#include <limits.h>
> +#include <errno.h>
> +#include "libc.h"
> +
> +int __gen_tempname (char *, int, int);
> +
> +int mkostemp(char *template, int flags)
> +{
> +	return __gen_tempname (template, 0, flags);
> +}
> diff --git a/src/temp/mkostemps.c b/src/temp/mkostemps.c
> new file mode 100644
> index 0000000..9affae3
> --- /dev/null
> +++ b/src/temp/mkostemps.c
> @@ -0,0 +1,16 @@
> +#define _GNU_SOURCE
> +#include <string.h>
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <fcntl.h>
> +#include <unistd.h>
> +#include <limits.h>
> +#include <errno.h>
> +#include "libc.h"
> +
> +int __gen_tempname (char *, int, int);
> +
> +int mkostemps(char *template, int len, int flags)
> +{
> +	return __gen_tempname (template, len, flags);
> +}
> diff --git a/src/temp/mkstemp.c b/src/temp/mkstemp.c
> index a390d42..08914d4 100644
> --- a/src/temp/mkstemp.c
> +++ b/src/temp/mkstemp.c
> @@ -7,22 +7,11 @@
>   #include <errno.h>
>   #include "libc.h"
>   
> -char *__mktemp(char *);
> +int __gen_tempname (char *, int, int);
>   
>   int mkstemp(char *template)
>   {
> -	int fd, retries = 100, t0 = *template;
> -	while (retries--) {
> -		if (!*__mktemp(template)) return -1;
> -		if ((fd = open(template, O_RDWR | O_CREAT | O_EXCL, 0600))>=0)
> -			return fd;
> -		if (errno != EEXIST) return -1;
> -		/* this is safe because mktemp verified
> -		 * that we have a valid template string */
> -		template[0] = t0;
> -		strcpy(template+strlen(template)-6, "XXXXXX");
> -	}
> -	return -1;
> +	return __gen_tempname (template, 0, O_RDWR);
>   }
>   
>   LFS64(mkstemp);
> diff --git a/src/temp/mkstemps.c b/src/temp/mkstemps.c
> new file mode 100644
> index 0000000..e194444
> --- /dev/null
> +++ b/src/temp/mkstemps.c
> @@ -0,0 +1,16 @@
> +#define _GNU_SOURCE
> +#include <string.h>
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <fcntl.h>
> +#include <unistd.h>
> +#include <limits.h>
> +#include <errno.h>
> +#include "libc.h"
> +
> +int __gen_tempname (char *, int, int);
> +
> +int mkstemps(char *template, int len)
> +{
> +	return __gen_tempname (template, len, O_RDWR);
> +}
> diff --git a/src/temp/tempname.c b/src/temp/tempname.c
> new file mode 100644
> index 0000000..1c198bf
> --- /dev/null
> +++ b/src/temp/tempname.c
> @@ -0,0 +1,42 @@
> +#include <string.h>
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <fcntl.h>
> +#include <unistd.h>
> +#include <limits.h>
> +#include <errno.h>
> +#include "libc.h"
> +
> +char *__mktemp(char *);
> +
> +int __gen_tempname (char *template, int len, int flags)
> +{
> +	if (len < 0) return EINVAL;
> +
> +	int templen = strlen(template)-len;
> +	if (templen<6) return EINVAL;
> +
> +	char *suffix = (char *)malloc((len+1)*sizeof(char));
> +	/* Copy the last len chars plus the null termination */
> +	int i;
> +	for (i = 0; i <= len; i++)
> +		suffix[i] = template[templen+i];
> +	/* Null terminate the template before the suffice */
> +	template[templen] = '\0';
> +
> +	int fd, retries = 100, t0 = *template;
> +	while (retries--) {
> +		if (!*__mktemp(template)) return -1;
> +		/* Copy back the suffix */
> +		for (i = 0; i <= len; i++)
> +			template[templen+i] = suffix[i];
> +		if ((fd = open(template, flags | O_CREAT | O_EXCL, 0600))>=0)
> +			return fd;
> +		if (errno != EEXIST) return -1;
> +		/* this is safe because mktemp verified
> +		 * that we have a valid template string */
> +		template[0] = t0;
> +		strcpy(template+templen-6, "XXXXXX");
> +	}
> +	return -1;
> +}


-- 
Anthony G. Basile, Ph.D.
Gentoo Linux Developer [Hardened]
E-Mail    : blueness@gentoo.org
GnuPG FP  : 1FED FAD9 D82C 52A5 3BAB  DC79 9384 FA6E F52D 4BBA
GnuPG ID  : F52D4BBA



^ permalink raw reply	[flat|nested] 27+ messages in thread

* [PATCH] Add support for mkostemp, mkstemps and mkostemps
@ 2013-01-28  2:36 Anthony G. Basile
  2013-01-28  5:01 ` Anthony G. Basile
  0 siblings, 1 reply; 27+ messages in thread
From: Anthony G. Basile @ 2013-01-28  2:36 UTC (permalink / raw)
  To: musl; +Cc: Anthony G. Basile, Anthony G. Basile

From: "Anthony G. Basile" <basile@opensource.dyc.edu>

Signed-off-by: Anthony G. Basile <blueness@gentoo.org>
---
 include/stdlib.h     |  6 ++++++
 src/temp/mkostemp.c  | 16 ++++++++++++++++
 src/temp/mkostemps.c | 16 ++++++++++++++++
 src/temp/mkstemp.c   | 15 ++-------------
 src/temp/mkstemps.c  | 16 ++++++++++++++++
 src/temp/tempname.c  | 42 ++++++++++++++++++++++++++++++++++++++++++
 6 files changed, 98 insertions(+), 13 deletions(-)
 create mode 100644 src/temp/mkostemp.c
 create mode 100644 src/temp/mkostemps.c
 create mode 100644 src/temp/mkstemps.c
 create mode 100644 src/temp/tempname.c

diff --git a/include/stdlib.h b/include/stdlib.h
index 671d188..4210f40 100644
--- a/include/stdlib.h
+++ b/include/stdlib.h
@@ -95,6 +95,9 @@ int posix_memalign (void **, size_t, size_t);
 int setenv (const char *, const char *, int);
 int unsetenv (const char *);
 int mkstemp (char *);
+int mkostemp (char *, int);
+int mkstemps (char *, int);
+int mkostemps (char *, int, int);
 char *mkdtemp (char *);
 int getsubopt (char **, char *const *, char **);
 int rand_r (unsigned *);
@@ -150,6 +153,9 @@ char *gcvt(double, int, char *);
 
 #if defined(_LARGEFILE64_SOURCE) || defined(_GNU_SOURCE)
 #define mkstemp64 mkstemp
+#define mkostemp64 mkostemp
+#define mkstemps64 mkstemps
+#define mkostemps64 mkostemps
 #endif
 
 #ifdef __cplusplus
diff --git a/src/temp/mkostemp.c b/src/temp/mkostemp.c
new file mode 100644
index 0000000..4fd374c
--- /dev/null
+++ b/src/temp/mkostemp.c
@@ -0,0 +1,16 @@
+#define _GNU_SOURCE
+#include <string.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <fcntl.h>
+#include <unistd.h>
+#include <limits.h>
+#include <errno.h>
+#include "libc.h"
+
+int __gen_tempname (char *, int, int);
+
+int mkostemp(char *template, int flags)
+{
+	return __gen_tempname (template, 0, flags);
+}
diff --git a/src/temp/mkostemps.c b/src/temp/mkostemps.c
new file mode 100644
index 0000000..9affae3
--- /dev/null
+++ b/src/temp/mkostemps.c
@@ -0,0 +1,16 @@
+#define _GNU_SOURCE
+#include <string.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <fcntl.h>
+#include <unistd.h>
+#include <limits.h>
+#include <errno.h>
+#include "libc.h"
+
+int __gen_tempname (char *, int, int);
+
+int mkostemps(char *template, int len, int flags)
+{
+	return __gen_tempname (template, len, flags);
+}
diff --git a/src/temp/mkstemp.c b/src/temp/mkstemp.c
index a390d42..08914d4 100644
--- a/src/temp/mkstemp.c
+++ b/src/temp/mkstemp.c
@@ -7,22 +7,11 @@
 #include <errno.h>
 #include "libc.h"
 
-char *__mktemp(char *);
+int __gen_tempname (char *, int, int);
 
 int mkstemp(char *template)
 {
-	int fd, retries = 100, t0 = *template;
-	while (retries--) {
-		if (!*__mktemp(template)) return -1;
-		if ((fd = open(template, O_RDWR | O_CREAT | O_EXCL, 0600))>=0)
-			return fd;
-		if (errno != EEXIST) return -1;
-		/* this is safe because mktemp verified
-		 * that we have a valid template string */
-		template[0] = t0;
-		strcpy(template+strlen(template)-6, "XXXXXX");
-	}
-	return -1;
+	return __gen_tempname (template, 0, O_RDWR);
 }
 
 LFS64(mkstemp);
diff --git a/src/temp/mkstemps.c b/src/temp/mkstemps.c
new file mode 100644
index 0000000..e194444
--- /dev/null
+++ b/src/temp/mkstemps.c
@@ -0,0 +1,16 @@
+#define _GNU_SOURCE
+#include <string.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <fcntl.h>
+#include <unistd.h>
+#include <limits.h>
+#include <errno.h>
+#include "libc.h"
+
+int __gen_tempname (char *, int, int);
+
+int mkstemps(char *template, int len)
+{
+	return __gen_tempname (template, len, O_RDWR);
+}
diff --git a/src/temp/tempname.c b/src/temp/tempname.c
new file mode 100644
index 0000000..1c198bf
--- /dev/null
+++ b/src/temp/tempname.c
@@ -0,0 +1,42 @@
+#include <string.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <fcntl.h>
+#include <unistd.h>
+#include <limits.h>
+#include <errno.h>
+#include "libc.h"
+
+char *__mktemp(char *);
+
+int __gen_tempname (char *template, int len, int flags)
+{
+	if (len < 0) return EINVAL;
+
+	int templen = strlen(template)-len;
+	if (templen<6) return EINVAL;
+
+	char *suffix = (char *)malloc((len+1)*sizeof(char));
+	/* Copy the last len chars plus the null termination */
+	int i;
+	for (i = 0; i <= len; i++)
+		suffix[i] = template[templen+i];
+	/* Null terminate the template before the suffice */
+	template[templen] = '\0';
+
+	int fd, retries = 100, t0 = *template;
+	while (retries--) {
+		if (!*__mktemp(template)) return -1;
+		/* Copy back the suffix */
+		for (i = 0; i <= len; i++)
+			template[templen+i] = suffix[i];
+		if ((fd = open(template, flags | O_CREAT | O_EXCL, 0600))>=0)
+			return fd;
+		if (errno != EEXIST) return -1;
+		/* this is safe because mktemp verified
+		 * that we have a valid template string */
+		template[0] = t0;
+		strcpy(template+templen-6, "XXXXXX");
+	}
+	return -1;
+}
-- 
1.7.12.4



^ permalink raw reply	[flat|nested] 27+ messages in thread

end of thread, other threads:[~2013-02-03  3:30 UTC | newest]

Thread overview: 27+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-01-28  5:06 [PATCH] Add support for mkostemp, mkstemps and mkostemps Anthony G. Basile
2013-01-28  8:47 ` John Spencer
2013-01-28  9:37 ` Szabolcs Nagy
2013-01-28 17:33   ` Szabolcs Nagy
2013-01-29 23:16   ` Anthony G. Basile
2013-01-30  7:21     ` Rich Felker
2013-01-30  7:59       ` Hardy Falk
2013-01-30 13:45         ` Szabolcs Nagy
2013-01-30 16:51           ` Rich Felker
2013-01-30 19:12             ` Szabolcs Nagy
2013-01-30 19:22               ` Rich Felker
2013-01-31  0:28                 ` Szabolcs Nagy
2013-01-28 16:33 ` Rich Felker
2013-02-02  3:48 ` Rich Felker
2013-02-02 18:46   ` Anthony G. Basile
  -- strict thread matches above, loose matches on Subject: below --
2013-02-02 21:15 Anthony G. Basile
2013-02-02 21:17 ` Anthony G. Basile
2013-02-03  3:30 ` Rich Felker
2013-02-02 18:45 Anthony G. Basile
2013-02-02 19:51 ` John Spencer
2013-02-02 19:59   ` John Spencer
2013-02-02 22:11   ` Szabolcs Nagy
2013-02-02 20:14 ` Szabolcs Nagy
2013-02-02 20:38   ` Anthony G. Basile
2013-02-02 22:22     ` Szabolcs Nagy
2013-01-28  2:36 Anthony G. Basile
2013-01-28  5:01 ` Anthony G. Basile

Code repositories for project(s) associated with this public inbox

	https://git.vuxu.org/mirror/musl/

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).