Re: Use of `mktemp' is dangerous, better use `mkstemp'...

[Ángel]

On 2021-01-15 at 10:00 -0500, Chet Ramey wrote:
> On 1/14/21 11:45 PM, Jeffrey Walton wrote:
> > Hi Everyone,
> > 
> > I'm building Bash 5.1 from sources. This may be of interest:
> > 
> > /usr/bin/ld: ./lib/sh/libsh.a(tmpfile.o): in function
> > `sh_mktmpname':
> > /home/jwalton/Build-Scripts/bash-5.1/lib/sh/tmpfile.c:160: warning:
> > the use of `mktemp' is dangerous, better use `mkstemp' or `mkdtemp'
> 
> It's a spurious warning.

mktemp() would only end up being used (with all its issues) if the OS
doesn't provide a DEV FD or there's no mkdtemp(). In other (safe)
Operating Systems, it would be possible to avoid the warning by
building a slightly different libsh.a in such case.

I have made a quick patch (see attachment) and it does work.
My system would normally report that warning. My compiler is smart
enough to ellide mktemp with this change. bash tests pass after the
change, both with dev-fs and disabling it (with only run-procsub
failing, but that's expected).

Best regards



diff --git a/externs.h b/externs.h
index 3d7716c6..f25cc281 100644
--- a/externs.h
+++ b/externs.h
@@ -482,7 +482,7 @@ extern void print_timeval ();
 #define MT_USERANDOM		0x0004
 #define MT_TEMPLATE		0x0008
 
-extern char *sh_mktmpname PARAMS((char *, int));
+extern char *sh_mktmpfifo PARAMS((char *, int));
 extern int sh_mktmpfd PARAMS((char *, int, char **));
 /* extern FILE *sh_mktmpfp PARAMS((char *, int, char **)); */
 extern char *sh_mktmpdir PARAMS((char *, int));
diff --git a/lib/sh/tmpfile.c b/lib/sh/tmpfile.c
index ef8b067b..e179d8ef 100644
--- a/lib/sh/tmpfile.c
+++ b/lib/sh/tmpfile.c
@@ -131,7 +131,7 @@ sh_seedrand ()
 #endif
 }
 
-char *
+static char *
 sh_mktmpname (nameroot, flags)
      char *nameroot;
      int flags;
@@ -309,3 +309,22 @@ sh_mktmpdir (nameroot, flags)
   return (filename);
 #endif /* !USE_MKDTEMP */
 }
+
+#ifndef HAVE_DEV_FD
+char *
+sh_mktmpfifo (nameroot, flags)
+     char *nameroot;
+     int flags;
+{
+  char *tname;
+
+  tname = sh_mktmpname (nameroot, flags);
+  if (mkfifo (tname, 0600) < 0)
+    {
+      free (tname);
+      return ((char *)NULL);
+    }
+
+  return (tname);
+}
+#endif
diff --git a/subst.c b/subst.c
index 6132316a..47008eb3 100644
--- a/subst.c
+++ b/subst.c
@@ -5609,14 +5609,12 @@ make_named_pipe ()
 {
   char *tname;
 
-  tname = sh_mktmpname ("sh-np", MT_USERANDOM|MT_USETMPDIR);
-  if (mkfifo (tname, 0600) < 0)
+  tname = sh_mktmpfifo ("sh-np", MT_USERANDOM|MT_USETMPDIR);
+  if (tname)
     {
-      free (tname);
-      return ((char *)NULL);
+      add_fifo_list (tname);
     }
 
-  add_fifo_list (tname);
   return (tname);
 }