Hi guys,
I am continuing in this thread since it's related to the SRV resolution
feature.
During incorporation of this feature to Fedora and RHEL, static analysis
found some
minor flaws that I want to report and propose addressing them.
There is an unnecessary double check of the "addr_list" pointer for null
value in "smtp_service_addr"
function. I removed it.
There is an unnecessary double check of the "addr_list" pointer for null
value in "smtp_domain_addr"
function. I removed it.
In posttls-finger there are problems with using "service" char pointer in
logging. I initialized the variable
to null and ensured that it does not point into already freed space.
Thanks for any response.
On Mon, Feb 20, 2023 at 12:24 AM Wietse Venema <wie...@porcupine.org> wrote:
> Wietse Venema:
> > Tomas Korbar:
> > > Hi guys,
> > > It's great news that this is at least in non production release!
> > > Thanks for all your work on this. Let me know if I can provide further
> > > help.
> >
> > As you can see I overhauled the user interface a bit. Does the user
> > interface cover your use cases well? The documentation describes a
> > number of examples.
> >
> > https://www.postfix.org/postconf.5.html#use_srv_lookup
>
> This is now in the postfix-3.8-20230219 regular release. I kept
> simplifying the code until even I could understand why it works.
>
> Wietse
>
>
commit 54383a02fe93e0c3db7bec5a139e12b62db25885
Author: Tomas Korbar <tkor...@redhat.com>
Date: Tue May 16 12:36:08 2023 +0200
Remove dead code branch from smtp_service_addr function
best_found variable can be directly set to value of addr_list->pref,
because the if statement before guarantees that the addr_list variable
can not be null.
diff --git a/src/smtp/smtp_addr.c b/src/smtp/smtp_addr.c
index 5e23388..0a9cd89 100644
--- a/src/smtp/smtp_addr.c
+++ b/src/smtp/smtp_addr.c
@@ -810,7 +810,7 @@ DNS_RR *smtp_service_addr(const char *name, const char *service, DNS_RR **mxrr,
break;
}
/* Optional loop prevention, similar to smtp_domain_addr(). */
- best_found = (addr_list ? addr_list->pref : IMPOSSIBLE_PREFERENCE);
+ best_found = addr_list->pref;
if (msg_verbose)
smtp_print_addr(aname, addr_list);
if ((misc_flags & SMTP_MISC_FLAG_LOOP_DETECT)
commit 48f9df7c8df79d9af8a0e6254d4af5fde3d46e17
Author: Tomas Korbar <tkor...@redhat.com>
Date: Tue May 16 12:50:14 2023 +0200
Initialize service property in connect_remote function
When reconnection is required then parsing of destination
is avoided and service property can be used in logging without
proper initialization. Initialize it to 0 and if required
than substitute it with default service.
diff --git a/src/posttls-finger/posttls-finger.c b/src/posttls-finger/posttls-finger.c
index 2f3a58e..8ace9ba 100644
--- a/src/posttls-finger/posttls-finger.c
+++ b/src/posttls-finger/posttls-finger.c
@@ -1592,11 +1592,12 @@ static void connect_remote(STATE *state, char *dest)
DNS_RR *addr;
char *buf;
char *domain;
- char *service;
+ char *service = 0;
+ char *def_service = state->smtp ? "smtp" : "24";
/* When reconnecting use IP address of previous session */
if (state->addr == 0) {
- buf = parse_destination(dest, state->smtp ? "smtp" : "24",
+ buf = parse_destination(dest, def_service,
&domain, &service, &state->port);
if (!state->nexthop)
state->nexthop = mystrdup(domain);
@@ -1623,7 +1624,7 @@ static void connect_remote(STATE *state, char *dest)
if (level == TLS_LEV_INVALID
|| (state->stream = connect_addr(state, addr)) == 0) {
msg_info("Failed to establish session to %s:%s via %s:%u: %s",
- dest, service, HNAME(addr), addr->port,
+ dest, service ? service : def_service, HNAME(addr), addr->port,
vstring_str(state->why->reason));
continue;
}
commit 2f99e0ec025774e9a9bdaa91c631b3148893b3e3
Author: Tomas Korbar <tkor...@redhat.com>
Date: Tue May 16 12:51:54 2023 +0200
Do not free buf in connect_remote function prematurely
service variable is a pointer into buf and since it is used
in logging, it is not possible to free buf until it is certain
that service will not be used anymore.
diff --git a/src/posttls-finger/posttls-finger.c b/src/posttls-finger/posttls-finger.c
index 8ace9ba..09fea9b 100644
--- a/src/posttls-finger/posttls-finger.c
+++ b/src/posttls-finger/posttls-finger.c
@@ -1590,7 +1590,7 @@ static char *parse_destination(char *destination, char *def_service,
static void connect_remote(STATE *state, char *dest)
{
DNS_RR *addr;
- char *buf;
+ char *buf = 0;
char *domain;
char *service = 0;
char *def_service = state->smtp ? "smtp" : "24";
@@ -1607,11 +1607,11 @@ static void connect_remote(STATE *state, char *dest)
state->addr = service_addr(state, domain, service);
else
state->addr = domain_addr(state, domain);
- myfree(buf);
if (state->addr == 0) {
msg_info("Destination address lookup failed: %s",
vstring_str(state->why->reason));
+ myfree(buf);
return;
}
}
@@ -1638,6 +1638,8 @@ static void connect_remote(STATE *state, char *dest)
state->addr = addr;
break;
}
+ if (buf)
+ myfree(buf);
}
/* connect_dest - connect to given inet: or unix: destination */
commit c6fe7870b1c68fdc5133fdea3a3752b8d728d5d5
Author: Tomas Korbar <tkor...@redhat.com>
Date: Tue May 16 15:19:04 2023 +0200
Remove dead code branch from smtp_domain_addr function
best_found variable can be directly set to value of addr_list->pref,
because the if statement before guarantees that the addr_list variable
can not be null.
diff --git a/src/smtp/smtp_addr.c b/src/smtp/smtp_addr.c
index 0a9cd89..d7a053b 100644
--- a/src/smtp/smtp_addr.c
+++ b/src/smtp/smtp_addr.c
@@ -637,7 +637,7 @@ DNS_RR *smtp_domain_addr(const char *name, DNS_RR **mxrr, int misc_flags,
msg_warn("no MX host for %s has a valid address record", name);
break;
}
- best_found = (addr_list ? addr_list->pref : IMPOSSIBLE_PREFERENCE);
+ best_found = addr_list->pref;
if (msg_verbose)
smtp_print_addr(name, addr_list);
if ((misc_flags & SMTP_MISC_FLAG_LOOP_DETECT)
_______________________________________________
Postfix-devel mailing list -- postfix-devel@postfix.org
To unsubscribe send an email to postfix-devel-le...@postfix.org
