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