From a1b614285518c1e4928919b905e992f35f4a3224 Mon Sep 17 00:00:00 2001 From: Werner Koch Date: Wed, 28 Oct 2009 12:02:15 +0000 Subject: [PATCH] [scd] Memory leak fix. [g13] Send MOUNTPOINT status line --- THANKS | 1 + common/ChangeLog | 6 +- common/status.h | 2 + doc/DETAILS | 6 ++ g13/be-encfs.c | 2 +- g13/g13.c | 3 + g13/g13.h | 2 + g13/mount.c | 37 ++++++++++- g13/mountinfo.c | 22 ++++++- g13/mountinfo.h | 3 +- g13/server.c | 165 +++++++++++++++++++++-------------------------- scd/ChangeLog | 6 ++ scd/command.c | 1 + scd/scdaemon.c | 6 +- 14 files changed, 163 insertions(+), 99 deletions(-) diff --git a/THANKS b/THANKS index 8e4c9129a..c86fda3bb 100644 --- a/THANKS +++ b/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 diff --git a/common/ChangeLog b/common/ChangeLog index c6c43ec3f..1b09fd916 100644 --- a/common/ChangeLog +++ b/common/ChangeLog @@ -1,8 +1,12 @@ +2009-10-28 Werner Koch + + * status.h (STATUS_MOUNTPOINT): New. + 2009-10-16 Marcus Brinkmann * Makefile.am (libcommon_a_CFLAGS): Use LIBASSUAN_CFLAGS instead of LIBASSUAN_PTH_CFLAGS. - + 2009-10-13 Werner Koch * exechelp.c (gnupg_kill_process): New. diff --git a/common/status.h b/common/status.h index a11f2a38c..bb5429dc0 100644 --- a/common/status.h +++ b/common/status.h @@ -124,6 +124,8 @@ enum STATUS_PKA_TRUST_GOOD, STATUS_TRUNCATED, + STATUS_MOUNTPOINT, + STATUS_ERROR }; diff --git a/doc/DETAILS b/doc/DETAILS index f4be2b95e..ba809d1e3 100644 --- a/doc/DETAILS +++ b/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 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 ===================================== diff --git a/g13/be-encfs.c b/g13/be-encfs.c index 250084a48..524f09e6b 100644 --- a/g13/be-encfs.c +++ b/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; } diff --git a/g13/g13.c b/g13/g13.c index 754371df2..152058e28 100644 --- a/g13/g13.c +++ b/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", diff --git a/g13/g13.h b/g13/g13.h index 3c52b50f4..90b6d5013 100644 --- a/g13/g13.h +++ b/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*/ diff --git a/g13/mount.c b/g13/mount.c index dc23b6b2b..2ab5cc636 100644 --- a/g13/mount.c +++ b/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; } diff --git a/g13/mountinfo.c b/g13/mountinfo.c index 100c1e475..07c6240d4 100644 --- a/g13/mountinfo.c +++ b/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", - idx, m->container, m->mountpoint, m->conttype, m->rid); + log_info ("mtab[%d] %s on %s type %d rid %u%s\n", + idx, m->container, m->mountpoint, m->conttype, m->rid, + m->flags.remove?" [remove]":""); } diff --git a/g13/mountinfo.h b/g13/mountinfo.h index 187dff0db..b8ac5bd7b 100644 --- a/g13/mountinfo.h +++ b/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); diff --git a/g13/server.c b/g13/server.c index e9b9a0a33..a6906aaa3 100644 --- a/g13/server.c +++ b/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] [] 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 */ -/* gpgsm_status2 (ctrl_t ctrl, int no, ...) */ -/* { */ -/* gpg_error_t err = 0; */ -/* va_list arg_ptr; */ -/* const char *text; */ + va_start (arg_ptr, no); -/* va_start (arg_ptr, no); */ - -/* if (ctrl->no_server && ctrl->status_fd == -1) */ -/* ; /\* No status wanted. *\/ */ -/* else if (ctrl->no_server) */ -/* { */ -/* if (!statusfp) */ -/* { */ -/* if (ctrl->status_fd == 1) */ -/* statusfp = stdout; */ -/* else if (ctrl->status_fd == 2) */ -/* statusfp = stderr; */ -/* else */ -/* statusfp = fdopen (ctrl->status_fd, "w"); */ + if (ctrl->no_server && ctrl->status_fd == -1) + ; /* No status wanted. */ + else if (ctrl->no_server) + { + if (!statusfp) + { + if (ctrl->status_fd == 1) + statusfp = stdout; + else if (ctrl->status_fd == 2) + statusfp = stderr; + 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) */ -/* { */ -/* 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); */ + fputs ("[GNUPG:] ", statusfp); + fputs (get_status_string (no), statusfp); -/* while ( (text = va_arg (arg_ptr, const char*) )) */ -/* { */ -/* putc ( ' ', statusfp ); */ -/* for (; *text; text++) */ -/* { */ -/* if (*text == '\n') */ -/* fputs ( "\\n", statusfp ); */ -/* else if (*text == '\r') */ -/* fputs ( "\\r", statusfp ); */ -/* else */ -/* putc ( *(const byte *)text, statusfp ); */ -/* } */ -/* } */ -/* putc ('\n', statusfp); */ -/* fflush (statusfp); */ -/* } */ -/* else */ -/* { */ -/* assuan_context_t ctx = ctrl->server_local->assuan_ctx; */ -/* char buf[950], *p; */ -/* size_t n; */ + while ( (text = va_arg (arg_ptr, const char*) )) + { + putc ( ' ', statusfp ); + for (; *text; text++) + { + if (*text == '\n') + fputs ( "\\n", statusfp ); + else if (*text == '\r') + fputs ( "\\r", statusfp ); + else + putc ( *(const byte *)text, statusfp ); + } + } + putc ('\n', statusfp); + fflush (statusfp); + } + else + { + assuan_context_t ctx = ctrl->server_local->assuan_ctx; + char buf[950], *p; + size_t n; -/* p = buf; */ -/* n = 0; */ -/* while ( (text = va_arg (arg_ptr, const char *)) ) */ -/* { */ -/* if (n) */ -/* { */ -/* *p++ = ' '; */ -/* n++; */ -/* } */ -/* for ( ; *text && n < DIM (buf)-2; n++) */ -/* *p++ = *text++; */ -/* } */ -/* *p = 0; */ -/* err = assuan_write_status (ctx, get_status_string (no), buf); */ -/* } */ + p = buf; + n = 0; + while ( (text = va_arg (arg_ptr, const char *)) ) + { + if (n) + { + *p++ = ' '; + n++; + } + for ( ; *text && n < DIM (buf)-2; n++) + *p++ = *text++; + } + *p = 0; + err = assuan_write_status (ctx, get_status_string (no), buf); + } -/* va_end (arg_ptr); */ -/* 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); */ -/* } */ + va_end (arg_ptr); + return err; +} /* 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) } - - diff --git a/scd/ChangeLog b/scd/ChangeLog index d1ab1c643..9a545d87f 100644 --- a/scd/ChangeLog +++ b/scd/ChangeLog @@ -1,3 +1,9 @@ +2009-10-25 Werner Koch + + * 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 * AM_CFLAGS, scdaemon_LDADD: Use libassuan instead of libassuan-pth. diff --git a/scd/command.c b/scd/command.c index 09c0b8e45..b70455eec 100644 --- a/scd/command.c +++ b/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++) diff --git a/scd/scdaemon.c b/scd/scdaemon.c index f483d2098..5823c9948 100644 --- a/scd/scdaemon.c +++ b/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; }