On Thu, Sep 26, 2024 at 6:11 AM Paul Moore <paul@xxxxxxxxxxxxxx> wrote:
>
> Streamline the code in selinux_nlmsg_lookup() to improve the code flow,
> readability, and remove the unnecessary local variables.
>
> Signed-off-by: Paul Moore <paul@xxxxxxxxxxxxxx>
> ---
> security/selinux/nlmsgtab.c | 90 ++++++++++++++++---------------------
> 1 file changed, 39 insertions(+), 51 deletions(-)
>
> diff --git a/security/selinux/nlmsgtab.c b/security/selinux/nlmsgtab.c
> index acc7d74b99d5..f68b4f493de6 100644
> --- a/security/selinux/nlmsgtab.c
> +++ b/security/selinux/nlmsgtab.c
> @@ -168,34 +168,12 @@ static int nlmsg_perm(u16 nlmsg_type, u32 *perm, const struct nlmsg_perm *tab, s
>
> int selinux_nlmsg_lookup(u16 sclass, u16 nlmsg_type, u32 *perm)
> {
> - int err = 0;
> -
> - if (selinux_policycap_netlink_xperm()) {
> - switch (sclass) {
> - case SECCLASS_NETLINK_ROUTE_SOCKET:
> - *perm = NETLINK_ROUTE_SOCKET__NLMSG;
> - break;
> - case SECCLASS_NETLINK_TCPDIAG_SOCKET:
> - *perm = NETLINK_TCPDIAG_SOCKET__NLMSG;
> - break;
> - case SECCLASS_NETLINK_XFRM_SOCKET:
> - *perm = NETLINK_XFRM_SOCKET__NLMSG;
> - break;
> - case SECCLASS_NETLINK_AUDIT_SOCKET:
> - *perm = NETLINK_AUDIT_SOCKET__NLMSG;
> - break;
> - /* While it is possible to add a similar permission to other
> - * netlink classes, note that the extended permission value is
> - * matched against the nlmsg_type field. Notably,
> - * SECCLASS_NETLINK_GENERIC_SOCKET uses dynamic values for this
> - * field, which means that it cannot be added as-is.
> - */
> - default:
> - err = -ENOENT;
> - break;
> - }
> - return err;
> - }
> + /* While it is possible to add a similar permission to other netlink
> + * classes, note that the extended permission value is matched against
> + * the nlmsg_type field. Notably, SECCLASS_NETLINK_GENERIC_SOCKET uses
> + * dynamic values for this field, which means that it cannot be added
> + * as-is.
> + */
>
> switch (sclass) {
> case SECCLASS_NETLINK_ROUTE_SOCKET:
> @@ -205,42 +183,52 @@ int selinux_nlmsg_lookup(u16 sclass, u16 nlmsg_type, u32 *perm)
> * before updating the BUILD_BUG_ON() macro!
> */
> BUILD_BUG_ON(RTM_MAX != (RTM_NEWTUNNEL + 3));
> - err = nlmsg_perm(nlmsg_type, perm, nlmsg_route_perms,
> - sizeof(nlmsg_route_perms));
> - break;
>
> + if (selinux_policycap_netlink_xperm()) {
> + *perm = NETLINK_ROUTE_SOCKET__NLMSG;
> + return 0;
> + }
> + return nlmsg_perm(nlmsg_type, perm, nlmsg_route_perms,
> + sizeof(nlmsg_route_perms));
> + break;
> case SECCLASS_NETLINK_TCPDIAG_SOCKET:
> - err = nlmsg_perm(nlmsg_type, perm, nlmsg_tcpdiag_perms,
> - sizeof(nlmsg_tcpdiag_perms));
> + if (selinux_policycap_netlink_xperm()) {
> + *perm = NETLINK_TCPDIAG_SOCKET__NLMSG;
> + return 0;
> + }
> + return nlmsg_perm(nlmsg_type, perm, nlmsg_tcpdiag_perms,
> + sizeof(nlmsg_tcpdiag_perms));
> break;
> -
> case SECCLASS_NETLINK_XFRM_SOCKET:
> /* If the BUILD_BUG_ON() below fails you must update the
> * structures at the top of this file with the new mappings
> * before updating the BUILD_BUG_ON() macro!
> */
> BUILD_BUG_ON(XFRM_MSG_MAX != XFRM_MSG_GETDEFAULT);
> - err = nlmsg_perm(nlmsg_type, perm, nlmsg_xfrm_perms,
> - sizeof(nlmsg_xfrm_perms));
> - break;
>
> - case SECCLASS_NETLINK_AUDIT_SOCKET:
> - if ((nlmsg_type >= AUDIT_FIRST_USER_MSG &&
> - nlmsg_type <= AUDIT_LAST_USER_MSG) ||
> - (nlmsg_type >= AUDIT_FIRST_USER_MSG2 &&
> - nlmsg_type <= AUDIT_LAST_USER_MSG2)) {
> - *perm = NETLINK_AUDIT_SOCKET__NLMSG_RELAY;
> - } else {
> - err = nlmsg_perm(nlmsg_type, perm, nlmsg_audit_perms,
> - sizeof(nlmsg_audit_perms));
> + if (selinux_policycap_netlink_xperm()) {
> + *perm = NETLINK_XFRM_SOCKET__NLMSG;
> + return 0;
> }
> + return nlmsg_perm(nlmsg_type, perm, nlmsg_xfrm_perms,
> + sizeof(nlmsg_xfrm_perms));
> break;
> -
> - /* No messaging from userspace, or class unknown/unhandled */
> - default:
> - err = -ENOENT;
> + case SECCLASS_NETLINK_AUDIT_SOCKET:
> + if (selinux_policycap_netlink_xperm()) {
> + *perm = NETLINK_XFRM_SOCKET__NLMSG;
Should it be NETLINK_AUDIT_SOCKET__NLMSG here?
