[scd] Memory leak fix.

[g13] Send MOUNTPOINT status line

THANKS

@@ -145,6 +145,7 @@ Keith Clayton              keith at claytons.org
 Ken Takusagawa             ken.takusagawa.2  at gmail.com
 Kevin Ryde                 user42 at zip.com.au
 Kiss Gabor                 kissg at ssg.ki.iif.hu
+Klaus Flittner             klaus at flittner org
 Klaus Singvogel            ks at caldera.de
 Kurt Garloff               garloff at suse.de
 Lars Kellogg-Stedman	   lars at bu.edu

common/ChangeLog

@@ -1,8 +1,12 @@
+2009-10-28  Werner Koch  <wk@g10code.com>
+
+	* status.h (STATUS_MOUNTPOINT): New.
+
 2009-10-16  Marcus Brinkmann  <marcus@g10code.com>
 
 	* Makefile.am (libcommon_a_CFLAGS): Use LIBASSUAN_CFLAGS instead
 	of LIBASSUAN_PTH_CFLAGS.
-	
+
 2009-10-13  Werner Koch  <wk@g10code.com>
 
 	* exechelp.c (gnupg_kill_process): New.

common/status.h

@@ -124,6 +124,8 @@ enum
     STATUS_PKA_TRUST_GOOD,
 
     STATUS_TRUNCATED,
+    STATUS_MOUNTPOINT,
+
     STATUS_ERROR
 };
 

doc/DETAILS

@@ -680,6 +680,12 @@ more arguments in future versions.
         A backup key named FNAME has been created for the key with
         KEYID.
 
+    MOUNTPOINT <name>
+        NAME is a percent-plus escaped filename describing the
+        mountpoint for the current operation (e.g. g13 --mount).  This
+        may either be the specified mountpoint or one randomly choosen
+        by g13.
+
 
 Format of the "--attribute-fd" output
 =====================================

g13/be-encfs.c

@@ -418,7 +418,7 @@ be_encfs_create_container (ctrl_t ctrl, const char *fname, tupledesc_t tuples,
     {
       err = gpg_error_from_syserror ();
       log_error (_("can't create directory `%s': %s\n"),
-                 "/tmp/g13-XXXXXX", gpg_strerror (err));
+                 "/tmp/.#g13_XXXXXX", gpg_strerror (err));
       goto leave;
     }
 

g13/g13.c

@@ -420,6 +420,8 @@ main ( int argc, char **argv)
   /* Setup a default control structure for command line mode.  */
   memset (&ctrl, 0, sizeof ctrl);
   g13_init_default_ctrl (&ctrl);
+  ctrl.no_server = 1;
+  ctrl.status_fd = -1; /* No status output. */
 
   /* Set the default option file */
   if (default_config )
