Re: [GIT PULL] execve updates for v6.13-rc1 (take 2)

["Eric W. Biederman" <ebiederm@xmission.com>]

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