On Thu, Oct 04, 2018 at 02:33:48PM -0700, David Ahern wrote:
> From: David Ahern <dsah...@gmail.com>
>
> Add helper to check netlink message for route dumps. If the strict flag
> is set the dump request is expected to have an rtmsg struct as the header.
> All elements of the struct are expected to be 0 with the exception of
> rtm_flags (which is used by both ipv4 and ipv6 dumps) and no attributes
> can be appended. rtm_flags can only have RTM_F_CLONED and RTM_F_PREFIX
> set.
>
> Update inet_dump_fib, inet6_dump_fib, mpls_dump_routes, ipmr_rtm_dumproute,
> and ip6mr_rtm_dumproute to call this helper if strict data checking is
> enabled.
This also looks good to me apart from the \n after every:
+ type <err> = foo();
+
+ if (<err> < 0)
/* handle error */
I haven't seen this style in the code a lot or I haven't looked close
enough. It just seems visually cleaner to have this close together
without a newline:
+ type <err> = foo();
+ if (<err> < 0)
/* handle error */
>
> Signed-off-by: David Ahern <dsah...@gmail.com>
> ---
> include/net/ip_fib.h | 2 ++
> net/ipv4/fib_frontend.c | 43 +++++++++++++++++++++++++++++++++++++++++--
> net/ipv4/ipmr.c | 9 +++++++++
> net/ipv6/ip6_fib.c | 8 ++++++++
> net/ipv6/ip6mr.c | 9 +++++++++
> net/mpls/af_mpls.c | 8 ++++++++
> 6 files changed, 77 insertions(+), 2 deletions(-)
>
> diff --git a/include/net/ip_fib.h b/include/net/ip_fib.h
> index f7c109e37298..9846b79c9ee1 100644
> --- a/include/net/ip_fib.h
> +++ b/include/net/ip_fib.h
> @@ -452,4 +452,6 @@ static inline void fib_proc_exit(struct net *net)
>
> u32 ip_mtu_from_fib_result(struct fib_result *res, __be32 daddr);
>
> +int ip_valid_fib_dump_req(const struct nlmsghdr *nlh,
> + struct netlink_ext_ack *extack);
> #endif /* _NET_FIB_H */
> diff --git a/net/ipv4/fib_frontend.c b/net/ipv4/fib_frontend.c
> index 30e2bcc3ef2a..1583ec0a5154 100644
> --- a/net/ipv4/fib_frontend.c
> +++ b/net/ipv4/fib_frontend.c
> @@ -802,8 +802,41 @@ static int inet_rtm_newroute(struct sk_buff *skb, struct
> nlmsghdr *nlh,
> return err;
> }
>
> +int ip_valid_fib_dump_req(const struct nlmsghdr *nlh,
> + struct netlink_ext_ack *extack)
> +{
> + struct rtmsg *rtm;
> +
> + if (nlh->nlmsg_len < nlmsg_msg_size(sizeof(*rtm))) {
> + NL_SET_ERR_MSG(extack, "Invalid header");
> + return -EINVAL;
> + }
> +
> + rtm = nlmsg_data(nlh);
> + if (rtm->rtm_dst_len || rtm->rtm_src_len || rtm->rtm_tos ||
> + rtm->rtm_table || rtm->rtm_protocol || rtm->rtm_scope ||
> + rtm->rtm_type) {
> + NL_SET_ERR_MSG(extack,
> + "Invalid values in header for dump request");
> + return -EINVAL;
> + }
> +
> + if (rtm->rtm_flags & ~(RTM_F_CLONED | RTM_F_PREFIX)) {
> + NL_SET_ERR_MSG(extack, "Invalid flags for dump request");
> + return -EINVAL;
> + }
> + if (nlh->nlmsg_len != nlmsg_msg_size(sizeof(*rtm))) {
> + NL_SET_ERR_MSG(extack, "Invalid data after header");
> + return -EINVAL;
> + }
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(ip_valid_fib_dump_req);
> +
> static int inet_dump_fib(struct sk_buff *skb, struct netlink_callback *cb)
> {
> + const struct nlmsghdr *nlh = cb->nlh;
> struct net *net = sock_net(skb->sk);
> unsigned int h, s_h;
> unsigned int e = 0, s_e;
> @@ -811,8 +844,14 @@ static int inet_dump_fib(struct sk_buff *skb, struct
> netlink_callback *cb)
> struct hlist_head *head;
> int dumped = 0, err;
>
> - if (nlmsg_len(cb->nlh) >= sizeof(struct rtmsg) &&
> - ((struct rtmsg *) nlmsg_data(cb->nlh))->rtm_flags & RTM_F_CLONED)
> + if (cb->strict_check) {
> + err = ip_valid_fib_dump_req(nlh, cb->extack);
> + if (err)
> + return err;
> + }
> +
> + if (nlmsg_len(nlh) >= sizeof(struct rtmsg) &&
> + ((struct rtmsg *)nlmsg_data(nlh))->rtm_flags & RTM_F_CLONED)
> return skb->len;
>
> s_h = cb->args[0];
> diff --git a/net/ipv4/ipmr.c b/net/ipv4/ipmr.c
> index e6c48e08d53d..2a7963beecfb 100644
> --- a/net/ipv4/ipmr.c
> +++ b/net/ipv4/ipmr.c
> @@ -2527,6 +2527,15 @@ static int ipmr_rtm_getroute(struct sk_buff *in_skb,
> struct nlmsghdr *nlh,
>
> static int ipmr_rtm_dumproute(struct sk_buff *skb, struct netlink_callback
> *cb)
> {
> + const struct nlmsghdr *nlh = cb->nlh;
> +
> + if (cb->strict_check) {
> + int err = ip_valid_fib_dump_req(nlh, cb->extack);
> +
> + if (err)
> + return err;
> + }
> +
> return mr_rtm_dumproute(skb, cb, ipmr_mr_table_iter,
> _ipmr_fill_mroute, &mfc_unres_lock);
> }
> diff --git a/net/ipv6/ip6_fib.c b/net/ipv6/ip6_fib.c
> index 5516f55e214b..123786684476 100644
> --- a/net/ipv6/ip6_fib.c
> +++ b/net/ipv6/ip6_fib.c
> @@ -568,6 +568,7 @@ static int fib6_dump_table(struct fib6_table *table,
> struct sk_buff *skb,
>
> static int inet6_dump_fib(struct sk_buff *skb, struct netlink_callback *cb)
> {
> + const struct nlmsghdr *nlh = cb->nlh;
> struct net *net = sock_net(skb->sk);
> unsigned int h, s_h;
> unsigned int e = 0, s_e;
> @@ -577,6 +578,13 @@ static int inet6_dump_fib(struct sk_buff *skb, struct
> netlink_callback *cb)
> struct hlist_head *head;
> int res = 0;
>
> + if (cb->strict_check) {
> + int err = ip_valid_fib_dump_req(nlh, cb->extack);
> +
> + if (err)
> + return err;
> + }
> +
> s_h = cb->args[0];
> s_e = cb->args[1];
>
> diff --git a/net/ipv6/ip6mr.c b/net/ipv6/ip6mr.c
> index 6f07b8380425..8a94500c5532 100644
> --- a/net/ipv6/ip6mr.c
> +++ b/net/ipv6/ip6mr.c
> @@ -2457,6 +2457,15 @@ static void mrt6msg_netlink_event(struct mr_table
> *mrt, struct sk_buff *pkt)
>
> static int ip6mr_rtm_dumproute(struct sk_buff *skb, struct netlink_callback
> *cb)
> {
> + const struct nlmsghdr *nlh = cb->nlh;
> +
> + if (cb->strict_check) {
> + int err = ip_valid_fib_dump_req(nlh, cb->extack);
> +
> + if (err)
> + return err;
> + }
> +
> return mr_rtm_dumproute(skb, cb, ip6mr_mr_table_iter,
> _ip6mr_fill_mroute, &mfc_unres_lock);
> }
> diff --git a/net/mpls/af_mpls.c b/net/mpls/af_mpls.c
> index 55a30ee3d820..3e33934751b4 100644
> --- a/net/mpls/af_mpls.c
> +++ b/net/mpls/af_mpls.c
> @@ -2017,6 +2017,7 @@ static int mpls_dump_route(struct sk_buff *skb, u32
> portid, u32 seq, int event,
>
> static int mpls_dump_routes(struct sk_buff *skb, struct netlink_callback *cb)
> {
> + const struct nlmsghdr *nlh = cb->nlh;
> struct net *net = sock_net(skb->sk);
> struct mpls_route __rcu **platform_label;
> size_t platform_labels;
> @@ -2024,6 +2025,13 @@ static int mpls_dump_routes(struct sk_buff *skb,
> struct netlink_callback *cb)
>
> ASSERT_RTNL();
>
> + if (cb->strict_check) {
> + int err = ip_valid_fib_dump_req(nlh, cb->extack);
> +
> + if (err)
> + return err;
> + }
> +
> index = cb->args[0];
> if (index < MPLS_LABEL_FIRST_UNRESERVED)
> index = MPLS_LABEL_FIRST_UNRESERVED;
> --
> 2.11.0
>
