Message ID | 20220707035151.25469-1-selva.nair@gmail.com |
---|---|
State | Accepted |
Headers | show |
Series | [Openvpn-devel] Fix crash in xkey-provider in msvc builds | expand |
Hi, Any thoughts on this? Apart from the broken msvc builds that led to this, looks like the right thing to do, isn't it? Selva On Wed, Jul 6, 2022 at 11:52 PM <selva.nair@gmail.com> wrote: > From: Selva Nair <selva.nair@gmail.com> > > The function signature for xkey_load_generic_key had > function pointers defined as function types that seems > to work in gcc but not in msvc. > > Fix it by changing the function signatures to what was > intended. > Also revert part of commit 627d1a3d28638... as that work- > around should be no longer required. > > Reported by: Lev Stipakov https://github.com/lstipakov > > Signed-off-by: Selva Nair <selva.nair@gmail.com> > --- > src/openvpn/xkey_common.h | 2 +- > src/openvpn/xkey_helper.c | 6 +++--- > 2 files changed, 4 insertions(+), 4 deletions(-) > > diff --git a/src/openvpn/xkey_common.h b/src/openvpn/xkey_common.h > index e0e5ed5b..6d6a1e2c 100644 > --- a/src/openvpn/xkey_common.h > +++ b/src/openvpn/xkey_common.h > @@ -152,7 +152,7 @@ xkey_digest(const unsigned char *src, size_t srclen, > unsigned char *buf, > */ > EVP_PKEY * > xkey_load_generic_key(OSSL_LIB_CTX *libctx, void *handle, EVP_PKEY > *pubkey, > - XKEY_EXTERNAL_SIGN_fn sign_op, XKEY_PRIVKEY_FREE_fn > free_op); > + XKEY_EXTERNAL_SIGN_fn *sign_op, > XKEY_PRIVKEY_FREE_fn *free_op); > > extern OSSL_LIB_CTX *tls_libctx; /* Global */ > > diff --git a/src/openvpn/xkey_helper.c b/src/openvpn/xkey_helper.c > index 14f074ae..73235fd6 100644 > --- a/src/openvpn/xkey_helper.c > +++ b/src/openvpn/xkey_helper.c > @@ -115,7 +115,7 @@ xkey_load_management_key(OSSL_LIB_CTX *libctx, > EVP_PKEY *pubkey) > */ > EVP_PKEY * > xkey_load_generic_key(OSSL_LIB_CTX *libctx, void *handle, EVP_PKEY > *pubkey, > - XKEY_EXTERNAL_SIGN_fn sign_op, XKEY_PRIVKEY_FREE_fn > free_op) > + XKEY_EXTERNAL_SIGN_fn *sign_op, > XKEY_PRIVKEY_FREE_fn *free_op) > { > EVP_PKEY *pkey = NULL; > const char *origin = "external"; > @@ -125,8 +125,8 @@ xkey_load_generic_key(OSSL_LIB_CTX *libctx, void > *handle, EVP_PKEY *pubkey, > {"xkey-origin", OSSL_PARAM_UTF8_STRING, (char *) origin, 0, 0}, > {"pubkey", OSSL_PARAM_OCTET_STRING, &pubkey, sizeof(pubkey), 0}, > {"handle", OSSL_PARAM_OCTET_PTR, &handle, sizeof(handle), 0}, > - {"sign_op", OSSL_PARAM_OCTET_PTR, (void **) &sign_op, sizeof(void > *), 0}, > - {"free_op", OSSL_PARAM_OCTET_PTR, (void **) &free_op, sizeof(void > *), 0}, > + {"sign_op", OSSL_PARAM_OCTET_PTR, (void **) &sign_op, > sizeof(sign_op), 0}, > + {"free_op", OSSL_PARAM_OCTET_PTR, (void **) &free_op, > sizeof(free_op), 0}, > {NULL, 0, NULL, 0, 0} > }; > msg(M_INFO, "%s: handle = %p sign_op = %p free_op = %p ", > __FUNCTION__, > -- > 2.30.2 > > <div dir="ltr"><div>Hi,</div><div><br></div>Any thoughts on this? Apart from the broken msvc builds that led to this, looks like the right thing to do, isn't it?<div><br></div><div>Selva</div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Wed, Jul 6, 2022 at 11:52 PM <<a href="mailto:selva.nair@gmail.com">selva.nair@gmail.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">From: Selva Nair <<a href="mailto:selva.nair@gmail.com" target="_blank">selva.nair@gmail.com</a>><br> <br> The function signature for xkey_load_generic_key had<br> function pointers defined as function types that seems<br> to work in gcc but not in msvc.<br> <br> Fix it by changing the function signatures to what was<br> intended.<br> Also revert part of commit 627d1a3d28638... as that work-<br> around should be no longer required.<br> <br> Reported by: Lev Stipakov <a href="https://github.com/lstipakov" rel="noreferrer" target="_blank">https://github.com/lstipakov</a><br> <br> Signed-off-by: Selva Nair <<a href="mailto:selva.nair@gmail.com" target="_blank">selva.nair@gmail.com</a>><br> ---<br> src/openvpn/xkey_common.h | 2 +-<br> src/openvpn/xkey_helper.c | 6 +++---<br> 2 files changed, 4 insertions(+), 4 deletions(-)<br> <br> diff --git a/src/openvpn/xkey_common.h b/src/openvpn/xkey_common.h<br> index e0e5ed5b..6d6a1e2c 100644<br> --- a/src/openvpn/xkey_common.h<br> +++ b/src/openvpn/xkey_common.h<br> @@ -152,7 +152,7 @@ xkey_digest(const unsigned char *src, size_t srclen, unsigned char *buf,<br> */<br> EVP_PKEY *<br> xkey_load_generic_key(OSSL_LIB_CTX *libctx, void *handle, EVP_PKEY *pubkey,<br> - XKEY_EXTERNAL_SIGN_fn sign_op, XKEY_PRIVKEY_FREE_fn free_op);<br> + XKEY_EXTERNAL_SIGN_fn *sign_op, XKEY_PRIVKEY_FREE_fn *free_op);<br> <br> extern OSSL_LIB_CTX *tls_libctx; /* Global */<br> <br> diff --git a/src/openvpn/xkey_helper.c b/src/openvpn/xkey_helper.c<br> index 14f074ae..73235fd6 100644<br> --- a/src/openvpn/xkey_helper.c<br> +++ b/src/openvpn/xkey_helper.c<br> @@ -115,7 +115,7 @@ xkey_load_management_key(OSSL_LIB_CTX *libctx, EVP_PKEY *pubkey)<br> */<br> EVP_PKEY *<br> xkey_load_generic_key(OSSL_LIB_CTX *libctx, void *handle, EVP_PKEY *pubkey,<br> - XKEY_EXTERNAL_SIGN_fn sign_op, XKEY_PRIVKEY_FREE_fn free_op)<br> + XKEY_EXTERNAL_SIGN_fn *sign_op, XKEY_PRIVKEY_FREE_fn *free_op)<br> {<br> EVP_PKEY *pkey = NULL;<br> const char *origin = "external";<br> @@ -125,8 +125,8 @@ xkey_load_generic_key(OSSL_LIB_CTX *libctx, void *handle, EVP_PKEY *pubkey,<br> {"xkey-origin", OSSL_PARAM_UTF8_STRING, (char *) origin, 0, 0},<br> {"pubkey", OSSL_PARAM_OCTET_STRING, &pubkey, sizeof(pubkey), 0},<br> {"handle", OSSL_PARAM_OCTET_PTR, &handle, sizeof(handle), 0},<br> - {"sign_op", OSSL_PARAM_OCTET_PTR, (void **) &sign_op, sizeof(void *), 0},<br> - {"free_op", OSSL_PARAM_OCTET_PTR, (void **) &free_op, sizeof(void *), 0},<br> + {"sign_op", OSSL_PARAM_OCTET_PTR, (void **) &sign_op, sizeof(sign_op), 0},<br> + {"free_op", OSSL_PARAM_OCTET_PTR, (void **) &free_op, sizeof(free_op), 0},<br> {NULL, 0, NULL, 0, 0}<br> };<br> msg(M_INFO, "%s: handle = %p sign_op = %p free_op = %p ", __FUNCTION__,<br> -- <br> 2.30.2<br> <br> </blockquote></div>
Am 07.07.22 um 05:51 schrieb selva.nair@gmail.com: > From: Selva Nair <selva.nair@gmail.com> > > The function signature for xkey_load_generic_key had > function pointers defined as function types that seems > to work in gcc but not in msvc. > > Fix it by changing the function signatures to what was > intended. > Also revert part of commit 627d1a3d28638... as that work- > around should be no longer required. > > Reported by: Lev Stipakov https://github.com/lstipakov > I am surprised that this commit is not a noop but it is "more" correct this. So without any idea if this actually fixes Windows or not, I approve this commit on the basis of a cosmetic fix that is actually right. Arne
Taking the agreement from Arne as formal ACK (checked on IRC). I'm not sure I grok all the details of this, but I can attest that unit_tests/test_provider still works (on a 3.0.x build), and that calls the "xkey_load_generic_key()" function being modified here. So, this is good. Lev promised to verify MSVC builds early next week. Your patch has been applied to the master branch. commit 2b2e4d0ad5c3a309af8425be8e6e605975fb2a43 Author: Selva Nair Date: Wed Jul 6 23:51:51 2022 -0400 Fix crash in xkey-provider in msvc builds Signed-off-by: Selva Nair <selva.nair@gmail.com> Acked-by: Arne Schwabe <arne@rfc2549.org> Message-Id: <20220707035151.25469-1-selva.nair@gmail.com> URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg24664.html Signed-off-by: Gert Doering <gert@greenie.muc.de> -- kind regards, Gert Doering
diff --git a/src/openvpn/xkey_common.h b/src/openvpn/xkey_common.h index e0e5ed5b..6d6a1e2c 100644 --- a/src/openvpn/xkey_common.h +++ b/src/openvpn/xkey_common.h @@ -152,7 +152,7 @@ xkey_digest(const unsigned char *src, size_t srclen, unsigned char *buf, */ EVP_PKEY * xkey_load_generic_key(OSSL_LIB_CTX *libctx, void *handle, EVP_PKEY *pubkey, - XKEY_EXTERNAL_SIGN_fn sign_op, XKEY_PRIVKEY_FREE_fn free_op); + XKEY_EXTERNAL_SIGN_fn *sign_op, XKEY_PRIVKEY_FREE_fn *free_op); extern OSSL_LIB_CTX *tls_libctx; /* Global */ diff --git a/src/openvpn/xkey_helper.c b/src/openvpn/xkey_helper.c index 14f074ae..73235fd6 100644 --- a/src/openvpn/xkey_helper.c +++ b/src/openvpn/xkey_helper.c @@ -115,7 +115,7 @@ xkey_load_management_key(OSSL_LIB_CTX *libctx, EVP_PKEY *pubkey) */ EVP_PKEY * xkey_load_generic_key(OSSL_LIB_CTX *libctx, void *handle, EVP_PKEY *pubkey, - XKEY_EXTERNAL_SIGN_fn sign_op, XKEY_PRIVKEY_FREE_fn free_op) + XKEY_EXTERNAL_SIGN_fn *sign_op, XKEY_PRIVKEY_FREE_fn *free_op) { EVP_PKEY *pkey = NULL; const char *origin = "external"; @@ -125,8 +125,8 @@ xkey_load_generic_key(OSSL_LIB_CTX *libctx, void *handle, EVP_PKEY *pubkey, {"xkey-origin", OSSL_PARAM_UTF8_STRING, (char *) origin, 0, 0}, {"pubkey", OSSL_PARAM_OCTET_STRING, &pubkey, sizeof(pubkey), 0}, {"handle", OSSL_PARAM_OCTET_PTR, &handle, sizeof(handle), 0}, - {"sign_op", OSSL_PARAM_OCTET_PTR, (void **) &sign_op, sizeof(void *), 0}, - {"free_op", OSSL_PARAM_OCTET_PTR, (void **) &free_op, sizeof(void *), 0}, + {"sign_op", OSSL_PARAM_OCTET_PTR, (void **) &sign_op, sizeof(sign_op), 0}, + {"free_op", OSSL_PARAM_OCTET_PTR, (void **) &free_op, sizeof(free_op), 0}, {NULL, 0, NULL, 0, 0} }; msg(M_INFO, "%s: handle = %p sign_op = %p free_op = %p ", __FUNCTION__,