GtkEntry memory vtable (to use non-pageable memory for passwords)

[Stef <stef-list memberwebs com>]

A while back Matthias asked on the gnome-keyring mailing list why we
were shipping our own forked GtkEntry. [1]

I explained that one of the goals of gnome-keyring is to try and keep
passwords and encryption keys out of the swap file by storing them in
non-pageable memory. The forked GtkEntry initially came from a gpg-agent
implementation written by Albrecht Dress, then I brought that code into
seahorse and finally into gnome-keyring.

Here's a patch that tries to extend GtkEntry to be able to use another
memory allocator. Adds a property called 'memory-vtable' which is a
GMemVTable structure.

An initial rough patch is attached. It tries to guarantee that text in
the GtkEntry will only live in memory allocated by the configured memory
allocator. However I've run into some problems implementing this,
because of static buffers, UTF8 normalization. These are marked by 'XXX'
in the patch.

Perhaps the above guarantee is too tough? It would be easier to make
memory buffer guarantees only for the !entry->visible code paths. But
this could be confusing from a documentation and API perspective, where
the 'memory-vtable' property doesn't actually do what it implies.

This leads me to wonder if perhaps the password entry control in GTK+
might fare better as a separate widget. There's an insane amount of "if
(entry->visible)" in the code and alternate code paths for password entry.

Looking for advice...

Cheers,

Stef

[1]
http://mail.gnome.org/archives/gnome-keyring-list/2009-January/msg00002.html

diff --git a/gtk/gtkentry.c b/gtk/gtkentry.c
index 04ae129..10098a4 100644
--- a/gtk/gtkentry.c
+++ b/gtk/gtkentry.c
@@ -142,6 +142,8 @@ struct _GtkEntryPrivate
   gint start_y;
 
   gchar *im_module;
+
+  GMemVTable memory_vtable;
 };
 
 typedef struct _GtkEntryPasswordHint GtkEntryPasswordHint;
@@ -220,7 +222,8 @@ enum {
   PROP_TOOLTIP_TEXT_SECONDARY,
   PROP_TOOLTIP_MARKUP_PRIMARY,
   PROP_TOOLTIP_MARKUP_SECONDARY,
-  PROP_IM_MODULE
+  PROP_IM_MODULE,
+  PROP_MEMORY_VTABLE
 };
 
 static guint signals[LAST_SIGNAL] = { 0 };
@@ -242,6 +245,9 @@ static void   gtk_entry_get_property         (GObject          *object,
                                               guint             prop_id,
                                               GValue           *value,
                                               GParamSpec       *pspec);
+static GObject*  gtk_entry_constructor       (GType                 type,
+                                              guint                 n_props,
+                                              GObjectConstructParam *props);
 static void   gtk_entry_finalize             (GObject          *object);
 static void   gtk_entry_destroy              (GtkObject        *object);
 static void   gtk_entry_dispose              (GObject          *object);
@@ -541,6 +547,7 @@ gtk_entry_class_init (GtkEntryClass *class)
   widget_class = (GtkWidgetClass*) class;
   gtk_object_class = (GtkObjectClass *)class;
 
+  gobject_class->constructor = gtk_entry_constructor;
   gobject_class->dispose = gtk_entry_dispose;
   gobject_class->finalize = gtk_entry_finalize;
   gobject_class->set_property = gtk_entry_set_property;
@@ -1185,6 +1192,21 @@ gtk_entry_class_init (GtkEntryClass *class)
                                                         GTK_PARAM_READWRITE));
 
   /**
+   * GtkEntry:memory-vtable:
+   * 
+   * An optional GMemVTable for allocating memory used by the text 
+   * in the entry.  
+   *
+   * Since: 2.XX
+   */
+  g_object_class_install_property (gobject_class,
+                                   PROP_MEMORY_VTABLE,
+                                   g_param_spec_pointer ("memory-vtable",
+                                                         P_("Memory VTable"),
+                                                         P_("GMemVTable for allocating entry text memory"),
+                                                         GTK_PARAM_READWRITE | G_PARAM_CONSTRUCT_ONLY));
+
+  /**
    * GtkEntry:icon-prelight:
    *
    * The prelight style property determines whether activatable
@@ -1915,6 +1937,11 @@ gtk_entry_set_property (GObject         *object,
         gtk_im_multicontext_set_context_id (GTK_IM_MULTICONTEXT (entry->im_context), priv->im_module);
       break;
 
+    case PROP_MEMORY_VTABLE:
+      g_return_if_fail (g_value_get_pointer (value));
+      memcpy (&priv->memory_vtable, g_value_get_pointer (value), sizeof (GMemVTable));
+      break;
+
     case PROP_SCROLL_OFFSET:
     case PROP_CURSOR_POSITION:
     default:
@@ -2122,6 +2149,10 @@ gtk_entry_get_property (GObject         *object,
                            gtk_entry_get_icon_tooltip_markup (entry, GTK_ENTRY_ICON_SECONDARY));
       break;
 
+    case PROP_MEMORY_VTABLE:
+      g_value_set_pointer (value, &priv->memory_vtable);
+      break;
+
     default:
       G_OBJECT_WARN_INVALID_PROPERTY_ID (object, prop_id, pspec);
       break;
@@ -2177,9 +2208,9 @@ gtk_entry_init (GtkEntry *entry)
   
   GTK_WIDGET_SET_FLAGS (entry, GTK_CAN_FOCUS);
 
-  entry->text_size = MIN_SIZE;
-  entry->text = g_malloc (entry->text_size);
-  entry->text[0] = '\0';
+  /* Null until gtk_entry_constructor */
+  entry->text_size = 0;
+  entry->text = NULL;
 
   entry->editable = TRUE;
   entry->visible = TRUE;
