| Message ID | 20181004213355.14899-14-dsahern@kernel.org |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | David Miller |
| Headers | show |
| Series | rtnetlink: Add support for rigid checking of data in dump request | expand |
On Thu, Oct 04, 2018 at 02:33:48PM -0700, David Ahern wrote: > From: David Ahern <dsahern@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 <dsahern@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 >
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;