[GIT PULL] execve updates for v6.13-rc1 (take 2)
Eric W. Biederman
ebiederm at xmission.com
Fri Nov 29 19:33:19 UTC 2024
Casey and the smack folks my apologies for copying you in.
I just read the code below a little more carefully and it is definitely
a systemd bug.
mac_smack_read_fd reads the xattr that smack will apply as a label if it
is present. So there is no reason for systemd to apply the label
itself. Worse smack_bprm_creds_for_exec applies checks before
applying the label (aka is the superblock trusted) that systemd doesn't.
Which means systemd might apply a label from a smack xattr when
smack wouldn't.
> static int setup_smack(
> const ExecParameters *params,
> const ExecContext *context,
> int executable_fd) {
> int r;
>
> assert(params);
> assert(executable_fd >= 0);
>
> if (context->smack_process_label) {
> r = mac_smack_apply_pid(0, context->smack_process_label);
> if (r < 0)
> return r;
> } else if (params->fallback_smack_process_label) {
> _cleanup_free_ char *exec_label = NULL;
>
> r = mac_smack_read_fd(executable_fd, SMACK_ATTR_EXEC, &exec_label);
> if (r < 0 && !ERRNO_IS_XATTR_ABSENT(r))
> return r;
>
> r = mac_smack_apply_pid(0, exec_label ?: params->fallback_smack_process_label);
> if (r < 0)
> return r;
> }
>
> return 0;
> }
Which means the systemd code should really be:
> static int setup_smack(
> const ExecParameters *params,
> const ExecContext *context) {
> int r;
>
> assert(params);
> if (context->smack_process_label) {
> r = mac_smack_apply_pid(0, context->smack_process_label);
> if (r < 0)
> return r;
> } else if (params->fallback_smack_process_label) {
> r = mac_smack_apply_pid(0, params->fallback_smack_process_label);
> if (r < 0)
> return r;
> }
>
> return 0;
> }
At which point systemd has no need to open the executable file
descriptor and thus no need to play with fexecve.
Eric
More information about the Linux-security-module-archive
mailing list