🔴 In selinux_getpolicytype() (lib/ansible/module_utils/compat/selinux.py:74-80), the C string allocated by _selinux.selinux_getpolicytype() is never freed with freecon(), unlike lgetfilecon_raw() and matchpathcon() in the same file which both correctly call _selinux.freecon(context) before returning. The fix is to add _selinux.freecon(policytype) after decoding the value and before return result.
Extended reasoning...
The selinux_getpolicytype() function in the new compat layer calls _selinux.selinux_getpolicytype(ctypes.byref(policytype)). The C function allocates a string internally (via strdup()) and stores the pointer in the c_char_p buffer passed by reference. The caller is responsible for freeing this memory using freecon().
Lines 74-80 of lib/ansible/module_utils/compat/selinux.py:
def selinux_getpolicytype():
policytype = ctypes.c_char_p()
rc = _selinux.selinux_getpolicytype(ctypes.byref(policytype))
if rc == 0 and policytype.value is not None:
result = [rc, policytype.value.decode('utf-8', 'surrogateescape')]
return result # freecon(policytype) missing here
return [rc, '']lgetfilecon_raw() (line 38) and matchpathcon() (line 51) both correctly call _selinux.freecon(context) after decoding the value and before returning. Python's garbage collector frees the ctypes.c_char_p Python wrapper object but does NOT free the underlying C-heap memory, because ctypes does not own allocations made by C library functions.
Every call to selinux_getpolicytype() leaks the C-allocated policy type string. This is called from SelinuxFactCollector.collect() in facts/system/selinux.py on every fact collection run on SELinux-enabled systems. While Ansible modules are short-lived and the OS reclaims memory at exit, the leak is real and violates the libselinux API contract.
Add _selinux.freecon(policytype) after decoding the value:
def selinux_getpolicytype():
policytype = ctypes.c_char_p()
rc = _selinux.selinux_getpolicytype(ctypes.byref(policytype))
if rc == 0 and policytype.value is not None:
result = [rc, policytype.value.decode('utf-8', 'surrogateescape')]
_selinux.freecon(policytype) # add this line
return result
return [rc, '']- Ansible collects facts on a SELinux-enabled system, calling
SelinuxFactCollector.collect(). collect()callsselinux.selinux_getpolicytype()(facts/system/selinux.py line 79).- The C function allocates e.g.
"targeted\0"viastrdup()and stores the pointer inpolicytype. policytype.value.decode(...)produces a Python string;resultholds[0, "targeted"].return resultexecutes — the C allocation is never passed tofreecon()and leaks.- On every subsequent fact collection run, another allocation is made and leaked.
🟡 Both new error messages in yum.py contain a double period ('instead.. Please') because the original sentence ending in '.' was concatenated with a new sentence starting with '. Please', producing malformed user-facing output. Fix by removing the trailing period before the continuation text in both strings at lines 1611 and 1614.
Extended reasoning...
The bug is a straightforward typo introduced in this PR in lib/ansible/modules/yum.py. The original error strings ended cleanly with a single period: '...use the dnf Ansible module instead.' The PR replaced them with longer strings that append additional guidance, but failed to remove the trailing period before appending the new sentence.
The resulting strings contain 'instead.. Please' -- two consecutive periods where one is expected. This is visible in both the HAS_RPM_PYTHON and HAS_YUM_PYTHON error messages at lines 1611 and 1614.
The specific code path: when the yum module runs on a system where either HAS_RPM_PYTHON or HAS_YUM_PYTHON is False (i.e., the required Python 2 bindings are not present), these error messages are appended to error_msgs and eventually surfaced to the user via self.module.fail_json(msg='. '.join(error_msgs)). Any user missing the RPM or yum Python bindings will see the double period in the failure message.
Existing code does not prevent this because it is purely a string literal typo -- no validation of error message strings takes place.
The impact is cosmetic: users see 'instead.. Please' instead of 'instead. Please', which looks unprofessional and may cause confusion, but the message is still intelligible and functionality is unaffected.
Step-by-step proof:
- On a Python 3 system, 'import rpm' fails so HAS_RPM_PYTHON = False.
- run() is invoked; respawn logic finds no suitable interpreter and falls through.
- error_msgs.append('...instead.. Please install the python-rpm package...') is executed (line 1611).
- self.module.fail_json(msg='. '.join(error_msgs)) is called with the double-period string.
- The user sees: 'The Python 2 bindings for rpm are needed for this module. If you require Python 3 support use the dnf Ansible module instead.. Please install the python-rpm package...'
Fix: change 'instead.. ' to 'instead. ' in both string literals at lines 1611 and 1614.
🟡 In apt_repository.py main(), the else branch of if params['install_python_apt']: wraps module.fail_json(...) in a redundant inner if not HAVE_PYTHON_APT: guard that is always True — we are already inside the outer if not HAVE_PYTHON_APT: block and nothing in the else-branch modifies that global. The original code called module.fail_json directly; the new inner condition is dead code that obscures intent and may suggest the developer meant to check after install_python_apt() ran (which only executes in the other branch).
Extended reasoning...
What the bug is and how it manifests
In lib/ansible/modules/apt_repository.py, the PR restructured the missing-apt handling inside main() to add respawn logic. The resulting code structure is roughly:
if not HAVE_PYTHON_APT: # outer — HAVE_PYTHON_APT is False here if not has_respawned(): interpreter = probe_interpreters_for_module(...) if interpreter: respawn_module(interpreter) if params['install_python_apt']: install_python_apt(module) else: if not HAVE_PYTHON_APT: # inner — always True, redundant module.fail_json(...)
The inner if not HAVE_PYTHON_APT: at the else-branch is always True, making it dead code.
The specific code path that triggers it
This branch is reached when: (1) apt cannot be imported at module load time, (2) either the module has already been respawned or no suitable interpreter was found, and (3) the user set install_python_apt=False. In that scenario, execution falls into the else: block and hits the always-True guard.
Why existing code does not prevent it
HAVE_PYTHON_APT is a module-level global set once at import time. Inside the outer if not HAVE_PYTHON_APT: block, the only statement that sets it to True is install_python_apt(module) — and that function is only called in the if params['install_python_apt']: branch, not the else: branch under examination. respawn_module() either calls sys.exit() (replacing the process) or raises an exception; it never returns and cannot affect the global. Therefore HAVE_PYTHON_APT is still False when the inner check is evaluated.
Impact
No functional impact — module.fail_json(...) is still always reached in this branch. However, the spurious guard is misleading dead code that implies HAVE_PYTHON_APT could theoretically be True at that point. It also diverges from the analogous pattern in apt.py (same PR), which calls module.fail_json() directly without a second check.
How to fix
Remove the inner if not HAVE_PYTHON_APT: guard and call module.fail_json(...) directly:
else: module.fail_json(msg="{0} must be installed and visible from {1}.".format(PYTHON_APT, sys.executable))
Step-by-step proof
- Module starts; import apt fails at top of file → HAVE_PYTHON_APT = False.
- Execution enters main(). if not HAVE_PYTHON_APT: is True, we enter the outer block.
- Either has_respawned() returns True, or probe_interpreters_for_module returns None — either way respawn_module() is not called and we continue.
- params['install_python_apt'] is False (user configured this) → we take the else branch.
- HAVE_PYTHON_APT is still False; nothing between step 2 and here has modified it.
- if not HAVE_PYTHON_APT: evaluates to not False = True — the condition is trivially and always satisfied.
- module.fail_json(...) is called. The inner guard filtered nothing and served no purpose.