@@ -678,6 +680,7 @@ main ( int argc, char **argv)
     case aServer:
       {
         start_idle_task ();
+        ctrl.no_server = 0;
         err = g13_server (&ctrl);
         if (err)
           log_error ("server exited with error: %s <%s>\n",

g13/g13.h

@@ -106,6 +106,8 @@ struct server_control_s
 void g13_exit (int rc);
 void g13_init_default_ctrl (struct server_control_s *ctrl);
 
+/*-- server.c (commonly used, thus declared here) --*/
+gpg_error_t g13_status (ctrl_t ctrl, int no, ...) GNUPG_GCC_A_SENTINEL(0);
 
 
 #endif /*G13_H*/

g13/mount.c

@@ -250,15 +250,35 @@ g13_mount_container (ctrl_t ctrl, const char *filename, const char *mountpoint)
   const unsigned char *value;
   int conttype;
   unsigned int rid;
+  char *mountpoint_buffer = NULL;
 
   /* A quick check to see whether the container exists.  */
   if (access (filename, R_OK))
     return gpg_error_from_syserror ();
 
+  if (!mountpoint)
+    {
+      mountpoint_buffer = xtrystrdup ("/tmp/g13-XXXXXX");
+      if (!mountpoint_buffer)
+        return gpg_error_from_syserror ();
+      if (!mkdtemp (mountpoint_buffer))
+        {
+          err = gpg_error_from_syserror ();
+          log_error (_("can't create directory `%s': %s\n"),
+                     "/tmp/g13-XXXXXX", gpg_strerror (err));
+          xfree (mountpoint_buffer);
+          return err;
+        }
+      mountpoint = mountpoint_buffer;
+    }
+
   /* Try to take a lock.  */
   lock = create_dotlock (filename);
   if (!lock)
-    return gpg_error_from_syserror ();
+    {
+      xfree (mountpoint_buffer);
+      return gpg_error_from_syserror ();
+    }
 
   if (make_dotlock (lock, 0))
     {
@@ -318,9 +338,21 @@ g13_mount_container (ctrl_t ctrl, const char *filename, const char *mountpoint)
   err = be_mount_container (ctrl, conttype, filename, mountpoint, tuples, &rid);
   if (!err)
     {
-      err = mountinfo_add_mount (filename, mountpoint, conttype, rid);
+      err = mountinfo_add_mount (filename, mountpoint, conttype, rid,
+                                 !!mountpoint_buffer);
       /* Fixme: What shall we do if this fails?  Add a provisional
          mountinfo entry first and remove it on error? */
+      if (!err)
+        {
+          char *tmp = percent_plus_escape (mountpoint);
+          if (!tmp)
+            err = gpg_error_from_syserror ();
+          else
+            {
+              g13_status (ctrl, STATUS_MOUNTPOINT, tmp, NULL);
+              xfree (tmp);
+            }
+        }
     }
 
  leave:
@@ -328,6 +360,7 @@ g13_mount_container (ctrl_t ctrl, const char *filename, const char *mountpoint)
   xfree (keyblob);
   xfree (enckeyblob);
   destroy_dotlock (lock);
+  xfree (mountpoint_buffer);
   return err;
 }
 

g13/mountinfo.c

@@ -43,6 +43,10 @@ struct mounttable_s
   char *mountpoint;  /* Name of the mounttype.  */
   int conttype;      /* Type of the container.  */
   unsigned int rid;  /* Identifier of the runner task.  */
+  struct {
+    unsigned int remove:1;  /* True if the mountpoint shall be removed
+                               on umount.  */
+  } flags;
 };
 
 
@@ -55,7 +59,7 @@ size_t mounttable_size;
 /* Add CONTAINER,MOUNTPOINT,CONTTYPE,RID to the mounttable.  */
 gpg_error_t
 mountinfo_add_mount (const char *container, const char *mountpoint,
-                     int conttype, unsigned int rid)
+                     int conttype, unsigned int rid, int remove_flag)
 {
   size_t idx;
   mtab_t m;
@@ -96,6 +100,7 @@ mountinfo_add_mount (const char *container, const char *mountpoint,
     }
   m->conttype = conttype;
   m->rid = rid;
+  m->flags.remove = !!remove_flag;
   m->in_use = 1;
 
   return 0;
@@ -125,6 +130,16 @@ mountinfo_del_mount (const char *container, const char *mountpoint,
   for (idx=0, m = mounttable; idx < mounttable_size; idx++, m++)
     if (m->in_use && m->rid == rid)
       {
+        if (m->flags.remove && m->mountpoint)
+          {
+            /* FIXME: This does not always work because the umount may
+               not have completed yet.  We should add the mountpoints
+               to an idle queue and retry a remove.  */ 
+            if (rmdir (m->mountpoint))
+              log_error ("error removing mount point `%s': %s\n",
+                         m->mountpoint, 
+                         gpg_strerror (gpg_error_from_syserror ()));
+          }
         m->in_use = 0;
         xfree (m->container);
         m->container = NULL;
@@ -177,7 +192,8 @@ mountinfo_dump_all (void)
 
   for (idx=0, m = mounttable; idx < mounttable_size; idx++, m++)
     if (m->in_use)
-      log_info ("mtab[%d] %s on %s type %d rid %u\n", 
+      log_info ("mtab[%d] %s on %s type %d rid %u%s\n", 
-                idx, m->container, m->mountpoint, m->conttype, m->rid);
+                idx, m->container, m->mountpoint, m->conttype, m->rid,
+                m->flags.remove?" [remove]":"");
 }
 

g13/mountinfo.h

@@ -25,7 +25,8 @@ typedef struct mounttable_s *mtab_t;
 
 gpg_error_t mountinfo_add_mount (const char *container,
                                  const char *mountpoint,
-                                 int conttype, unsigned int rid);
+                                 int conttype, unsigned int rid,
+                                 int remove_flag);
 gpg_error_t mountinfo_del_mount (const char *container,
                                  const char *mountpoint,
                                  unsigned int rid);

g13/server.c

@@ -33,6 +33,10 @@
 #include "mount.h"
 #include "create.h"
 
+
+/* The filepointer for status message used in non-server mode */
+static FILE *statusfp;
+
 /* Local data for this server module.  A pointer to this is stored in
    the CTRL object of each connection.  */
 struct server_local_s 
@@ -310,7 +314,8 @@ cmd_mount (assuan_context_t ctx, char *line)
 /* UMOUNT [options] [<mountpoint>]
 
    Unmount the currently open file or the one opened at MOUNTPOINT.
-   MOUNTPOINT must be percent-plus escaped.
+   MOUNTPOINT must be percent-plus escaped.  On success the mountpoint
+   is returned via a "MOUNTPOINT" status line.
  */
 static gpg_error_t
 cmd_umount (assuan_context_t ctx, char *line)
@@ -662,99 +667,81 @@ g13_server (ctrl_t ctrl)
 }
 
 
