Re: [PATCH net-next] rtnetlink: add RTNH_REJECT_MASK
flat view
From: Ido Schimmel <hidden>
Date: 2021-11-28 14:03:33
On Fri, Nov 26, 2021 at 04:43:11PM +0300, Alexander Mikhalitsyn wrote:
quoted hunk ↗ jump to hunk
diff --git a/include/uapi/linux/rtnetlink.h b/include/uapi/linux/rtnetlink.h index 5888492a5257..9c065e2fdef9 100644 --- a/include/uapi/linux/rtnetlink.h +++ b/include/uapi/linux/rtnetlink.h@@ -417,6 +417,9 @@ struct rtnexthop { #define RTNH_COMPARE_MASK (RTNH_F_DEAD | RTNH_F_LINKDOWN | \ RTNH_F_OFFLOAD | RTNH_F_TRAP) +/* these flags can't be set by the userspace */ +#define RTNH_REJECT_MASK (RTNH_F_DEAD | RTNH_F_LINKDOWN) + /* Macros to handle hexthops */ #define RTNH_ALIGNTO 4diff --git a/net/ipv4/fib_semantics.c b/net/ipv4/fib_semantics.c index 4c0c33e4710d..805f5e05b56d 100644 --- a/net/ipv4/fib_semantics.c +++ b/net/ipv4/fib_semantics.c@@ -685,7 +685,7 @@ static int fib_get_nhs(struct fib_info *fi, struct rtnexthop *rtnh, return -EINVAL; } - if (rtnh->rtnh_flags & (RTNH_F_DEAD | RTNH_F_LINKDOWN)) { + if (rtnh->rtnh_flags & RTNH_REJECT_MASK) { NL_SET_ERR_MSG(extack, "Invalid flags for nexthop - can not contain DEAD or LINKDOWN"); return -EINVAL;@@ -1363,7 +1363,7 @@ struct fib_info *fib_create_info(struct fib_config *cfg, goto err_inval; } - if (cfg->fc_flags & (RTNH_F_DEAD | RTNH_F_LINKDOWN)) { + if (cfg->fc_flags & RTNH_REJECT_MASK) { NL_SET_ERR_MSG(extack, "Invalid rtm_flags - can not contain DEAD or LINKDOWN");
Instead of a deny list as in the legacy nexthop code, the new nexthop code has an allow list (from rtm_to_nh_config()):
if (nhm->nh_flags & ~NEXTHOP_VALID_USER_FLAGS) {
NL_SET_ERR_MSG(extack, "Invalid nexthop flags in ancillary header");
goto out;
}
Where:
#define NEXTHOP_VALID_USER_FLAGS RTNH_F_ONLINK
So while the legacy nexthop code allows setting flags such as RTNH_F_OFFLOAD, the new nexthop code denies them. I don't have a use case for setting these flags from user space so I don't care if we allow or deny them, but I believe the legacy and new nexthop code should be consistent. WDYT? Should we allow these flags in the new nexthop code as well or keep denying them?
goto err_inval; -- 2.31.1