From f85e2fb9c252cd70e7630366e1886ba367c739ae Mon Sep 17 00:00:00 2001
From: Ralph Little <skelband@gmail.com>
Date: Sun, 1 May 2022 22:02:55 +0000
Subject: [PATCH] Resolve "Fix `mkstemp` error handling"

---
 backend/avision.c             | 47 +++++++++++++++++------------------
 backend/avision.h             |  1 -
 backend/bh.c                  |  5 ++++
 backend/canon-sane.c          | 29 +++------------------
 backend/canon630u-common.c    | 11 ++++++--
 backend/canon_lide70-common.c |  2 +-
 backend/dc25.c                | 30 ++++++++--------------
 7 files changed, 52 insertions(+), 73 deletions(-)

diff --git a/backend/avision.c b/backend/avision.c
index 399663d4f..86653e97f 100644
--- a/backend/avision.c
+++ b/backend/avision.c
@@ -7488,11 +7488,27 @@ reader_process (void *data)
     return SANE_STATUS_NO_MEM;
 
   if (dev->adf_offset_compensation) {
+    char duplex_offtmp_fname [] = "/tmp/avision-offtmp-XXXXXX";
+
+    int fd = mkstemp(duplex_offtmp_fname);
+    if (fd == -1) {
+      DBG (1, "reader_process: failed to generate temporary fname for ADF offset compensation temp file\n");
+      return SANE_STATUS_NO_MEM;
+    }
+    DBG (1, "reader_process: temporary fname for ADF offset compensation temp file: %s\n",
+         duplex_offtmp_fname);
+
+    if (unlink(duplex_offtmp_fname) == -1) {
+      DBG(1, "reader_process: failed to delete temporary file prior to use: %s\n", duplex_offtmp_fname);
+      // continue though.
+    }
+
     DBG (3, "reader_process: redirecting output data to temp file for ADF offset compensation.\n");
     fp_fd = fp;
-    fp = fopen (s->duplex_offtmp_fname, "w+");
+    fp = fdopen (fd, "w+");
     if (!fp) {
       fclose(fp_fd);
+      close(fd);
       return SANE_STATUS_NO_MEM;
     }
   }
@@ -8653,33 +8669,21 @@ sane_open (SANE_String_Const devicename, SANE_Handle *handle)
        dev->hw->offset.duplex.rear.bottom != 0) )
     dev->adf_offset_compensation = SANE_TRUE;
 
-  if (dev->adf_offset_compensation) {
-    strncpy(s->duplex_offtmp_fname, "/tmp/avision-offtmp-XXXXXX", PATH_MAX);
-
-    if (! mktemp(s->duplex_offtmp_fname) ) {
-      DBG (1, "sane_open: failed to generate temporary fname for ADF offset compensation temp file\n");
-      return SANE_STATUS_NO_MEM;
-    }
-    else {
-      DBG (1, "sane_open: temporary fname for ADF offset compensation temp file: %s\n",
-	   s->duplex_offtmp_fname);
-    }
-  }
-
   if (dev->inquiry_duplex_interlaced || dev->scanner_type == AV_FILM ||
       dev->hw->feature_type & AV_ADF_FLIPPING_DUPLEX) {
     /* Might need at least *DOS (Windows flavour and OS/2) portability fix
        However, I was told Cygwin (et al.) takes care of it. */
+
+    /* Create the file but close the fd. It is not used to open the file later. :( */
     strncpy(s->duplex_rear_fname, "/tmp/avision-rear-XXXXXX", PATH_MAX);
 
-    if (! mkstemp(s->duplex_rear_fname) ) {
+    int fd = mkstemp(s->duplex_rear_fname);
+    if (fd == -1) {
       DBG (1, "sane_open: failed to generate temporary fname for duplex scans\n");
       return SANE_STATUS_NO_MEM;
     }
-    else {
-      DBG (1, "sane_open: temporary fname for duplex scans: %s\n",
-	   s->duplex_rear_fname);
-    }
+    DBG (1, "sane_open: temporary fname for duplex scans: %s\n", s->duplex_rear_fname);
+    close(fd);
   }
 
   /* calibrate film scanners, as this must be done without the
@@ -8775,11 +8779,6 @@ sane_close (SANE_Handle handle)
     *(s->duplex_rear_fname) = 0;
   }
 
-  if (*(s->duplex_offtmp_fname)) {
-    unlink (s->duplex_offtmp_fname);
-    *(s->duplex_offtmp_fname) = 0;
-  }
-
   free (handle);
 }
 
diff --git a/backend/avision.h b/backend/avision.h
index aed6d9ee9..3f8b6b598 100644
--- a/backend/avision.h
+++ b/backend/avision.h
@@ -502,7 +502,6 @@ typedef struct Avision_Scanner
 
   /* Internal data for duplex scans */
   char duplex_rear_fname [PATH_MAX];
-  char duplex_offtmp_fname [PATH_MAX];
   SANE_Bool duplex_rear_valid;
 
   color_mode c_mode;
diff --git a/backend/bh.c b/backend/bh.c
index 5a6308db7..31ebe93c3 100644
--- a/backend/bh.c
+++ b/backend/bh.c
@@ -1895,6 +1895,11 @@ start_scan (BH_Scanner *s)
 		    {
 		      DBG(1, "sane_start: error opening barfile `%s'\n",
 			  s->barfname);
+		      if (fd !=-1)
+		        {
+		          close(fd);
+		          unlink(s->barfname);
+		        }
 		      status = SANE_STATUS_IO_ERROR;
 		    }
 		}
