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/+/1026?usp=email
to review the following change.
Change subject: dns: fix potential NULL pointer dereference
......................................................................
dns: fix potential NULL pointer dereference
Fix issue reported by Coverity (CID 1646952): Dereferencing a pointer
that might be NULL dvf when calling env_set_write_file.
In addition to the fix, output a log line in case this happens, because
it will influence to communication with the updown runner process, i.e.
setting up / tearing down DNS things will not work as expected.
Change-Id: I275bf939f43577427e14890e7093d63c5213ae5d
Signed-off-by: Heiko Hund <[email protected]>
---
M src/openvpn/dns.c
1 file changed, 35 insertions(+), 26 deletions(-)
git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/26/1026/1
diff --git a/src/openvpn/dns.c b/src/openvpn/dns.c
index 9927961..1299a13 100644
--- a/src/openvpn/dns.c
+++ b/src/openvpn/dns.c
@@ -691,12 +691,13 @@
static const char *
write_dns_vars_file(bool up, const struct options *o, const struct tuntap *tt,
struct gc_arena *gc)
{
- struct env_set *es = env_set_create(gc);
const char *dvf = platform_create_temp_file(o->tmp_dir, "dvf", gc);
-
- updown_env_set(up, &o->dns_options, tt, es);
- env_set_write_file(dvf, es);
-
+ if (dvf)
+ {
+ struct env_set *es = env_set_create(gc);
+ updown_env_set(up, &o->dns_options, tt, es);
+ env_set_write_file(dvf, es);
+ }
return dvf;
}
@@ -731,34 +732,42 @@
int rfd = updown_runner->fds[0];
int wfd = updown_runner->fds[1];
const char *dvf = write_dns_vars_file(up, o, tt, &gc);
- size_t dvf_size = strlen(dvf) + 1;
-
- while (1)
+ if (!dvf)
{
- ssize_t len = write(wfd, dvf, dvf_size);
- if (len < dvf_size)
- {
- if (len == -1 && errno == EINTR)
- {
- continue;
- }
- msg(M_ERR | M_ERRNO, "could not send dns vars filename");
- }
- break;
+ msg(M_ERR, "could not create dns vars file");
+ status = 1;
}
-
- while (1)
+ else
{
- ssize_t len = read(rfd, &status, sizeof(status));
- if (len < sizeof(status))
+ size_t dvf_size = strlen(dvf) + 1;
+
+ while (1)
{
- if (len == -1 && errno == EINTR)
+ ssize_t len = write(wfd, dvf, dvf_size);
+ if (len < dvf_size)
{
- continue;
+ if (len == -1 && errno == EINTR)
+ {
+ continue;
+ }
+ msg(M_ERR | M_ERRNO, "could not send dns vars filename");
}
- msg(M_ERR | M_ERRNO, "could not receive dns updown status");
+ break;
}
- break;
+
+ while (1)
+ {
+ ssize_t len = read(rfd, &status, sizeof(status));
+ if (len < sizeof(status))
+ {
+ if (len == -1 && errno == EINTR)
+ {
+ continue;
+ }
+ msg(M_ERR | M_ERRNO, "could not receive dns updown
status");
+ }
+ break;
+ }
}
gc_free(&gc);
--
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1026?usp=email
To unsubscribe, or for help writing mail filters, visit
http://gerrit.openvpn.net/settings
Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: I275bf939f43577427e14890e7093d63c5213ae5d
Gerrit-Change-Number: 1026
Gerrit-PatchSet: 1
Gerrit-Owner: d12fk <[email protected]>
Gerrit-Reviewer: flichtenheld <[email protected]>
Gerrit-Reviewer: plaisthos <[email protected]>
Gerrit-CC: openvpn-devel <[email protected]>
Gerrit-Attention: plaisthos <[email protected]>
Gerrit-Attention: flichtenheld <[email protected]>
Gerrit-MessageType: newchange
_______________________________________________
Openvpn-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/openvpn-devel