@@ -2197,6 +2228,10 @@ gtk_entry_init (GtkEntry *entry)
   priv->progress_fraction = 0.0;
   priv->progress_pulse_fraction = 0.1;
 
+  priv->memory_vtable.malloc = g_malloc;
+  priv->memory_vtable.realloc = g_realloc;
+  priv->memory_vtable.free = g_free;
+
   gtk_drag_dest_set (GTK_WIDGET (entry),
                      GTK_DEST_DEFAULT_HIGHLIGHT,
                      NULL, 0,
@@ -2218,6 +2253,19 @@ gtk_entry_init (GtkEntry *entry)
 		    G_CALLBACK (gtk_entry_delete_surrounding_cb), entry);
 }
 
+static GObject*
+gtk_entry_constructor (GType type, guint n_props, GObjectConstructParam *props)
+{
+  GtkEntry *entry = GTK_ENTRY (G_OBJECT_CLASS (gtk_entry_parent_class)->constructor (type, n_props, props));
+  GtkEntryPrivate *priv = GTK_ENTRY_GET_PRIVATE (entry);
+  
+  entry->text_size = MIN_SIZE;
+  entry->text = (priv->memory_vtable.malloc) (entry->text_size);
+  entry->text[0] = '\0';
+
+  return G_OBJECT (entry);
+}
+
 static gint
 get_icon_width (GtkEntry             *entry,
                 GtkEntryIconPosition  icon_pos)
@@ -2413,7 +2461,7 @@ gtk_entry_finalize (GObject *object)
     {
       if (!entry->visible)
 	trash_area (entry->text, strlen (entry->text));
-      g_free (entry->text);
+      (priv->memory_vtable.free) (entry->text);
       entry->text = NULL;
     }
 
