Skip to content

mac_do(4): added traditional syscalls support to mac_do(4)#5

Open
thesynthax wants to merge 2 commits intoOlCe2:oc-thesynthaxfrom
thesynthax:macdo/syscalls
Open

mac_do(4): added traditional syscalls support to mac_do(4)#5
thesynthax wants to merge 2 commits intoOlCe2:oc-thesynthaxfrom
thesynthax:macdo/syscalls

Conversation

@thesynthax
Copy link

Added support for setuid(2), seteuid(2), setreuid(2), setresuid(2), setgid(2), setegid(2), setregid(2), setresgid(2), setgroups(2) in MAC/do

Signed-off-by: Kushagra Srivastava <kushagra1403@gmail.com>
Copy link
Owner

@OlCe2 OlCe2 left a comment

Choose a reason for hiding this comment

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

This overall looks fine. Please see inline comments.

Comment on lines 1948 to 1947
}
else
} else {
break;
}
Copy link
Owner

Choose a reason for hiding this comment

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

Gratuitous style change (please revert).

Comment on lines 1990 to 1989
}
else
} else {
break;
}
Copy link
Owner

Choose a reason for hiding this comment

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

Gratuitous style change (please revert).

}
}

/* 'gid' wasn't explicitly accepted. */
Copy link
Owner

Choose a reason for hiding this comment

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

Please keep this comment.

Comment on lines 2027 to 2030
const int supp_ngroups = old_cred->cr_ngroups;
const gid_t *supp_groups = (supp_ngroups > 0) ? &new_cred->cr_groups[1] : NULL;

return (rule_grant_setgroups(rule, old_cred, supp_ngroups, supp_groups));
Copy link
Owner

Choose a reason for hiding this comment

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

Suggested change
const int supp_ngroups = old_cred->cr_ngroups;
const gid_t *supp_groups = (supp_ngroups > 0) ? &new_cred->cr_groups[1] : NULL;
return (rule_grant_setgroups(rule, old_cred, supp_ngroups, supp_groups));
return (rule_grant_setgroups(rule, old_cred, old_cred->cr_ngroups, new_cred->cr_groups));

After recent changes in -CURRENT, cr_groups now only holds supplementary groups (and not the effective GID).

(gid_flags & MDF_SUPP_MASK) != 0;
id_nb_t rule_idx = 0;
int old_idx = 1, new_idx = 1;
int old_idx = 1, new_idx = 0;
Copy link
Owner

Choose a reason for hiding this comment

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

Should be:

Suggested change
int old_idx = 1, new_idx = 0;
int old_idx = 0, new_idx = 0;

after recent changes in -CURRENT. Will fix that in a separate commit (tomorrow probably).

/* Bail out fast if we aren't concerned. */
if (priv != PRIV_CRED_SETCRED)
return (EPERM);
switch (priv) {
Copy link
Owner

Choose a reason for hiding this comment

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

For all sub-blocks of this switch, please move the code in separate functions (e.g., priv_grant_setcred(), priv_grant_user(), priv_grant_group(), etc.).

This will remove indentation and separate the different recipes. Declarations in a switch are valid in all the switch block, so you had to add a pair of braces around each block. With this change, they won't be necessary.

Signed-off-by: Kushagra Srivastava <kushagra1403@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants