Re: [OpenWrt-Devel] [PATCH 2/3] Fix SSL negotiation being interrupted by .notify_write from BIO method.
On 2014-12-12 05:16, Yousong Zhou wrote:
On 12 December 2014 at 00:42, Felix Fietkau n...@openwrt.org wrote:
On 2014-11-11 11:34, Yousong Zhou wrote:
ustream_ssl_check_conn() may be called by .notify_write while a previous
SSL_connect() is still in process. This can happen because the
.notify_write callback will may be triggered by writes in the BIO
methods.
Signed-off-by: Yousong Zhou yszhou4t...@gmail.com
---
ustream-ssl.c | 19 +++
ustream-ssl.h |1 +
2 files changed, 16 insertions(+), 4 deletions(-)
diff --git a/ustream-ssl.c b/ustream-ssl.c
index dd0faf9..84104b0 100644
--- a/ustream-ssl.c
+++ b/ustream-ssl.c
@@ -34,12 +34,22 @@ static void ustream_ssl_error_cb(struct uloop_timeout
*t)
us-notify_error(us, error, __ustream_ssl_strerror(us-error,
buffer, sizeof(buffer)));
}
+static enum ssl_conn_status ustream_ssl_do_connect(struct ustream_ssl *us)
+{
+ enum ssl_conn_status status;
+
+ us-connecting = true;
+ status = __ustream_ssl_connect(us);
+ us-connecting = false;
+ return status;
+}
+
I think this can be fixed in a much simpler way. Simply prevent
re-entrant calls to __ustream_ssl_connect through a static variable.
Guarding it with a single static variable do not work well with
multiple instances of ustream_ssl.
How so? Even with multiple instances a call stack from one instance that
calls do_connect should not end up with re-entrant calls for a different
instance.
- Felix
___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/cgi-bin/mailman/listinfo/openwrt-devel
Re: [OpenWrt-Devel] [PATCH 2/3] Fix SSL negotiation being interrupted by .notify_write from BIO method.
On 12 December 2014 at 19:36, Felix Fietkau n...@openwrt.org wrote:
On 2014-12-12 05:16, Yousong Zhou wrote:
On 12 December 2014 at 00:42, Felix Fietkau n...@openwrt.org wrote:
On 2014-11-11 11:34, Yousong Zhou wrote:
ustream_ssl_check_conn() may be called by .notify_write while a previous
SSL_connect() is still in process. This can happen because the
.notify_write callback will may be triggered by writes in the BIO
methods.
Signed-off-by: Yousong Zhou yszhou4t...@gmail.com
---
ustream-ssl.c | 19 +++
ustream-ssl.h |1 +
2 files changed, 16 insertions(+), 4 deletions(-)
diff --git a/ustream-ssl.c b/ustream-ssl.c
index dd0faf9..84104b0 100644
--- a/ustream-ssl.c
+++ b/ustream-ssl.c
@@ -34,12 +34,22 @@ static void ustream_ssl_error_cb(struct uloop_timeout
*t)
us-notify_error(us, error,
__ustream_ssl_strerror(us-error, buffer, sizeof(buffer)));
}
+static enum ssl_conn_status ustream_ssl_do_connect(struct ustream_ssl *us)
+{
+ enum ssl_conn_status status;
+
+ us-connecting = true;
+ status = __ustream_ssl_connect(us);
+ us-connecting = false;
+ return status;
+}
+
I think this can be fixed in a much simpler way. Simply prevent
re-entrant calls to __ustream_ssl_connect through a static variable.
Guarding it with a single static variable do not work well with
multiple instances of ustream_ssl.
How so? Even with multiple instances a call stack from one instance that
calls do_connect should not end up with re-entrant calls for a different
instance.
But we do not want do_connect() of different instances intervening
with each other, do we?
yousong
___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/cgi-bin/mailman/listinfo/openwrt-devel
Re: [OpenWrt-Devel] [PATCH 2/3] Fix SSL negotiation being interrupted by .notify_write from BIO method.
On 2014-11-11 11:34, Yousong Zhou wrote:
ustream_ssl_check_conn() may be called by .notify_write while a previous
SSL_connect() is still in process. This can happen because the
.notify_write callback will may be triggered by writes in the BIO
methods.
Signed-off-by: Yousong Zhou yszhou4t...@gmail.com
---
ustream-ssl.c | 19 +++
ustream-ssl.h |1 +
2 files changed, 16 insertions(+), 4 deletions(-)
diff --git a/ustream-ssl.c b/ustream-ssl.c
index dd0faf9..84104b0 100644
--- a/ustream-ssl.c
+++ b/ustream-ssl.c
@@ -34,12 +34,22 @@ static void ustream_ssl_error_cb(struct uloop_timeout *t)
us-notify_error(us, error, __ustream_ssl_strerror(us-error,
buffer, sizeof(buffer)));
}
+static enum ssl_conn_status ustream_ssl_do_connect(struct ustream_ssl *us)
+{
+ enum ssl_conn_status status;
+
+ us-connecting = true;
+ status = __ustream_ssl_connect(us);
+ us-connecting = false;
+ return status;
+}
+
I think this can be fixed in a much simpler way. Simply prevent
re-entrant calls to __ustream_ssl_connect through a static variable. The
other checks for us-connecting should be unnecessary, I think
!us-connected is enough.
- Felix
___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/cgi-bin/mailman/listinfo/openwrt-devel
Re: [OpenWrt-Devel] [PATCH 2/3] Fix SSL negotiation being interrupted by .notify_write from BIO method.
On 12 December 2014 at 00:42, Felix Fietkau n...@openwrt.org wrote:
On 2014-11-11 11:34, Yousong Zhou wrote:
ustream_ssl_check_conn() may be called by .notify_write while a previous
SSL_connect() is still in process. This can happen because the
.notify_write callback will may be triggered by writes in the BIO
methods.
Signed-off-by: Yousong Zhou yszhou4t...@gmail.com
---
ustream-ssl.c | 19 +++
ustream-ssl.h |1 +
2 files changed, 16 insertions(+), 4 deletions(-)
diff --git a/ustream-ssl.c b/ustream-ssl.c
index dd0faf9..84104b0 100644
--- a/ustream-ssl.c
+++ b/ustream-ssl.c
@@ -34,12 +34,22 @@ static void ustream_ssl_error_cb(struct uloop_timeout *t)
us-notify_error(us, error, __ustream_ssl_strerror(us-error,
buffer, sizeof(buffer)));
}
+static enum ssl_conn_status ustream_ssl_do_connect(struct ustream_ssl *us)
+{
+ enum ssl_conn_status status;
+
+ us-connecting = true;
+ status = __ustream_ssl_connect(us);
+ us-connecting = false;
+ return status;
+}
+
I think this can be fixed in a much simpler way. Simply prevent
re-entrant calls to __ustream_ssl_connect through a static variable.
Guarding it with a single static variable do not work well with
multiple instances of ustream_ssl.
The
other checks for us-connecting should be unnecessary, I think
!us-connected is enough.
Yes.
yousong
- Felix
___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/cgi-bin/mailman/listinfo/openwrt-devel
