opendmarc/opendmarc.c:dmarfc_config_free: don't assert conf->conf_refcnt == 0

[dilyanpalauzov]

As described at trusteddomainproject/OpenDKIM#22.

diff --git a/opendmarc/opendmarc.c b/opendmarc/opendmarc.c
--- a/opendmarc/opendmarc.c
+++ b/opendmarc/opendmarc.c
@@ -3932,7 +3932,6 @@ static void
 dmarcf_config_free(struct dmarcf_config *conf)
 {
        assert(conf != NULL);
-       assert(conf->conf_refcnt == 0);
 
        if (conf->conf_data != NULL)
                config_free(conf->conf_data);

[thegushi]

This has been removed in the develop branch.

[dilyanpalauzov]

On the develop branch conf_refcnt is still present.

[thegushi]

I thought I had removed this. I will see if we can get this fixed before 1.4.1.

…

On Apr 8, 2021, at 12:09 PM, Дилян Палаузов ***@***.***> wrote: On the develop branch conf_refcnt is still present. — You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub <#18 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAIWKKCKTSHJNEDQBR4F2QDTHX5NXANCNFSM4FCHOTZA>.

[mskucherawy]

Reopening as it needs to be resolved for OpenDMARC. It's still present on "develop".

[leres]

I'm running 1.4.2 on FreeBSD 13.3-RELEASE and recently started seeing crashes. I built with debug symbols and left opendmarc running from gdb. The next time it crashed I got this stack trace:

Thread 1 received signal SIGABRT, Aborted.
Sent by thr_kill() from pid 30872 and user 26.
0x000000080044041a in thr_kill () from /lib/libc.so.7
(gdb) bt
#0  0x000000080044041a in thr_kill () from /lib/libc.so.7
#1  0x00000008003b9e64 in raise () from /lib/libc.so.7
#2  0x000000080046a6f9 in abort () from /lib/libc.so.7
#3  0x000000080039d201 in __assert () from /lib/libc.so.7
#4  0x0000000000210c0f in dmarcf_config_free (conf=0x800a1a000)
    at opendmarc.c:4162
#5  0x0000000000213460 in main (argc=11, argv=0x7fffffffe2e8)
    at opendmarc.c:5323

Line 4162 is the one referenced in this issue:

assert(conf->conf_refcnt == 0);

I came to github, did a search for conf_refcnt, and found this issue. Should this problematic assert() be removed or not?

[thegushi]

A fix for this has been submitted in PR #298 on the fix/crashes-18-140-152-256 branch, which also covers the related bugs in #18, #140, #152, and #256.

If you are able to test against that branch and confirm the fix, that would be very helpful for getting it merged. Thank you!

[dilyanpalauzov]

No, I cannot test it. I stopped submitting proposals for changes here long time ago and my branch, which I use in production, has diverged from what is upstream. I am not not in a mood to rebase my branch on what is upstream. And even if I test it, it does not mean, that in the next few years the change will be included in a release.

As an example for the processes here, this conversation happened in 2021:

On the develop branch conf_refcnt is still present.

I thought I had removed this. I will see if we can get this fixed before 1.4.1.

In 2026 the file on the develop branch conf_refcnt is still present.

[thegushi]

Your skepticism is understood, and I apologize.

Either way, I'm planning on a release (as well as a fix for our website and mailing lists) fairly soon. There's a new DMARCbis RFC likely to come out soon and I'm preparing for that.