| Message ID | fb2bbc9568a7d7d21a00b791a2d4f488cfcd8a50.1560561432.git.sbrivio@redhat.com |
|---|---|
| State | Changes Requested |
| Delegated to: | David Miller |
| Headers | show |
| Series | Fix listing (IPv4, IPv6) and flushing (IPv6) of cached route exceptions | expand |
On 6/14/19 7:32 PM, Stefano Brivio wrote: > ip_valid_fib_dump_req() does two things: performs strict checking on > netlink attributes for dump requests, and sets a dump filter if netlink > attributes require it. > > We might want to just set a filter, without performing strict validation. > > Rename it to ip_filter_fib_dump_req(), and add a 'strict' boolean > argument that must be set if strict validation is requested. > > This patch doesn't introduce any functional changes. > > Signed-off-by: Stefano Brivio <sbrivio@redhat.com> > --- > v4: New patch > Can you explain why this patch is needed? The existing function requires strict mode and is needed to enable any of the kernel side filtering beyond the RTM_F_CLONED setting in rtm_flags.
On Fri, 14 Jun 2019 20:54:49 -0600 David Ahern <dsahern@gmail.com> wrote: > On 6/14/19 7:32 PM, Stefano Brivio wrote: > > ip_valid_fib_dump_req() does two things: performs strict checking on > > netlink attributes for dump requests, and sets a dump filter if netlink > > attributes require it. > > > > We might want to just set a filter, without performing strict validation. > > > > Rename it to ip_filter_fib_dump_req(), and add a 'strict' boolean > > argument that must be set if strict validation is requested. > > > > This patch doesn't introduce any functional changes. > > > > Signed-off-by: Stefano Brivio <sbrivio@redhat.com> > > --- > > v4: New patch > > > > Can you explain why this patch is needed? The existing function requires > strict mode and is needed to enable any of the kernel side filtering > beyond the RTM_F_CLONED setting in rtm_flags. It's mostly to have proper NLM_F_MATCH support. Let's pick an iproute2 version without strict checking support (< 5.0), that sets NLM_F_MATCH though. Then we need this check: if (nlh->nlmsg_len < nlmsg_msg_size(sizeof(*rtm))) and to set filter parameters not just based on flags (i.e. RTM_F_CLONED), but also on table, protocol, etc. For example one might want to: 'ip route list cache table main', and this is then taken into account in fn_trie_dump_leaf() and rt6_dump_route(). Reusing this function avoids a nice amount of duplicated code and allows to have an almost common path with strict checking.
On 6/14/19 9:13 PM, Stefano Brivio wrote: > On Fri, 14 Jun 2019 20:54:49 -0600 > David Ahern <dsahern@gmail.com> wrote: > >> On 6/14/19 7:32 PM, Stefano Brivio wrote: >>> ip_valid_fib_dump_req() does two things: performs strict checking on >>> netlink attributes for dump requests, and sets a dump filter if netlink >>> attributes require it. >>> >>> We might want to just set a filter, without performing strict validation. >>> >>> Rename it to ip_filter_fib_dump_req(), and add a 'strict' boolean >>> argument that must be set if strict validation is requested. >>> >>> This patch doesn't introduce any functional changes. >>> >>> Signed-off-by: Stefano Brivio <sbrivio@redhat.com> >>> --- >>> v4: New patch >>> >> >> Can you explain why this patch is needed? The existing function requires >> strict mode and is needed to enable any of the kernel side filtering >> beyond the RTM_F_CLONED setting in rtm_flags. > > It's mostly to have proper NLM_F_MATCH support. Let's pick an iproute2 > version without strict checking support (< 5.0), that sets NLM_F_MATCH > though. Then we need this check: > > if (nlh->nlmsg_len < nlmsg_msg_size(sizeof(*rtm))) but that check existed long before any of the strict checking and kernel side filtering was added. > > and to set filter parameters not just based on flags (i.e. RTM_F_CLONED), > but also on table, protocol, etc. and to do that you *must* have strict checking. There is no way to trust userspace without that strict flag set because iproute2 for the longest time sent the wrong header for almost all dump requests. > > For example one might want to: 'ip route list cache table main', and this > is then taken into account in fn_trie_dump_leaf() and rt6_dump_route(). > > Reusing this function avoids a nice amount of duplicated code and allows > to have an almost common path with strict checking. >
On Fri, 14 Jun 2019 21:16:54 -0600 David Ahern <dsahern@gmail.com> wrote: > On 6/14/19 9:13 PM, Stefano Brivio wrote: > > On Fri, 14 Jun 2019 20:54:49 -0600 > > David Ahern <dsahern@gmail.com> wrote: > > > >> On 6/14/19 7:32 PM, Stefano Brivio wrote: > >>> ip_valid_fib_dump_req() does two things: performs strict checking on > >>> netlink attributes for dump requests, and sets a dump filter if netlink > >>> attributes require it. > >>> > >>> We might want to just set a filter, without performing strict validation. > >>> > >>> Rename it to ip_filter_fib_dump_req(), and add a 'strict' boolean > >>> argument that must be set if strict validation is requested. > >>> > >>> This patch doesn't introduce any functional changes. > >>> > >>> Signed-off-by: Stefano Brivio <sbrivio@redhat.com> > >>> --- > >>> v4: New patch > >>> > >> > >> Can you explain why this patch is needed? The existing function requires > >> strict mode and is needed to enable any of the kernel side filtering > >> beyond the RTM_F_CLONED setting in rtm_flags. > > > > It's mostly to have proper NLM_F_MATCH support. Let's pick an iproute2 > > version without strict checking support (< 5.0), that sets NLM_F_MATCH > > though. Then we need this check: > > > > if (nlh->nlmsg_len < nlmsg_msg_size(sizeof(*rtm))) > > but that check existed long before any of the strict checking and kernel > side filtering was added. Indeed. And now I'm recycling it, even if strict checking is not requested. > > and to set filter parameters not just based on flags (i.e. RTM_F_CLONED), > > but also on table, protocol, etc. > > and to do that you *must* have strict checking. There is no way to trust > userspace without that strict flag set because iproute2 for the longest > time sent the wrong header for almost all dump requests. So you're implying that: - we shouldn't support NLM_F_MATCH - we should keep this broken for iproute2 < 5.0.0? I guess this might be acceptable, but please state it clearly. By the way, if really needed, we can do strict checking even if not requested. But this might add more and more userspace breakage, I guess.
On Sat, 15 Jun 2019 05:27:05 +0200 Stefano Brivio <sbrivio@redhat.com> wrote: > On Fri, 14 Jun 2019 21:16:54 -0600 > David Ahern <dsahern@gmail.com> wrote: > > > On 6/14/19 9:13 PM, Stefano Brivio wrote: > > > On Fri, 14 Jun 2019 20:54:49 -0600 > > > David Ahern <dsahern@gmail.com> wrote: > > > > > >> On 6/14/19 7:32 PM, Stefano Brivio wrote: > > >>> ip_valid_fib_dump_req() does two things: performs strict checking on > > >>> netlink attributes for dump requests, and sets a dump filter if netlink > > >>> attributes require it. > > >>> > > >>> We might want to just set a filter, without performing strict validation. > > >>> > > >>> Rename it to ip_filter_fib_dump_req(), and add a 'strict' boolean > > >>> argument that must be set if strict validation is requested. > > >>> > > >>> This patch doesn't introduce any functional changes. > > >>> > > >>> Signed-off-by: Stefano Brivio <sbrivio@redhat.com> > > >>> --- > > >>> v4: New patch > > >>> > > >> > > >> Can you explain why this patch is needed? The existing function requires > > >> strict mode and is needed to enable any of the kernel side filtering > > >> beyond the RTM_F_CLONED setting in rtm_flags. > > > > > > It's mostly to have proper NLM_F_MATCH support. Let's pick an iproute2 > > > version without strict checking support (< 5.0), that sets NLM_F_MATCH > > > though. Then we need this check: > > > > > > if (nlh->nlmsg_len < nlmsg_msg_size(sizeof(*rtm))) > > > > but that check existed long before any of the strict checking and kernel > > side filtering was added. > > Indeed. And now I'm recycling it, even if strict checking is not > requested. > > > > and to set filter parameters not just based on flags (i.e. RTM_F_CLONED), > > > but also on table, protocol, etc. > > > > and to do that you *must* have strict checking. There is no way to trust > > userspace without that strict flag set because iproute2 for the longest > > time sent the wrong header for almost all dump requests. > > So you're implying that: > > - we shouldn't support NLM_F_MATCH > > - we should keep this broken for iproute2 < 5.0.0? > > I guess this might be acceptable, but please state it clearly. > > By the way, if really needed, we can do strict checking even if not > requested. But this might add more and more userspace breakage, I guess. Maybe I have a simpler alternative, that doesn't allow filters without strict checking (your concern above) and fixes the issue for most iproute2 versions (except for 'ip -6 route cache flush' from 5.0.0 to current, unpatched version). I would also like to avoid introducing this bug: - 'ip route list cache table main' currently returns nothing (bug) - 'ip route list cache table main' with v1-v3 would return all cached routes (new bug) and retain this feature from v4: - if neither NLM_F_MATCH nor other filters are set, dump all cached and uncached routes. There's no way to get cached and uncached ones with a single request, otherwise. This would also fit RFC 3549. We could do this: - strict checking enabled (iproute2 >= 5.0.0): - in inet{,6}_dump_fib(): if NLM_F_MATCH is set, set filter->filter_set in any case - in fn_trie_dump_leaf() and rt6_dump_route(): use filter->filter_set to decide if we want to filter depending on RTM_F_CLONED being set/unset. If other filters (rt_type, dev, protocol) are not set, they are still wildcards (existing implementation) - no strict checking (iproute2 < 5.0.0): - we can't filter consistently, so apply no filters at all: dump all the routes (filter->filter_set not set), cached and uncached. That means more netlink messages, but no spam as iproute2 filters them anyway, and list/flush cache commands work again. I would drop 1/8, turn 2/8 and 6/8 into a straightforward: if (cb->strict_check) { err = ip_valid_fib_dump_req(net, nlh, &filter, cb); if (err < 0) return err; + if (nlh->nlmsg_flags & NLM_F_MATCH) + filter.filter_set = 1; } else if (nlmsg_len(nlh) >= sizeof(struct rtmsg)) { struct rtmsg *rtm = nlmsg_data(nlh); and other patches remain the same. What do you think?
On 6/14/19 9:27 PM, Stefano Brivio wrote: >>>> Can you explain why this patch is needed? The existing function requires >>>> strict mode and is needed to enable any of the kernel side filtering >>>> beyond the RTM_F_CLONED setting in rtm_flags. >>> >>> It's mostly to have proper NLM_F_MATCH support. Let's pick an iproute2 >>> version without strict checking support (< 5.0), that sets NLM_F_MATCH >>> though. Then we need this check: >>> >>> if (nlh->nlmsg_len < nlmsg_msg_size(sizeof(*rtm))) >> >> but that check existed long before any of the strict checking and kernel >> side filtering was added. > > Indeed. And now I'm recycling it, even if strict checking is not > requested. > >>> and to set filter parameters not just based on flags (i.e. RTM_F_CLONED), >>> but also on table, protocol, etc. >> >> and to do that you *must* have strict checking. There is no way to trust >> userspace without that strict flag set because iproute2 for the longest >> time sent the wrong header for almost all dump requests. > > So you're implying that: > > - we shouldn't support NLM_F_MATCH > > - we should keep this broken for iproute2 < 5.0.0? > > I guess this might be acceptable, but please state it clearly. > > By the way, if really needed, we can do strict checking even if not > requested. But this might add more and more userspace breakage, I guess. > Prior to 5.0 and strict checking, iproute2 was sending ifinfomsg as the header struct - which is wrong for routes. ifi_flags just happens to have the same offset as rtm_flags so the check for RTM_F_CLONED is ok, but nothing else sent in the get request (e.g., potentially appended attributes) can be trusted, so the !strict path you are adding with nlmsg_parse_deprecated is wrong. The kernel side filter argument can be used and treating RTM_F_CLONED as a filter is ok, but not the new parsing code.
On 6/16/19 2:04 PM, Stefano Brivio wrote: > We could do this: > > - strict checking enabled (iproute2 >= 5.0.0): > - in inet{,6}_dump_fib(): if NLM_F_MATCH is set, set > filter->filter_set in any case > > - in fn_trie_dump_leaf() and rt6_dump_route(): use filter->filter_set > to decide if we want to filter depending on RTM_F_CLONED being > set/unset. If other filters (rt_type, dev, protocol) are not set, > they are still wildcards (existing implementation) > > - no strict checking (iproute2 < 5.0.0): > - we can't filter consistently, so apply no filters at all: dump all > the routes (filter->filter_set not set), cached and uncached. That > means more netlink messages, but no spam as iproute2 filters them > anyway, and list/flush cache commands work again. > > I would drop 1/8, turn 2/8 and 6/8 into a straightforward: > > if (cb->strict_check) { > err = ip_valid_fib_dump_req(net, nlh, &filter, cb); > if (err < 0) > return err; > + if (nlh->nlmsg_flags & NLM_F_MATCH) > + filter.filter_set = 1; > } else if (nlmsg_len(nlh) >= sizeof(struct rtmsg)) { > struct rtmsg *rtm = nlmsg_data(nlh); > > and other patches remain the same. > > What do you think? > With strict checking (5.0 and forward): - RTM_F_CLONED NOT set means dump only FIB entries - RTM_F_CLONED set means dump only exceptions Without strict checking (old iproute2 on any kernel): - dump all, userspace has to sort Kernel side this can be handled with new field, dump_exceptions, in the filter that defaults to true and then is reset in the strict path if the flag is not set.
On Mon, 17 Jun 2019 07:38:54 -0600 David Ahern <dsahern@gmail.com> wrote: > On 6/16/19 2:04 PM, Stefano Brivio wrote: > > We could do this: > > > > - strict checking enabled (iproute2 >= 5.0.0): > > - in inet{,6}_dump_fib(): if NLM_F_MATCH is set, set > > filter->filter_set in any case > > > > - in fn_trie_dump_leaf() and rt6_dump_route(): use filter->filter_set > > to decide if we want to filter depending on RTM_F_CLONED being > > set/unset. If other filters (rt_type, dev, protocol) are not set, > > they are still wildcards (existing implementation) > > > > - no strict checking (iproute2 < 5.0.0): > > - we can't filter consistently, so apply no filters at all: dump all > > the routes (filter->filter_set not set), cached and uncached. That > > means more netlink messages, but no spam as iproute2 filters them > > anyway, and list/flush cache commands work again. > > > > I would drop 1/8, turn 2/8 and 6/8 into a straightforward: > > > > if (cb->strict_check) { > > err = ip_valid_fib_dump_req(net, nlh, &filter, cb); > > if (err < 0) > > return err; > > + if (nlh->nlmsg_flags & NLM_F_MATCH) > > + filter.filter_set = 1; > > } else if (nlmsg_len(nlh) >= sizeof(struct rtmsg)) { > > struct rtmsg *rtm = nlmsg_data(nlh); > > > > and other patches remain the same. > > > > What do you think? > > > > With strict checking (5.0 and forward): > - RTM_F_CLONED NOT set means dump only FIB entries > - RTM_F_CLONED set means dump only exceptions Okay. Should we really ignore the RFC and NLM_F_MATCH though? If we add field(s) to the filter, it comes almost for free, something like: if (nlh->nlmsg_flags & NLM_F_MATCH) filter->dump_exceptions = rtm->rtm_flags & RTM_F_CLONED; instead of: filter->dump_exceptions = rtm->rtm_flags & RTM_F_CLONED; > Without strict checking (old iproute2 on any kernel): > - dump all, userspace has to sort > > Kernel side this can be handled with new field, dump_exceptions, in the > filter that defaults to true and then is reset in the strict path if the > flag is not set. I guess we need to add two fields, we'll need a 'dump_routes' too. Otherwise, the dump functions can't distinguish between the three cases ('no strict checking', 'strict checking and RTM_F_CLONED', 'strict checking and no RTM_F_CLONED'). How would you do this with a single additional field?
On 6/17/19 8:13 AM, Stefano Brivio wrote: >> >> With strict checking (5.0 and forward): >> - RTM_F_CLONED NOT set means dump only FIB entries >> - RTM_F_CLONED set means dump only exceptions > > Okay. Should we really ignore the RFC and NLM_F_MATCH though? If we add > field(s) to the filter, it comes almost for free, something like: > > if (nlh->nlmsg_flags & NLM_F_MATCH) > filter->dump_exceptions = rtm->rtm_flags & RTM_F_CLONED; > > instead of: > > filter->dump_exceptions = rtm->rtm_flags & RTM_F_CLONED; This is where you keep losing me. iproute2 has always set NLM_F_MATCH on dump requests, so that flag can not be used as a discriminator here. > >> Without strict checking (old iproute2 on any kernel): >> - dump all, userspace has to sort >> >> Kernel side this can be handled with new field, dump_exceptions, in the >> filter that defaults to true and then is reset in the strict path if the >> flag is not set. > > I guess we need to add two fields, we'll need a 'dump_routes' too. > > Otherwise, the dump functions can't distinguish between the three cases > ('no strict checking', 'strict checking and RTM_F_CLONED', 'strict > checking and no RTM_F_CLONED'). How would you do this with a single > additional field? > sure, separate fields are needed for the pre-strict mode use case. So, I take it we are converging on this: 1. non-strict mode, dump both (FIB entries and exceptions). Userspace has to filter. This is the legacy behavior you are trying to restore. 2. strict mode: a. dump only FIB entries if RTM_F_CLONED is not set b. dump only exception entries if RTM_F_CLONED is set Agreed? Martin, others, ok with this?
On Mon, 17 Jun 2019 11:06:51 -0600 David Ahern <dsahern@gmail.com> wrote: > On 6/17/19 8:13 AM, Stefano Brivio wrote: > >> > >> With strict checking (5.0 and forward): > >> - RTM_F_CLONED NOT set means dump only FIB entries > >> - RTM_F_CLONED set means dump only exceptions > > > > Okay. Should we really ignore the RFC and NLM_F_MATCH though? If we add > > field(s) to the filter, it comes almost for free, something like: > > > > if (nlh->nlmsg_flags & NLM_F_MATCH) > > filter->dump_exceptions = rtm->rtm_flags & RTM_F_CLONED; > > > > instead of: > > > > filter->dump_exceptions = rtm->rtm_flags & RTM_F_CLONED; > > This is where you keep losing me. iproute2 has always set NLM_F_MATCH on > dump requests, so that flag can not be used as a discriminator here. iproute2 yes, but some other users (I'm not aware of any so I have no examples) might *very* vaguely follow the RFC and expect consistent results. That was my only point here. Most likely just a theoretical one. > > > >> Without strict checking (old iproute2 on any kernel): > >> - dump all, userspace has to sort > >> > >> Kernel side this can be handled with new field, dump_exceptions, in the > >> filter that defaults to true and then is reset in the strict path if the > >> flag is not set. > > > > I guess we need to add two fields, we'll need a 'dump_routes' too. > > > > Otherwise, the dump functions can't distinguish between the three cases > > ('no strict checking', 'strict checking and RTM_F_CLONED', 'strict > > checking and no RTM_F_CLONED'). How would you do this with a single > > additional field? > > > > sure, separate fields are needed for the pre-strict mode use case. Well, they are needed, in general. They both start as true, non-strict mode doesn't clear them, strict mode clears one. That's how I would do it. > So, I take it we are converging on this: > > 1. non-strict mode, dump both (FIB entries and exceptions). Userspace > has to filter. This is the legacy behavior you are trying to restore. > > 2. strict mode: > a. dump only FIB entries if RTM_F_CLONED is not set > b. dump only exception entries if RTM_F_CLONED is set > > Agreed? Agreed in general, maybe let me know what you think about the NLM_F_MATCH point above though.
diff --git a/include/net/ip_fib.h b/include/net/ip_fib.h index bbeff32fb6cb..76094a0b97cf 100644 --- a/include/net/ip_fib.h +++ b/include/net/ip_fib.h @@ -493,9 +493,9 @@ 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(struct net *net, const struct nlmsghdr *nlh, - struct fib_dump_filter *filter, - struct netlink_callback *cb); +int ip_filter_fib_dump_req(struct net *net, const struct nlmsghdr *nlh, + struct fib_dump_filter *filter, + struct netlink_callback *cb, bool strict); int fib_nexthop_info(struct sk_buff *skb, const struct fib_nh_common *nh, unsigned char *flags, bool skip_oif); diff --git a/net/ipv4/fib_frontend.c b/net/ipv4/fib_frontend.c index e54c2bcbb465..873fc5c4721c 100644 --- a/net/ipv4/fib_frontend.c +++ b/net/ipv4/fib_frontend.c @@ -859,9 +859,9 @@ static int inet_rtm_newroute(struct sk_buff *skb, struct nlmsghdr *nlh, return err; } -int ip_valid_fib_dump_req(struct net *net, const struct nlmsghdr *nlh, - struct fib_dump_filter *filter, - struct netlink_callback *cb) +int ip_filter_fib_dump_req(struct net *net, const struct nlmsghdr *nlh, + struct fib_dump_filter *filter, + struct netlink_callback *cb, bool strict) { struct netlink_ext_ack *extack = cb->extack; struct nlattr *tb[RTA_MAX + 1]; @@ -876,12 +876,12 @@ int ip_valid_fib_dump_req(struct net *net, const struct nlmsghdr *nlh, } rtm = nlmsg_data(nlh); - if (rtm->rtm_dst_len || rtm->rtm_src_len || rtm->rtm_tos || - rtm->rtm_scope) { + if (strict && (rtm->rtm_dst_len || rtm->rtm_src_len || rtm->rtm_tos || + rtm->rtm_scope)) { NL_SET_ERR_MSG(extack, "Invalid values in header for FIB dump request"); return -EINVAL; } - if (rtm->rtm_flags & ~(RTM_F_CLONED | RTM_F_PREFIX)) { + if (strict && rtm->rtm_flags & ~(RTM_F_CLONED | RTM_F_PREFIX)) { NL_SET_ERR_MSG(extack, "Invalid flags for FIB dump request"); return -EINVAL; } @@ -892,10 +892,18 @@ int ip_valid_fib_dump_req(struct net *net, const struct nlmsghdr *nlh, filter->rt_type = rtm->rtm_type; filter->table_id = rtm->rtm_table; - err = nlmsg_parse_deprecated_strict(nlh, sizeof(*rtm), tb, RTA_MAX, - rtm_ipv4_policy, extack); - if (err < 0) - return err; + if (strict) { + err = nlmsg_parse_deprecated_strict(nlh, sizeof(*rtm), tb, + RTA_MAX, rtm_ipv4_policy, + extack); + if (err < 0) + return err; + } else { + err = nlmsg_parse_deprecated(nlh, sizeof(*rtm), tb, RTA_MAX, + rtm_ipv4_policy, extack); + if (err < 0) + return err; + } for (i = 0; i <= RTA_MAX; ++i) { int ifindex; @@ -914,6 +922,8 @@ int ip_valid_fib_dump_req(struct net *net, const struct nlmsghdr *nlh, return -ENODEV; break; default: + if (!strict) + break; NL_SET_ERR_MSG(extack, "Unsupported attribute in dump request"); return -EINVAL; } @@ -927,7 +937,7 @@ int ip_valid_fib_dump_req(struct net *net, const struct nlmsghdr *nlh, return 0; } -EXPORT_SYMBOL_GPL(ip_valid_fib_dump_req); +EXPORT_SYMBOL_GPL(ip_filter_fib_dump_req); static int inet_dump_fib(struct sk_buff *skb, struct netlink_callback *cb) { @@ -941,7 +951,7 @@ static int inet_dump_fib(struct sk_buff *skb, struct netlink_callback *cb) int dumped = 0, err; if (cb->strict_check) { - err = ip_valid_fib_dump_req(net, nlh, &filter, cb); + err = ip_filter_fib_dump_req(net, nlh, &filter, cb, true); if (err < 0) return err; } else if (nlmsg_len(nlh) >= sizeof(struct rtmsg)) { diff --git a/net/ipv4/ipmr.c b/net/ipv4/ipmr.c index c07bc82cbbe9..1e089acc9479 100644 --- a/net/ipv4/ipmr.c +++ b/net/ipv4/ipmr.c @@ -2597,8 +2597,8 @@ static int ipmr_rtm_dumproute(struct sk_buff *skb, struct netlink_callback *cb) int err; if (cb->strict_check) { - err = ip_valid_fib_dump_req(sock_net(skb->sk), cb->nlh, - &filter, cb); + err = ip_filter_fib_dump_req(sock_net(skb->sk), cb->nlh, + &filter, cb, true); if (err < 0) return err; } diff --git a/net/ipv6/ip6_fib.c b/net/ipv6/ip6_fib.c index 9180c8b6f764..b21a9ec02891 100644 --- a/net/ipv6/ip6_fib.c +++ b/net/ipv6/ip6_fib.c @@ -571,7 +571,7 @@ static int inet6_dump_fib(struct sk_buff *skb, struct netlink_callback *cb) if (cb->strict_check) { int err; - err = ip_valid_fib_dump_req(net, nlh, &arg.filter, cb); + err = ip_filter_fib_dump_req(net, nlh, &arg.filter, cb, true); if (err < 0) return err; } else if (nlmsg_len(nlh) >= sizeof(struct rtmsg)) { diff --git a/net/ipv6/ip6mr.c b/net/ipv6/ip6mr.c index e80d36c5073d..4960c3fe8e83 100644 --- a/net/ipv6/ip6mr.c +++ b/net/ipv6/ip6mr.c @@ -2487,8 +2487,8 @@ static int ip6mr_rtm_dumproute(struct sk_buff *skb, struct netlink_callback *cb) int err; if (cb->strict_check) { - err = ip_valid_fib_dump_req(sock_net(skb->sk), nlh, - &filter, cb); + err = ip_filter_fib_dump_req(sock_net(skb->sk), nlh, &filter, + cb, true); if (err < 0) return err; } diff --git a/net/mpls/af_mpls.c b/net/mpls/af_mpls.c index 198ec4fe4148..f54d2f5834f8 100644 --- a/net/mpls/af_mpls.c +++ b/net/mpls/af_mpls.c @@ -2078,7 +2078,7 @@ static int mpls_valid_fib_dump_req(struct net *net, const struct nlmsghdr *nlh, struct fib_dump_filter *filter, struct netlink_callback *cb) { - return ip_valid_fib_dump_req(net, nlh, filter, cb); + return ip_filter_fib_dump_req(net, nlh, filter, cb, true); } #else static int mpls_valid_fib_dump_req(struct net *net, const struct nlmsghdr *nlh,
ip_valid_fib_dump_req() does two things: performs strict checking on netlink attributes for dump requests, and sets a dump filter if netlink attributes require it. We might want to just set a filter, without performing strict validation. Rename it to ip_filter_fib_dump_req(), and add a 'strict' boolean argument that must be set if strict validation is requested. This patch doesn't introduce any functional changes. Signed-off-by: Stefano Brivio <sbrivio@redhat.com> --- v4: New patch include/net/ip_fib.h | 6 +++--- net/ipv4/fib_frontend.c | 34 ++++++++++++++++++++++------------ net/ipv4/ipmr.c | 4 ++-- net/ipv6/ip6_fib.c | 2 +- net/ipv6/ip6mr.c | 4 ++-- net/mpls/af_mpls.c | 2 +- 6 files changed, 31 insertions(+), 21 deletions(-)