diff --git a/backend/canon-sane.c b/backend/canon-sane.c
index 6957aa730..e31345765 100644
--- a/backend/canon-sane.c
+++ b/backend/canon-sane.c
@@ -1111,9 +1111,6 @@ sane_start (SANE_Handle handle)
   u_char cbuf[2];			/* modification for FB620S */
   size_t buf_size, i;
 
-  char tmpfilename[] = "/tmp/canon.XXXXXX"; /* for FB1200S */
-  char *thistmpfile; /* for FB1200S */
-
   DBG (1, ">> sane_start\n");
 
   s->tmpfile = -1; /* for FB1200S */
@@ -1121,36 +1118,18 @@ sane_start (SANE_Handle handle)
 /******* making a tempfile for 1200 dpi scanning of FB1200S ******/
   if (s->hw->info.model == FB1200)
     {
-      thistmpfile = strdup(tmpfilename);
-
-      if (thistmpfile != NULL)
-        {
-          if (!mkstemp(thistmpfile))
-            {
-              DBG(1, "mkstemp(thistmpfile) is failed\n");
-              return (SANE_STATUS_INVAL);
-	    }
-	}
-      else
-        {
-	  DBG(1, "strdup(thistmpfile) is failed\n");
-	  return (SANE_STATUS_INVAL);
-	}
-
-      s->tmpfile = open(thistmpfile, O_RDWR | O_CREAT | O_EXCL, 0600);
+      char tmpfilename[] = "/tmp/canon.XXXXXX"; /* for FB1200S */
 
+      s->tmpfile = mkstemp(tmpfilename);
       if (s->tmpfile == -1)
 	{
-	  DBG(1, "error opening temp file %s\n", thistmpfile);
+	  DBG(1, "error opening temp file %s\n", tmpfilename);
 	  DBG(1, "errno: %i; %s\n", errno, strerror(errno));
 	  errno = 0;
 	  return (SANE_STATUS_INVAL);
 	}
       DBG(1, " ****** tmpfile is opened ****** \n");
-
-      unlink(thistmpfile);
-      free (thistmpfile);
-      DBG(1, "free thistmpfile\n");
+      unlink(tmpfilename);
     }
 /******************************************************************/
 
diff --git a/backend/canon630u-common.c b/backend/canon630u-common.c
index ef264a989..deaef66cc 100644
--- a/backend/canon630u-common.c
+++ b/backend/canon630u-common.c
@@ -937,7 +937,10 @@ plugin_cal (CANON_Handle * s)
     {
       DBG (1, "No temp filename!\n");
       s->fname = strdup ("/tmp/cal.XXXXXX");
-      mkstemp (s->fname);
+
+      /* FIXME: we should be using fd, not discarding it, and also checking for error! */
+      int fd = mkstemp (s->fname);
+      close(fd);
     }
   s->width = 2551;
   s->height = 75;