+/* Send a status line with status ID NO.  The arguments are a list of
+   strings terminated by a NULL argument.  */
+gpg_error_t
+g13_status (ctrl_t ctrl, int no, ...)
+{
+  gpg_error_t err = 0;
+  va_list arg_ptr;
+  const char *text;
 
-/* gpg_error_t */
+  va_start (arg_ptr, no);
-/* gpgsm_status2 (ctrl_t ctrl, int no, ...) */
-/* { */
-/*   gpg_error_t err = 0; */
-/*   va_list arg_ptr; */
-/*   const char *text; */
 
-/*   va_start (arg_ptr, no); */
+  if (ctrl->no_server && ctrl->status_fd == -1)
-
+    ; /* No status wanted. */
-/*   if (ctrl->no_server && ctrl->status_fd == -1) */
+  else if (ctrl->no_server)
-/*     ; /\* No status wanted. *\/ */
+    {
-/*   else if (ctrl->no_server) */
+      if (!statusfp)
-/*     { */
+        {
-/*       if (!statusfp) */
+          if (ctrl->status_fd == 1)
-/*         { */
+            statusfp = stdout;
-/*           if (ctrl->status_fd == 1) */
+          else if (ctrl->status_fd == 2)
-/*             statusfp = stdout; */
+            statusfp = stderr;
-/*           else if (ctrl->status_fd == 2) */
+          else
-/*             statusfp = stderr; */
+            statusfp = fdopen (ctrl->status_fd, "w");
-/*           else */
+          
-/*             statusfp = fdopen (ctrl->status_fd, "w"); */
+          if (!statusfp)
+            {
+              log_fatal ("can't open fd %d for status output: %s\n",
+                         ctrl->status_fd, strerror(errno));
+            }
+        }
       
-/*           if (!statusfp) */
+      fputs ("[GNUPG:] ", statusfp);
-/*             { */
+      fputs (get_status_string (no), statusfp);
-/*               log_fatal ("can't open fd %d for status output: %s\n", */
-/*                          ctrl->status_fd, strerror(errno)); */
-/*             } */
-/*         } */
-      
-/*       fputs ("[GNUPG:] ", statusfp); */
-/*       fputs (get_status_string (no), statusfp); */
     
-/*       while ( (text = va_arg (arg_ptr, const char*) )) */
+      while ( (text = va_arg (arg_ptr, const char*) ))
-/*         { */
+        {
-/*           putc ( ' ', statusfp ); */
+          putc ( ' ', statusfp );
-/*           for (; *text; text++)  */
+          for (; *text; text++)
-/*             { */
+            {
-/*               if (*text == '\n') */
+              if (*text == '\n')
-/*                 fputs ( "\\n", statusfp ); */
+                fputs ( "\\n", statusfp );
-/*               else if (*text == '\r') */
+              else if (*text == '\r')
-/*                 fputs ( "\\r", statusfp ); */
+                fputs ( "\\r", statusfp );
-/*               else  */
+              else
-/*                 putc ( *(const byte *)text,  statusfp ); */
+                putc ( *(const byte *)text,  statusfp );
-/*             } */
+            }
-/*         } */
+        }
-/*       putc ('\n', statusfp); */
+      putc ('\n', statusfp);
-/*       fflush (statusfp); */
+      fflush (statusfp);
-/*     } */
+    }
-/*   else  */
+  else
-/*     { */
+    {
-/*       assuan_context_t ctx = ctrl->server_local->assuan_ctx; */
+      assuan_context_t ctx = ctrl->server_local->assuan_ctx;
-/*       char buf[950], *p; */
+      char buf[950], *p;
-/*       size_t n; */
+      size_t n;
 
-/*       p = buf;  */
+      p = buf;
-/*       n = 0; */
+      n = 0;
-/*       while ( (text = va_arg (arg_ptr, const char *)) ) */
+      while ( (text = va_arg (arg_ptr, const char *)) )
-/*         { */
+        {
-/*           if (n) */
+          if (n)
-/*             { */
+            {
-/*               *p++ = ' '; */
+              *p++ = ' ';
-/*               n++; */
+              n++;
-/*             } */
+            }
-/*           for ( ; *text && n < DIM (buf)-2; n++) */
+          for ( ; *text && n < DIM (buf)-2; n++)
-/*             *p++ = *text++; */
+            *p++ = *text++;
-/*         } */
+        }
-/*       *p = 0; */
+      *p = 0;
-/*       err = assuan_write_status (ctx, get_status_string (no), buf); */
+      err = assuan_write_status (ctx, get_status_string (no), buf);
-/*     } */
+    }
 
-/*   va_end (arg_ptr); */
+  va_end (arg_ptr);
-/*   return err; */
+  return err;
-/* } */
+}
-
-/* gpg_error_t */
-/* gpgsm_status (ctrl_t ctrl, int no, const char *text) */
-/* { */
-/*   return gpgsm_status2 (ctrl, no, text, NULL); */
-/* } */
-
-/* gpg_error_t */
-/* gpgsm_status_with_err_code (ctrl_t ctrl, int no, const char *text, */
-/*                             gpg_err_code_t ec) */
-/* { */
-/*   char buf[30]; */
-
-/*   sprintf (buf, "%u", (unsigned int)ec); */
-/*   if (text) */
-/*     return gpgsm_status2 (ctrl, no, text, buf, NULL); */
-/*   else */
-/*     return gpgsm_status2 (ctrl, no, buf, NULL); */
-/* } */
 
 
 /* Helper to notify the client about Pinentry events.  Returns an gpg
@@ -768,5 +755,3 @@ g13_proxy_pinentry_notify (ctrl_t ctrl, const unsigned char *line)
 }
 
 
-
-

scd/ChangeLog

@@ -1,3 +1,9 @@
+2009-10-25  Werner Koch  <wk@g10code.com>
+
+	* scdaemon.c (scd_deinit_default_ctrl): Release IN_DATA.
+	* command.c (cmd_setdata): Release IN_DATA.  Reported by Klaus
+	Flittner.
+
 2009-10-16  Marcus Brinkmann  <marcus@g10code.com>
 
 	* AM_CFLAGS, scdaemon_LDADD: Use libassuan instead of libassuan-pth.

scd/command.c

@@ -804,6 +804,7 @@ cmd_setdata (assuan_context_t ctx, char *line)
   if (!buf)
     return out_of_core ();
 
+  xfree (ctrl->in_data.value);
   ctrl->in_data.value = buf;
   ctrl->in_data.valuelen = n;
   for (p=line, n=0; n < ctrl->in_data.valuelen; p += 2, n++)

scd/scdaemon.c

@@ -895,7 +895,11 @@ scd_init_default_ctrl (ctrl_t ctrl)
 static void
 scd_deinit_default_ctrl (ctrl_t ctrl)
 {
-  (void)ctrl;
+  if (!ctrl)
+    return;
+  xfree (ctrl->in_data.value);
+  ctrl->in_data.value = NULL;
+  ctrl->in_data.valuelen = 0;
 }