On 12/14/18 10:43 AM, Roopa Prabhu wrote:
> diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
> index f8bdb8ad..fcea76b 100644
> --- a/net/core/rtnetlink.c
> +++ b/net/core/rtnetlink.c
> @@ -4021,6 +4021,111 @@ static int rtnl_fdb_dump(struct sk_buff *skb, struct
> netlink_callback *cb)
> return skb->len;
> }
>
> +static int rtnl_fdb_get(struct sk_buff *in_skb, struct nlmsghdr *nlh,
> + struct netlink_ext_ack *extack)
> +{
> + const struct net_device_ops *ops = NULL;
> + struct net *net = sock_net(in_skb->sk);
> + struct net_device *dev = NULL, *br_dev = NULL;
> + struct nlattr *tb[NDA_MAX + 1];
> + struct sk_buff *skb;
> + struct ndmsg *ndm;
> + int br_idx = 0;
> + u8 *addr;
> + u16 vid;
> + int err;
> +
> + err = nlmsg_parse(nlh, sizeof(*ndm), tb, NDA_MAX, NULL, extack);
New stuff should go in with strict checking, so nlmsg_parse_strict and a
check for any unsupported attributes (NDA) or ndm entries in the request.
Also, please add an nla_policy for easier NDA attributes ...
> + if (err < 0)
> + return err;
> +
> + ndm = nlmsg_data(nlh);
> + if (ndm->ndm_ifindex) {
> + dev = __dev_get_by_index(net, ndm->ndm_ifindex);
> + if (!dev) {
> + NL_SET_ERR_MSG(extack, "unknown dev ifindex");
General comment on all of the extack messages: No abbreviations in the
message, and it is nice to always start with a capital letter. "Unknown
device index"
> + return -ENODEV;
> + }
> + }
> +
> + if (!tb[NDA_LLADDR] || nla_len(tb[NDA_LLADDR]) != ETH_ALEN) {
> + NL_SET_ERR_MSG(extack, "invalid address");
> + return -EINVAL;
> + }
> +
> + addr = nla_data(tb[NDA_LLADDR]);
> +
> + err = fdb_vid_parse(tb[NDA_VLAN], &vid, extack);
> + if (err)
> + return err;
> +
> + if (tb[NDA_MASTER]) {
> + if (dev) {
> + NL_SET_ERR_MSG(extack, "master and dev are mutually
> exclusive");
> + return -EINVAL;
> + }
> +
> + br_idx = nla_get_u32(tb[NDA_MASTER]);
... so you know that NDA_MASTER is a u32.
> + br_dev = __dev_get_by_index(net, br_idx);
> + if (!br_dev) {
> + NL_SET_ERR_MSG(extack, "invalid master ifindex");
> + return -EINVAL;
> + }
> + ops = br_dev->netdev_ops;
> + }
> +