@@ -1581,8 +1584,12 @@ CANON_start_scan (CANON_Handle * scanner)
 
   /* choose a temp file name for scan data */
   scanner->fname = strdup ("/tmp/scan.XXXXXX");
-  if (!mkstemp (scanner->fname))
+
+  /* FIXME: we should be using fd, not discarding it! */
+  int fd = mkstemp (scanner->fname);
+  if (fd == -1)
     return SANE_STATUS_IO_ERROR;
+  close(fd);
 
   /* calibrate if needed */
   rv = init (scanner->fd);
diff --git a/backend/canon_lide70-common.c b/backend/canon_lide70-common.c
index 0882fec0a..4e58222d5 100644
--- a/backend/canon_lide70-common.c
+++ b/backend/canon_lide70-common.c
@@ -3249,7 +3249,7 @@ CANON_start_scan (CANON_Handle * chndl)
   chndl->fname = strdup ("/tmp/scan.XXXXXX");
   fd = mkstemp (chndl->fname);
 
-  if (!fd)
+  if (fd == -1)
     {
       return SANE_STATUS_IO_ERROR;
     }
diff --git a/backend/dc25.c b/backend/dc25.c
index 95f131f35..73eb53dae 100644
--- a/backend/dc25.c
+++ b/backend/dc25.c
@@ -134,8 +134,7 @@ static char tty_name[PATH_MAX];
 #define DEF_TTY_NAME "/dev/ttyS0"
 
 static speed_t tty_baud = DEFAULT_TTY_BAUD;
-static char *tmpname;
-static char tmpnamebuf[] = "/tmp/dc25XXXXXX";
+#define TMPFILE_PATTERN "/tmp/dc25XXXXXX";
 
 static Dc20Info *dc20_info;
 static Dc20Info CameraInfo;
@@ -2007,16 +2006,6 @@ sane_open (SANE_String_Const devicename, SANE_Handle * handle)
       DBG (1, "No device info\n");
     }
 
-  if (tmpname == NULL)
-    {
-      tmpname = tmpnamebuf;
-      if (!mkstemp (tmpname))
-	{
-	  DBG (1, "Unable to make temp file %s\n", tmpname);
-	  return SANE_STATUS_INVAL;
-	}
-    }
-
   DBG (3, "sane_open: pictures taken=%d\n", dc20_info->pic_taken);
 
   return SANE_STATUS_GOOD;
@@ -2430,14 +2419,15 @@ sane_start (SANE_Handle handle)
        * port overruns on a 90MHz pentium until I used hdparm
        * to set the "-u1" flag on the system drives.
        */
-      int fd;
+      char tmpnamebuf[] = TMPFILE_PATTERN;
 
-      fd = open (tmpname, O_CREAT | O_EXCL | O_WRONLY, 0600);
+      int fd = mkstemp (tmpnamebuf);
       if (fd == -1)
-	{
-	  DBG (0, "Unable to open tmp file\n");
-	  return SANE_STATUS_INVAL;
-	}
+        {
+          DBG (0, "Unable to make temp file %s\n", tmpnamebuf);
+          return SANE_STATUS_INVAL;
+        }
+
       f = fdopen (fd, "wb");
       if (f == NULL)
 	{
@@ -2509,12 +2499,12 @@ sane_start (SANE_Handle handle)
       else
 	{
 	  fclose (f);
-	  if (convert_pic (tmpname, SAVE_ADJASPECT | SAVE_24BITS) == -1)
+	  if (convert_pic (tmpnamebuf, SAVE_ADJASPECT | SAVE_24BITS) == -1)
 	    {
 	      DBG (3, "sane_open: unable to convert\n");
 	      return SANE_STATUS_INVAL;
 	    }
-	  unlink (tmpname);
+	  unlink (tmpnamebuf);
 	  outbytes = 0;
 	}
     }
-- 
GitLab