Skip to content

Conversation

@ikerexxe
Copy link
Collaborator

@ikerexxe ikerexxe commented Dec 4, 2025

Initialize option_flags structure to prevent garbage memory values in flags.chroot and flags.prefix fields. Uninitialized memory caused architecture-specific failures where process_selinux evaluation differed between x86_64 and aarch64, leading to pw_close() failures when SELinux contexts weren't properly managed.

Fixes: c0c9485 (2025-04-25; "src/useradd.c: chroot or prefix SELinux file context")
Link: https://bodhi.fedoraproject.org/updates/FEDORA-2025-3d835cfb15

@ikerexxe
Copy link
Collaborator Author

ikerexxe commented Dec 4, 2025

As an additional comment, I wonder if we should do the same in all definitions of struct option_flags, as this bug probably also occurs with other binaries.

Initialize option_flags structure to prevent garbage memory values in
`flags.chroot` and `flags.prefix` fields. Uninitialized memory caused
architecture-specific failures where process_selinux evaluation
differed between x86_64 and aarch64, leading to `pw_close()` failures
when SELinux contexts weren't properly managed.

Fixes: c0c9485 (2025-04-25; "src/useradd.c: chroot or prefix SELinux file context")
Link: <https://bodhi.fedoraproject.org/updates/FEDORA-2025-3d835cfb15>
Signed-off-by: Iker Pedrosa <ipedrosa@redhat.com>
Copy link
Collaborator

@alejandro-colomar alejandro-colomar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! LGTM.

Reviewed-by: Alejandro Colomar <alx@kernel.org>

@alejandro-colomar alejandro-colomar merged commit f4c9f5e into shadow-maint:master Dec 5, 2025
11 checks passed
@alejandro-colomar
Copy link
Collaborator

BTW, the commit that this fixes touched many more files. Did you check if any other files may have similar issues?

@ikerexxe
Copy link
Collaborator Author

ikerexxe commented Dec 5, 2025

This issue was detected by an automated test and I think that only tests useradd. Probably other binaries are also affected, that's why I wrote:

As an additional comment, I wonder if we should do the same in all definitions of struct option_flags, as this bug probably also occurs with other binaries.

I wanted to have the fix ready for useradd before delving into that, but you merged the changes quite fast.

WDYT? Should we also make similar changes in other binaries?

alejandro-colomar added a commit to alejandro-colomar/shadow that referenced this pull request Dec 5, 2025
Fixes: c0c9485 (2025-10-07; "src/useradd.c: chroot or prefix SELinux file context")
Link: <shadow-maint#1396>
Cc: Iker Pedrosa <ipedrosa@redhat.com>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
@alejandro-colomar
Copy link
Collaborator

This issue was detected by an automated test and I think that only tests useradd. Probably other binaries are also affected, that's why I wrote:

As an additional comment, I wonder if we should do the same in all definitions of struct option_flags, as this bug probably also occurs with other binaries.

I wanted to have the fix ready for useradd before delving into that, but you merged the changes quite fast.

Well, the fix was obviously good, so I merged it. :)

WDYT? Should we also make similar changes in other binaries?

Yup, I've checked the source code, and the bug affects all binaries. I've pushed a fix in #1400 .

alejandro-colomar added a commit to alejandro-colomar/shadow that referenced this pull request Dec 5, 2025
In all these functions, we were setting the flags to 'true' in
process_flags() if the appropriate command-line flag was specified.
However, they were never being defined to 'false', which should be the
default value.  Initialize these flags to false.

Fixes: c0c9485 (2025-10-07; "src/useradd.c: chroot or prefix SELinux file context")
Link: <shadow-maint#1396>
Cc: Iker Pedrosa <ipedrosa@redhat.com>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
alejandro-colomar added a commit to alejandro-colomar/shadow that referenced this pull request Dec 5, 2025
In all these functions, we were setting the flags to 'true' in
process_flags() if the appropriate command-line flag was specified.
However, they were never being defined to 'false', which should be the
default value.  Initialize these flags to false.

Fixes: c0c9485 (2025-10-07; "src/useradd.c: chroot or prefix SELinux file context")
Link: <shadow-maint#1396>
Reviewed-by: Serge Hallyn <serge@hallyn.com>
Cc: Iker Pedrosa <ipedrosa@redhat.com>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
ikerexxe pushed a commit that referenced this pull request Dec 5, 2025
In all these functions, we were setting the flags to 'true' in
process_flags() if the appropriate command-line flag was specified.
However, they were never being defined to 'false', which should be the
default value.  Initialize these flags to false.

Fixes: c0c9485 (2025-10-07; "src/useradd.c: chroot or prefix SELinux file context")
Link: <#1396>
Reviewed-by: Serge Hallyn <serge@hallyn.com>
Cc: Iker Pedrosa <ipedrosa@redhat.com>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
ikerexxe pushed a commit to ikerexxe/shadow that referenced this pull request Dec 9, 2025
In all these functions, we were setting the flags to 'true' in
process_flags() if the appropriate command-line flag was specified.
However, they were never being defined to 'false', which should be the
default value.  Initialize these flags to false.

Fixes: c0c9485 (2025-10-07; "src/useradd.c: chroot or prefix SELinux file context")
Link: <shadow-maint#1396>
Reviewed-by: Serge Hallyn <serge@hallyn.com>
Cc: Iker Pedrosa <ipedrosa@redhat.com>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants