| 
      
      
      From: cron2 (C. Review) <ge...@op...> - 2025-10-28 18:36:37
       | 
| 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/+/1323?usp=email
to review the following change.
Change subject: zeroize struct image in packet_id_persist_save() before writing to disk
......................................................................
zeroize struct image in packet_id_persist_save() before writing to disk
while this really is only a debug function, ensuring that no uninitialized
heap content ends up in padding in the structure and thus to disk is good
practice.
Reported-by: Joshua Rogers <co...@jo...>
Found-by: ZeroPath (https://zeropath.com/)
Change-Id: I7f4c7b0ca748975defca1e5104e7077a761cd49c
Signed-off-by: Gert Doering <ge...@gr...>
---
M src/openvpn/packet_id.c
1 file changed, 1 insertion(+), 0 deletions(-)
  git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/23/1323/1
diff --git a/src/openvpn/packet_id.c b/src/openvpn/packet_id.c
index 880eee1..08d9d9b 100644
--- a/src/openvpn/packet_id.c
+++ b/src/openvpn/packet_id.c
@@ -511,6 +511,7 @@
         && (p->time != p->time_last_written || p->id != p->id_last_written))
     {
         struct packet_id_persist_file_image image;
+        CLEAR(image);
         ssize_t n;
         off_t seek_ret;
         struct gc_arena gc = gc_new();
-- 
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1323?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: master
Gerrit-Change-Id: I7f4c7b0ca748975defca1e5104e7077a761cd49c
Gerrit-Change-Number: 1323
Gerrit-PatchSet: 1
Gerrit-Owner: cron2 <ge...@gr...>
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: flichtenheld (C. Review) <ge...@op...> - 2025-10-28 19:51:14
       | 
| Attention is currently required from: cron2, plaisthos. flichtenheld has posted comments on this change by cron2. ( http://gerrit.openvpn.net/c/openvpn/+/1323?usp=email ) Change subject: zeroize struct image in packet_id_persist_save() before writing to disk ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.openvpn.net/c/openvpn/+/1323?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: master Gerrit-Change-Id: I7f4c7b0ca748975defca1e5104e7077a761cd49c Gerrit-Change-Number: 1323 Gerrit-PatchSet: 1 Gerrit-Owner: cron2 <ge...@gr...> Gerrit-Reviewer: flichtenheld <fr...@li...> Gerrit-Reviewer: plaisthos <arn...@rf...> Gerrit-CC: openvpn-devel <ope...@li...> Gerrit-Attention: plaisthos <arn...@rf...> Gerrit-Attention: cron2 <ge...@gr...> Gerrit-Comment-Date: Tue, 28 Oct 2025 19:51:04 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes | 
| 
      
      
      From: Gert D. <ge...@gr...> - 2025-10-28 20:32:10
       | 
| while this really is only a debug function, ensuring that no uninitialized heap content ends up in padding in the structure and thus to disk is good practice. Reported-by: Joshua Rogers <co...@jo...> Found-by: ZeroPath (https://zeropath.com/) Change-Id: I7f4c7b0ca748975defca1e5104e7077a761cd49c Signed-off-by: Gert Doering <ge...@gr...> Acked-by: Frank Lichtenheld <fr...@li...> Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1323 --- This change was reviewed on Gerrit and approved by at least one developer. I request to merge it to master. Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1323 This mail reflects revision 1 of this Change. Acked-by according to Gerrit (reflected above): Frank Lichtenheld <fr...@li...> diff --git a/src/openvpn/packet_id.c b/src/openvpn/packet_id.c index 880eee1..08d9d9b 100644 --- a/src/openvpn/packet_id.c +++ b/src/openvpn/packet_id.c @@ -511,6 +511,7 @@ && (p->time != p->time_last_written || p->id != p->id_last_written)) { struct packet_id_persist_file_image image; + CLEAR(image); ssize_t n; off_t seek_ret; struct gc_arena gc = gc_new(); | 
| 
      
      
      From: Gert D. <ge...@gr...> - 2025-10-29 07:10:36
       | 
| Not the most critical path, but "initialize data structures before using"
is considered good practice :-)  - BB is also happy with it.
Patch has been applied to the master branch.
commit c58b6e73c3508b40b3d2f26eebfc3aa4df53e524
Author: Gert Doering
Date:   Tue Oct 28 21:31:50 2025 +0100
     zeroize struct image in packet_id_persist_save() before writing to disk
     Signed-off-by: Gert Doering <ge...@gr...>
     Acked-by: Frank Lichtenheld <fr...@li...>
     Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1323
     Message-Id: <202...@gr...>
     URL: https://www.mail-archive.com/ope...@li.../msg33983.html
     Signed-off-by: Gert Doering <ge...@gr...>
--
kind regards,
Gert Doering
 | 
| 
      
      
      From: cron2 (C. Review) <ge...@op...> - 2025-10-29 07:10:54
       | 
| cron2 has submitted this change. ( http://gerrit.openvpn.net/c/openvpn/+/1323?usp=email ) Change subject: zeroize struct image in packet_id_persist_save() before writing to disk ...................................................................... zeroize struct image in packet_id_persist_save() before writing to disk while this really is only a debug function, ensuring that no uninitialized heap content ends up in padding in the structure and thus to disk is good practice. Reported-by: Joshua Rogers <co...@jo...> Found-by: ZeroPath (https://zeropath.com/) Change-Id: I7f4c7b0ca748975defca1e5104e7077a761cd49c Signed-off-by: Gert Doering <ge...@gr...> Acked-by: Frank Lichtenheld <fr...@li...> Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1323 Message-Id: <202...@gr...> URL: https://www.mail-archive.com/ope...@li.../msg33983.html Signed-off-by: Gert Doering <ge...@gr...> --- M src/openvpn/packet_id.c 1 file changed, 1 insertion(+), 0 deletions(-) diff --git a/src/openvpn/packet_id.c b/src/openvpn/packet_id.c index 880eee1..08d9d9b 100644 --- a/src/openvpn/packet_id.c +++ b/src/openvpn/packet_id.c @@ -511,6 +511,7 @@ && (p->time != p->time_last_written || p->id != p->id_last_written)) { struct packet_id_persist_file_image image; + CLEAR(image); ssize_t n; off_t seek_ret; struct gc_arena gc = gc_new(); -- To view, visit http://gerrit.openvpn.net/c/openvpn/+/1323?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: master Gerrit-Change-Id: I7f4c7b0ca748975defca1e5104e7077a761cd49c Gerrit-Change-Number: 1323 Gerrit-PatchSet: 2 Gerrit-Owner: 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-29 07:10:58
       | 
| Hello flichtenheld, plaisthos, 
I'd like you to reexamine a change. Please visit
    http://gerrit.openvpn.net/c/openvpn/+/1323?usp=email
to look at the new patch set (#2).
The following approvals got outdated and were removed:
Code-Review+2 by flichtenheld
Change subject: zeroize struct image in packet_id_persist_save() before writing to disk
......................................................................
zeroize struct image in packet_id_persist_save() before writing to disk
while this really is only a debug function, ensuring that no uninitialized
heap content ends up in padding in the structure and thus to disk is good
practice.
Reported-by: Joshua Rogers <co...@jo...>
Found-by: ZeroPath (https://zeropath.com/)
Change-Id: I7f4c7b0ca748975defca1e5104e7077a761cd49c
Signed-off-by: Gert Doering <ge...@gr...>
Acked-by: Frank Lichtenheld <fr...@li...>
Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1323
Message-Id: <202...@gr...>
URL: https://www.mail-archive.com/ope...@li.../msg33983.html
Signed-off-by: Gert Doering <ge...@gr...>
---
M src/openvpn/packet_id.c
1 file changed, 1 insertion(+), 0 deletions(-)
  git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/23/1323/2
diff --git a/src/openvpn/packet_id.c b/src/openvpn/packet_id.c
index 880eee1..08d9d9b 100644
--- a/src/openvpn/packet_id.c
+++ b/src/openvpn/packet_id.c
@@ -511,6 +511,7 @@
         && (p->time != p->time_last_written || p->id != p->id_last_written))
     {
         struct packet_id_persist_file_image image;
+        CLEAR(image);
         ssize_t n;
         off_t seek_ret;
         struct gc_arena gc = gc_new();
-- 
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1323?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: master
Gerrit-Change-Id: I7f4c7b0ca748975defca1e5104e7077a761cd49c
Gerrit-Change-Number: 1323
Gerrit-PatchSet: 2
Gerrit-Owner: cron2 <ge...@gr...>
Gerrit-Reviewer: flichtenheld <fr...@li...>
Gerrit-Reviewer: plaisthos <arn...@rf...>
Gerrit-CC: openvpn-devel <ope...@li...>
 |