ssh: Fix allocation of pinentry buffer

NIIBE Yutaka gniibe at fsij.org
Tue Sep 29 04:08:32 CEST 2015


On 09/28/2015 07:41 PM, Werner Koch wrote:
> On Mon, 28 Sep 2015 10:54, gniibe at fsij.org said:
> 
>> I changed the pointer calculation using the cast to (char *), but it
>> is wrong still, because it doesn't care about alignment of the
>> structure.  It should be rounded up to the alignment.
> 
> Yep.  We better do separate mallocs for PI and PI2.  Overoptimizing
> things is never good.

I did consider your suggestion of: two allocations and two free-s, as
well as changing the structure itself (to have the field:
pin[MAX_PASSPHRASE_LEN + 1]).

I realized that sizeof operator cares alignment.  Given the situation
MAX_PASSPHRASE_LEN==255, following patch is minimum change.

I think that this is good for the fix for minimum impact.  I added a
comment at MAX_PASSPHRASE_LEN.


diff --git a/agent/agent.h b/agent/agent.h
index b3e8470..9e066c9 100644
--- a/agent/agent.h
+++ b/agent/agent.h
@@ -50,7 +50,8 @@
 /* The maximum length of a passphrase (in bytes).  Note: this is
    further contrained by the Assuan line length (and any other text on
    the same line).  However, the Assuan line length is 1k bytes so
-   this shouldn't be a problem in practice.  */
+   this shouldn't be a problem in practice.
+   (MAX_PASSPHRASE_LEN + 1) should be multiple of alignment size.  */
 #define MAX_PASSPHRASE_LEN 255


diff --git a/agent/command-ssh.c b/agent/command-ssh.c
index 8be1255..3f63f76 100644
--- a/agent/command-ssh.c
+++ b/agent/command-ssh.c
@@ -3107,7 +3107,8 @@ ssh_identity_register (ctrl_t ctrl, ssh_key_type_spec_t *spec,
       err = gpg_error_from_syserror ();
       goto out;
     }
-  pi2 = pi + (sizeof *pi + MAX_PASSPHRASE_LEN + 1);
+  pi2 = (struct pin_entry_info_s *)((char *)pi + (sizeof (*pi)
+                                                  + MAX_PASSPHRASE_LEN + 1));
   pi->max_length = MAX_PASSPHRASE_LEN + 1;
   pi->max_tries = 1;
   pi->with_repeat = 1;
diff --git a/agent/genkey.c b/agent/genkey.c
index 13858ca..54b7149 100644
--- a/agent/genkey.c
+++ b/agent/genkey.c
@@ -375,7 +375,8 @@ agent_ask_new_passphrase (ctrl_t ctrl, const char *prompt,
     }

   pi = gcry_calloc_secure (2, sizeof (*pi) + MAX_PASSPHRASE_LEN + 1);
-  pi2 = pi + (sizeof *pi + MAX_PASSPHRASE_LEN + 1);
+  pi2 = (struct pin_entry_info_s *)((char *)pi + (sizeof (*pi)
+                                                  + MAX_PASSPHRASE_LEN + 1));
   pi->max_length = MAX_PASSPHRASE_LEN + 1;
   pi->max_tries = 3;
   pi->with_qualitybar = 1;
-- 



More information about the Gnupg-devel mailing list