@@ -3734,7 +3782,7 @@ gtk_entry_motion_notify (GtkWidget      *widget,
           
           if (pixmap)
             g_object_unref (pixmap);
-          g_free (text);
+          (priv->memory_vtable.free) (text);
 
           entry->in_drag = FALSE;
           entry->button = 0;
@@ -4039,6 +4087,7 @@ gtk_entry_insert_text (GtkEditable *editable,
 		       gint        *position)
 {
   GtkEntry *entry = GTK_ENTRY (editable);
+  GtkEntryPrivate *priv = GTK_ENTRY_GET_PRIVATE (entry);
   gchar buf[64];
   gchar *text;
 
@@ -4046,11 +4095,12 @@ gtk_entry_insert_text (GtkEditable *editable,
     *position = entry->text_length;
   
   g_object_ref (editable);
-  
+
+  /* XXX: Text sits in a normal buffer here, doesn't use our memory vtable */  
   if (new_text_length <= 63)
     text = buf;
   else
-    text = g_new (gchar, new_text_length + 1);
+    text = (priv->memory_vtable.malloc) (new_text_length + 1);
 
   text[new_text_length] = '\0';
   strncpy (text, new_text, new_text_length);
@@ -4061,7 +4111,7 @@ gtk_entry_insert_text (GtkEditable *editable,
     trash_area (text, new_text_length);
 
   if (new_text_length > 63)
-    g_free (text);
+    (priv->memory_vtable.free) (text);
 
   g_object_unref (editable);
 }
@@ -4093,6 +4143,8 @@ gtk_entry_get_chars      (GtkEditable   *editable,
 			  gint           end_pos)
 {
   GtkEntry *entry = GTK_ENTRY (editable);
+  GtkEntryPrivate *priv = GTK_ENTRY_GET_PRIVATE (entry);
+  gchar *result;
   gint start_index, end_index;
   
   if (end_pos < 0)
@@ -4104,7 +4156,10 @@ gtk_entry_get_chars      (GtkEditable   *editable,
   start_index = g_utf8_offset_to_pointer (entry->text, start_pos) - entry->text;
   end_index = g_utf8_offset_to_pointer (entry->text, end_pos) - entry->text;
 
-  return g_strndup (entry->text + start_index, end_index - start_index);
+  result = (priv->memory_vtable.malloc) (end_index - start_index + 1);
+  memcpy (result, entry->text + start_index, end_index - start_index);
+  result[end_index - start_index] = 0;
+  return result;
 }
 
 static void
@@ -4302,6 +4357,7 @@ gtk_entry_real_insert_text (GtkEditable *editable,
 			    gint        *position)
 {
   GtkEntry *entry = GTK_ENTRY (editable);
+  GtkEntryPrivate *priv = GTK_ENTRY_GET_PRIVATE (entry);
   gint index;
   gint n_chars;
 
@@ -4344,14 +4400,14 @@ gtk_entry_real_insert_text (GtkEditable *editable,
 	}
 
       if (entry->visible)
-	entry->text = g_realloc (entry->text, entry->text_size);
+	entry->text = (priv->memory_vtable.realloc) (entry->text, entry->text_size);
       else
 	{
 	  /* Same thing, just slower and without leaving stuff in memory.  */
-	  gchar *et_new = g_malloc (entry->text_size);
+	  gchar *et_new = (priv->memory_vtable.malloc) (entry->text_size);
 	  memcpy (et_new, entry->text, MIN (prev_size, entry->text_size));
 	  trash_area (entry->text, prev_size);
-	  g_free (entry->text);
+	  (priv->memory_vtable.free) (entry->text);
 	  entry->text = et_new;
 	}
     }
@@ -4712,6 +4768,7 @@ static void
 gtk_entry_backspace (GtkEntry *entry)
 {
   GtkEditable *editable = GTK_EDITABLE (entry);
+  GtkEntryPrivate *priv = GTK_ENTRY_GET_PRIVATE (entry);
   gint prev_pos;
 
   _gtk_entry_reset_im_context (entry);
@@ -4748,6 +4805,8 @@ gtk_entry_backspace (GtkEntry *entry)
 	  cluster_text = gtk_editable_get_chars (editable,
 			  			 prev_pos,
 						 entry->current_pos);
+
+	  /* XXX: This duplicates text without using our memory_vtable */
 	  normalized_text = g_utf8_normalize (cluster_text,
 			  		      strlen (cluster_text),
 					      G_NORMALIZE_NFD);
@@ -4765,7 +4824,7 @@ gtk_entry_backspace (GtkEntry *entry)
 	    }
 
 	  g_free (normalized_text);
-	  g_free (cluster_text);
+	  (priv->memory_vtable.free) (cluster_text);
 	}
       else
 	{
@@ -4786,6 +4845,7 @@ static void
 gtk_entry_copy_clipboard (GtkEntry *entry)
 {
   GtkEditable *editable = GTK_EDITABLE (entry);
+  GtkEntryPrivate *priv = GTK_ENTRY_GET_PRIVATE (entry);
   gint start, end;
   gchar *str;
 
@@ -4801,7 +4861,7 @@ gtk_entry_copy_clipboard (GtkEntry *entry)
       gtk_clipboard_set_text (gtk_widget_get_clipboard (GTK_WIDGET (entry),
 							GDK_SELECTION_CLIPBOARD),
 			      str, -1);
-      g_free (str);
+      (priv->memory_vtable.free) (str);
     }
 }
 
@@ -5148,6 +5208,7 @@ gtk_entry_create_layout (GtkEntry *entry,
 
   if (preedit_length)
     {
+      /* XXX: Text buffer doesn't use our vtable */
       GString *tmp_string = g_string_new (NULL);
       
       gint cursor_index = g_utf8_offset_to_pointer (entry->text, entry->current_pos) - entry->text;
@@ -5223,6 +5284,7 @@ gtk_entry_create_layout (GtkEntry *entry,
         }
       else
         {
+          /* XXX: Text buffer doesn't use our memory vtable */
           GString *str = g_string_new (NULL);
           gunichar invisible_char;
           guint password_hint_timeout;
@@ -6025,6 +6087,7 @@ gtk_entry_get_public_chars (GtkEntry *entry,
     return g_strdup ("");
   else
     {
+      /* XXX: Text buffer doesn't use our memory vtable */
       GString *str = g_string_new (NULL);
       append_char (str, entry->invisible_char, end - start);
       return g_string_free (str, FALSE);
@@ -6120,13 +6183,14 @@ primary_get_cb (GtkClipboard     *clipboard,
 		gpointer          data)
 {
   GtkEntry *entry = GTK_ENTRY (data);
+  GtkEntryPrivate *priv = GTK_ENTRY_GET_PRIVATE (entry);
   gint start, end;
   
   if (gtk_editable_get_selection_bounds (GTK_EDITABLE (entry), &start, &end))
     {
       gchar *str = gtk_entry_get_public_chars (entry, start, end);
       gtk_selection_data_set_text (selection_data, str, -1);
-      g_free (str);
+      (priv->memory_vtable.free) (str);
     }
 }