On Sun, Oct 07, 2018 at 08:16:43PM -0700, David Ahern wrote:
> From: David Ahern <dsah...@gmail.com>
>
> Move the existing input checking for rtnl_fdb_dump into a helper,
> valid_fdb_dump_legacy. This function will retain the current
> logic that works around the 2 headers that userspace has been
> allowed to send up to this point.
>
> Signed-off-by: David Ahern <dsah...@gmail.com>
Acked-by: Christian Brauner <christ...@brauner.io>
> ---
> net/core/rtnetlink.c | 53
> ++++++++++++++++++++++++++++++++--------------------
> 1 file changed, 33 insertions(+), 20 deletions(-)
>
> diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
> index f6d2609cfa9f..c7509c789fb6 100644
> --- a/net/core/rtnetlink.c
> +++ b/net/core/rtnetlink.c
> @@ -3799,22 +3799,13 @@ int ndo_dflt_fdb_dump(struct sk_buff *skb,
> }
> EXPORT_SYMBOL(ndo_dflt_fdb_dump);
>
> -static int rtnl_fdb_dump(struct sk_buff *skb, struct netlink_callback *cb)
> +static int valid_fdb_dump_legacy(const struct nlmsghdr *nlh,
> + int *br_idx, int *brport_idx,
> + struct netlink_ext_ack *extack)
> {
> - struct net_device *dev;
> + struct ifinfomsg *ifm = nlmsg_data(nlh);
You could move this cast after the
if (nlmsg_len(nlh) != sizeof(struct ndmsg) &&
(nlmsg_len(nlh) != sizeof(struct ndmsg) +
check. It doesn't matter that much but it minimizes the risk of someone
accidently accessing struct ifinfomsg *ifm when it's an invalid cast.
> struct nlattr *tb[IFLA_MAX+1];
> - struct net_device *br_dev = NULL;
> - const struct net_device_ops *ops = NULL;
> - const struct net_device_ops *cops = NULL;
> - struct ifinfomsg *ifm = nlmsg_data(cb->nlh);
> - struct net *net = sock_net(skb->sk);
> - struct hlist_head *head;
> - int brport_idx = 0;
> - int br_idx = 0;
> - int h, s_h;
> - int idx = 0, s_idx;
> - int err = 0;
> - int fidx = 0;
> + int err;
>
> /* A hack to preserve kernel<->userspace interface.
> * Before Linux v4.12 this code accepted ndmsg since iproute2 v3.3.0.
> @@ -3823,20 +3814,42 @@ static int rtnl_fdb_dump(struct sk_buff *skb, struct
> netlink_callback *cb)
> * Fortunately these sizes don't conflict with the size of ifinfomsg
> * with an optional attribute.
> */
> - if (nlmsg_len(cb->nlh) != sizeof(struct ndmsg) &&
> - (nlmsg_len(cb->nlh) != sizeof(struct ndmsg) +
> + if (nlmsg_len(nlh) != sizeof(struct ndmsg) &&
> + (nlmsg_len(nlh) != sizeof(struct ndmsg) +
> nla_attr_size(sizeof(u32)))) {
> - err = nlmsg_parse(cb->nlh, sizeof(struct ifinfomsg), tb,
> - IFLA_MAX, ifla_policy, cb->extack);
> + err = nlmsg_parse(nlh, sizeof(struct ifinfomsg), tb, IFLA_MAX,
> + ifla_policy, extack);
> if (err < 0) {
> return -EINVAL;
> } else if (err == 0) {
> if (tb[IFLA_MASTER])
> - br_idx = nla_get_u32(tb[IFLA_MASTER]);
> + *br_idx = nla_get_u32(tb[IFLA_MASTER]);
> }
>
> - brport_idx = ifm->ifi_index;
> + *brport_idx = ifm->ifi_index;
> }
> + return 0;
> +}
> +
> +static int rtnl_fdb_dump(struct sk_buff *skb, struct netlink_callback *cb)
> +{
> + struct net_device *dev;
> + struct net_device *br_dev = NULL;
> + const struct net_device_ops *ops = NULL;
> + const struct net_device_ops *cops = NULL;
> + struct net *net = sock_net(skb->sk);
> + struct hlist_head *head;
> + int brport_idx = 0;
> + int br_idx = 0;
> + int h, s_h;
> + int idx = 0, s_idx;
> + int err = 0;
> + int fidx = 0;
> +
> + err = valid_fdb_dump_legacy(cb->nlh, &br_idx, &brport_idx,
> + cb->extack);
> + if (err < 0)
> + return err;
>
> if (br_idx) {
> br_dev = __dev_get_by_index(net, br_idx);
> --
> 2.11.0
>
