Bug 2100287

Ack. This is quite unexpected of course, there aren't that many changes to popt here, but then you never know.

Thanks for the report, I'll look into it ASAP.

Easily reproduced. authconfig appears to be relying on popt maintaining state after poptFreeContext(), in other words relying on popt leaking memory. And yes, popt was leaking memory here, probably for a long time, and this is one of the fixes in 1.19.

A pointer returned by poptGetArg() is only valid as long as the popt context is it was used on is valid. Authconfig needs to either
a) preserve the popt context as long as the data is needed (initialize context early, free just before exit is a good strategy to avoid these sort of issues)
b) make a copy of the data returned (and thus manage the allocation locally)

This is sufficient for fixing the above reproducer, I don't know if there are other cases left. I see a comment about discarding const "because it's not worth it" in cli_tool_common_opts() which may indicate another trouble spot.

diff --git a/src/cli/cli_tool.c b/src/cli/cli_tool.c
index 83bc1ef..4c1fbbe 100644
--- a/src/cli/cli_tool.c
+++ b/src/cli/cli_tool.c
@@ -379,7 +379,7 @@ errno_t cli_tool_popt_ex(struct cli_cmdline *cmdline,
             }
         }
 
-        *_fopt = fopt;
+        *_fopt = strdup(fopt);
     } else if (_fopt == NULL && fopt != NULL) {
         /* Unexpected free argument. */
         fprintf(stderr, _("Unexpected parameter: %s\n\n"), fopt);

This of course leaks memory, only now in authconfig (a cli tool leaking a bit of memory is a much lesser evil than a library doing so in any case). Fixing that is up to authconfig maintainers, I don't know whether a) or b) is preferable to them.

Just FWIW, https://github.com/rpm-software-management/popt/commit/7182e4618ad5a0186145fc2aa4a98c2229afdfa8 is what reveals the abuse in authconfig, and where there's one there may be more. We can certainly revert that patch temporarily if necessary, but I'd rather see the users fixed instead.

Thank you. I will try to fix it soon, but it won't be sooner than the beginning of next month.

The case which I'm not sure is caused by authconfig is live image boot. If we want to test, we'd need to run a test with the new popt and a patched authconfig and see if there's still a problem.

Panu, are you okay with waiting until Pavel can fix authconfig for popt 1.19 to go into Rawhide?

Further note on the live image case: the error messages I was seeing about sysctl and modprobe aren't actually the issue. They appear even in boots with popt 1.18 that work fine, they're apparently not fatal problems. One of them is caused by anaconda trying to load cramfs which isn't in the initramfs environment, and the other is caused by a modprobe config file installed by nfs-utils that tries to run sysctl when loading the nfs module; sysctl also isn't in the initramfs environment. I've filed separate issues on those. Whatever causes live images not to boot, it's not those errors. So it may well just be authconfig again.

> Panu, are you okay with waiting until Pavel can fix authconfig for popt 1.19 to go into Rawhide?

I'm off to a vacation after this week and will only be back in August. Michal (cc'd), I suppose babysitting the new popt in rawhide and helping out debugging possible other failure cases falls on your lap in the meanwhile. 

I know at least gdisk is affected but that's already tracked (bug 2100391), there may be others.

/me notes s/authconfig/authselect/ in all of the above, old names die hard :D

*** Bug 2101566 has been marked as a duplicate of this bug. ***

I'll build popt 1.19 with the memleak fix reverted to get out of the current limbo state - untagging is good for disaster prevention but rawhide + dist-git disagreeing is not a sustainable state.

FWIW, popt 1.19 minus the memleak patch is now back in rawhide.

This one contains the memleak patch so I can use it for testing? https://koji.fedoraproject.org/koji/buildinfo?buildID=1990967

(In reply to Pavel Březina from comment #11)
> This one contains the memleak patch so I can use it for testing?
> https://koji.fedoraproject.org/koji/buildinfo?buildID=1990967

Yes.

Please, test this scratch build: https://koji.fedoraproject.org/koji/taskinfo?taskID=89233014

Sorry for the delay: I tested, and it looks good. That authselect build looks like it works OK with the earlier popt build that has the memleak patch.

I guess we'll leave it to popt maintainers to decide when to go back to an unpatched build. I'll open a new bug if I still see problems if/when that happens.

FYI, the temporary revert now undone in popt-1.19~rc1-4