|
From: syzzer (C. Review) <ge...@op...> - 2025-10-26 14:33:12
|
Attention is currently required from: flichtenheld, plaisthos.
Hello plaisthos, flichtenheld,
I'd like you to do a code review.
Please visit
http://gerrit.openvpn.net/c/openvpn/+/1305?usp=email
to review the following change.
Change subject: ssl_mbedtls: fix missing perf_pop() call
......................................................................
ssl_mbedtls: fix missing perf_pop() call
This was triggered by a bug report submitted by Joshua Rogers, who
used ZeroPath to discover we missed a perf_pop() call in one of the
error paths of ssl_mbedtls.c.
Move an existing perf_pop call a bit upwards to fix that.
The perf code is always disabled by ENABLE_PERFORMANCE_METRICS being
commented out in perf.h. There is no configure flag. None of the active
developers remembers using it and the git log shows no actual code changes
since at least the project structure overhaul of 2012. So this has no
real-world impact.
Change-Id: I5b6881dc73358c8d1249ee2ceb968ede295105b0
Signed-off-by: Steffan Karger <st...@ka...>
---
M src/openvpn/ssl_mbedtls.c
1 file changed, 1 insertion(+), 1 deletion(-)
git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/05/1305/1
diff --git a/src/openvpn/ssl_mbedtls.c b/src/openvpn/ssl_mbedtls.c
index 8fb69c3..2862989 100644
--- a/src/openvpn/ssl_mbedtls.c
+++ b/src/openvpn/ssl_mbedtls.c
@@ -1489,13 +1489,13 @@
/* Error during read, check for retry error */
if (retval < 0)
{
+ perf_pop();
if (MBEDTLS_ERR_SSL_WANT_WRITE == retval || MBEDTLS_ERR_SSL_WANT_READ == retval)
{
return 0;
}
mbed_log_err(D_TLS_ERRORS, retval, "TLS_ERROR: read tls_read_plaintext error");
buf->len = 0;
- perf_pop();
return -1;
}
/* Nothing read, try again */
--
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1305?usp=email
To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings?usp=email
Gerrit-MessageType: newchange
Gerrit-Project: openvpn
Gerrit-Branch: release/2.6
Gerrit-Change-Id: I5b6881dc73358c8d1249ee2ceb968ede295105b0
Gerrit-Change-Number: 1305
Gerrit-PatchSet: 1
Gerrit-Owner: syzzer <st...@ka...>
Gerrit-Reviewer: flichtenheld <fr...@li...>
Gerrit-Reviewer: plaisthos <arn...@rf...>
Gerrit-CC: openvpn-devel <ope...@li...>
Gerrit-Attention: plaisthos <arn...@rf...>
Gerrit-Attention: flichtenheld <fr...@li...>
|
|
From: Gert D. <ge...@gr...> - 2025-10-26 14:35:53
|
From: Steffan Karger <st...@ka...> This was triggered by a bug report submitted by Joshua Rogers, who used ZeroPath to discover we missed a perf_pop() call in one of the error paths of ssl_mbedtls.c. Move an existing perf_pop call a bit upwards to fix that. The perf code is always disabled by ENABLE_PERFORMANCE_METRICS being commented out in perf.h. There is no configure flag. None of the active developers remembers using it and the git log shows no actual code changes since at least the project structure overhaul of 2012. So this has no real-world impact. Change-Id: I5b6881dc73358c8d1249ee2ceb968ede295105b0 Signed-off-by: Steffan Karger <st...@ka...> Acked-by: Gert Doering <ge...@gr...> Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1305 --- This change was reviewed on Gerrit and approved by at least one developer. I request to merge it to release/2.6. Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1305 This mail reflects revision 1 of this Change. Acked-by according to Gerrit (reflected above): Gert Doering <ge...@gr...> diff --git a/src/openvpn/ssl_mbedtls.c b/src/openvpn/ssl_mbedtls.c index 8fb69c3..2862989 100644 --- a/src/openvpn/ssl_mbedtls.c +++ b/src/openvpn/ssl_mbedtls.c @@ -1489,13 +1489,13 @@ /* Error during read, check for retry error */ if (retval < 0) { + perf_pop(); if (MBEDTLS_ERR_SSL_WANT_WRITE == retval || MBEDTLS_ERR_SSL_WANT_READ == retval) { return 0; } mbed_log_err(D_TLS_ERRORS, retval, "TLS_ERROR: read tls_read_plaintext error"); buf->len = 0; - perf_pop(); return -1; } /* Nothing read, try again */ |
|
From: Gert D. <ge...@gr...> - 2025-10-27 10:02:06
|
While this is sort-of a NOP in 2.6, at least it's a consistent NOP now :-)
Your patch has been applied to the release/2.6 branch.
commit e83c63f58135913bb2ca2f4345da8cd77098474b
Author: Steffan Karger
Date: Sun Oct 26 15:35:14 2025 +0100
ssl_mbedtls: fix missing perf_pop() call
Signed-off-by: Steffan Karger <st...@ka...>
Acked-by: Gert Doering <ge...@gr...>
Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1305
Message-Id: <202...@gr...>
URL: https://www.mail-archive.com/ope...@li.../msg33870.html
Signed-off-by: Gert Doering <ge...@gr...>
--
kind regards,
Gert Doering
|
|
From: cron2 (C. Review) <ge...@op...> - 2025-10-26 14:35:26
|
Attention is currently required from: flichtenheld, plaisthos, syzzer. cron2 has posted comments on this change by syzzer. ( http://gerrit.openvpn.net/c/openvpn/+/1305?usp=email ) Change subject: ssl_mbedtls: fix missing perf_pop() call ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.openvpn.net/c/openvpn/+/1305?usp=email To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: openvpn Gerrit-Branch: release/2.6 Gerrit-Change-Id: I5b6881dc73358c8d1249ee2ceb968ede295105b0 Gerrit-Change-Number: 1305 Gerrit-PatchSet: 1 Gerrit-Owner: syzzer <st...@ka...> Gerrit-Reviewer: cron2 <ge...@gr...> Gerrit-Reviewer: flichtenheld <fr...@li...> Gerrit-Reviewer: plaisthos <arn...@rf...> Gerrit-CC: openvpn-devel <ope...@li...> Gerrit-Attention: syzzer <st...@ka...> Gerrit-Attention: plaisthos <arn...@rf...> Gerrit-Attention: flichtenheld <fr...@li...> Gerrit-Comment-Date: Sun, 26 Oct 2025 14:35:06 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes |
|
From: cron2 (C. Review) <ge...@op...> - 2025-10-27 10:02:58
|
cron2 has uploaded a new patch set (#2) to the change originally created by syzzer. ( http://gerrit.openvpn.net/c/openvpn/+/1305?usp=email ) The following approvals got outdated and were removed: Code-Review+2 by cron2 Change subject: ssl_mbedtls: fix missing perf_pop() call ...................................................................... ssl_mbedtls: fix missing perf_pop() call This was triggered by a bug report submitted by Joshua Rogers, who used ZeroPath to discover we missed a perf_pop() call in one of the error paths of ssl_mbedtls.c. Move an existing perf_pop call a bit upwards to fix that. The perf code is always disabled by ENABLE_PERFORMANCE_METRICS being commented out in perf.h. There is no configure flag. None of the active developers remembers using it and the git log shows no actual code changes since at least the project structure overhaul of 2012. So this has no real-world impact. Change-Id: I5b6881dc73358c8d1249ee2ceb968ede295105b0 Signed-off-by: Steffan Karger <st...@ka...> Acked-by: Gert Doering <ge...@gr...> Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1305 Message-Id: <202...@gr...> URL: https://www.mail-archive.com/ope...@li.../msg33870.html Signed-off-by: Gert Doering <ge...@gr...> --- M src/openvpn/ssl_mbedtls.c 1 file changed, 1 insertion(+), 1 deletion(-) git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/05/1305/2 diff --git a/src/openvpn/ssl_mbedtls.c b/src/openvpn/ssl_mbedtls.c index 8fb69c3..2862989 100644 --- a/src/openvpn/ssl_mbedtls.c +++ b/src/openvpn/ssl_mbedtls.c @@ -1489,13 +1489,13 @@ /* Error during read, check for retry error */ if (retval < 0) { + perf_pop(); if (MBEDTLS_ERR_SSL_WANT_WRITE == retval || MBEDTLS_ERR_SSL_WANT_READ == retval) { return 0; } mbed_log_err(D_TLS_ERRORS, retval, "TLS_ERROR: read tls_read_plaintext error"); buf->len = 0; - perf_pop(); return -1; } /* Nothing read, try again */ -- To view, visit http://gerrit.openvpn.net/c/openvpn/+/1305?usp=email To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings?usp=email Gerrit-MessageType: newpatchset Gerrit-Project: openvpn Gerrit-Branch: release/2.6 Gerrit-Change-Id: I5b6881dc73358c8d1249ee2ceb968ede295105b0 Gerrit-Change-Number: 1305 Gerrit-PatchSet: 2 Gerrit-Owner: syzzer <st...@ka...> Gerrit-Reviewer: cron2 <ge...@gr...> Gerrit-Reviewer: flichtenheld <fr...@li...> Gerrit-Reviewer: plaisthos <arn...@rf...> Gerrit-CC: openvpn-devel <ope...@li...> |
|
From: cron2 (C. Review) <ge...@op...> - 2025-10-27 10:02:59
|
cron2 has submitted this change. ( http://gerrit.openvpn.net/c/openvpn/+/1305?usp=email ) Change subject: ssl_mbedtls: fix missing perf_pop() call ...................................................................... ssl_mbedtls: fix missing perf_pop() call This was triggered by a bug report submitted by Joshua Rogers, who used ZeroPath to discover we missed a perf_pop() call in one of the error paths of ssl_mbedtls.c. Move an existing perf_pop call a bit upwards to fix that. The perf code is always disabled by ENABLE_PERFORMANCE_METRICS being commented out in perf.h. There is no configure flag. None of the active developers remembers using it and the git log shows no actual code changes since at least the project structure overhaul of 2012. So this has no real-world impact. Change-Id: I5b6881dc73358c8d1249ee2ceb968ede295105b0 Signed-off-by: Steffan Karger <st...@ka...> Acked-by: Gert Doering <ge...@gr...> Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1305 Message-Id: <202...@gr...> URL: https://www.mail-archive.com/ope...@li.../msg33870.html Signed-off-by: Gert Doering <ge...@gr...> --- M src/openvpn/ssl_mbedtls.c 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/openvpn/ssl_mbedtls.c b/src/openvpn/ssl_mbedtls.c index 8fb69c3..2862989 100644 --- a/src/openvpn/ssl_mbedtls.c +++ b/src/openvpn/ssl_mbedtls.c @@ -1489,13 +1489,13 @@ /* Error during read, check for retry error */ if (retval < 0) { + perf_pop(); if (MBEDTLS_ERR_SSL_WANT_WRITE == retval || MBEDTLS_ERR_SSL_WANT_READ == retval) { return 0; } mbed_log_err(D_TLS_ERRORS, retval, "TLS_ERROR: read tls_read_plaintext error"); buf->len = 0; - perf_pop(); return -1; } /* Nothing read, try again */ -- To view, visit http://gerrit.openvpn.net/c/openvpn/+/1305?usp=email To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings?usp=email Gerrit-MessageType: merged Gerrit-Project: openvpn Gerrit-Branch: release/2.6 Gerrit-Change-Id: I5b6881dc73358c8d1249ee2ceb968ede295105b0 Gerrit-Change-Number: 1305 Gerrit-PatchSet: 2 Gerrit-Owner: syzzer <st...@ka...> Gerrit-Reviewer: cron2 <ge...@gr...> Gerrit-Reviewer: flichtenheld <fr...@li...> Gerrit-Reviewer: plaisthos <arn...@rf...> Gerrit-CC: openvpn-devel <ope...@li...> |