On 31/12/2015 16:41, Yuval Shaia wrote:
> To support security applications, that need to filter out connections based
> on SGID, an ioctl command to retrieve SGID of a given socket is added.
Could you elaborate on the security applications? How do you see this ioctl
being used?
> diff --git a/drivers/infiniband/ulp/ipoib/ipoib_ioctl.c b/drivers/infiniband/ulp/ipoib/ipoib_ioctl.c
> new file mode 100644
> index 0000000..124d545
> --- /dev/null
> +++ b/drivers/infiniband/ulp/ipoib/ipoib_ioctl.c
> +static int ipoib_get_sguid(struct net_device *dev, int fd, u64 *sgid,
> + u64 *subnet_prefix)
A GID is composed of a GUID and a subnet prefix.
> +{
> + struct socket *sock;
> + struct inet_sock *inetsock;
> + struct neighbour *neigh;
> + int rc = 0;
> + union ib_gid *gid;
> + struct list_head *dev_list = 0;
> + struct ipoib_dev_priv *priv = netdev_priv(dev);
> + u16 pkey_index = priv->pkey_index;
> + struct ipoib_dev_priv *child_priv;
> +
> + sock = sockfd_lookup(fd, &rc);
> + if (IS_ERR_OR_NULL(sock))
> + return -EINVAL;
> +
> + inetsock = inet_sk(sock->sk);
> +
> + neigh = neigh_lookup(&arp_tbl, &inetsock->inet_daddr, dev);
Also, isn't inet_daddr the destination address? But the function claims
to return the SGID. I guess these can be ambiguous but still it seems
confusing.
> + if (!IS_ERR_OR_NULL(neigh))
> + goto found;
> +
> + /* If not found try in all other ipoib devices */
> + dev_list = ipoib_get_dev_list(priv->ca);
> + if (!dev_list)
> + return -EINVAL;
> +
> + list_for_each_entry(priv, dev_list, list) {
This list can contain devices that belong to a different namespace. It
doesn't make sense to do a neighbor lookup on these. Also, you can simply use
neigh_lookup_nodev if you don't care about the device.
> + if (priv->pkey_index == pkey_index) {
> + neigh = neigh_lookup(&arp_tbl, &inetsock->inet_daddr,
> + priv->dev);
> + if (!IS_ERR_OR_NULL(neigh))
> + goto found;
> + }
> + list_for_each_entry(child_priv, &priv->child_intfs, list) {
> + if (child_priv->pkey_index == pkey_index) {
> + neigh = neigh_lookup(&arp_tbl,
> + &inetsock->inet_daddr,
> + child_priv->dev);
> + if (!IS_ERR_OR_NULL(neigh))
> + goto found;
> + }
> + }
> + }
> +
> + return -ENODEV;
> +
> +found:
> + if (!(neigh->nud_state & NUD_VALID))
> + return -EINVAL;
> +
> + gid = (union ib_gid *)(neigh->ha + 4);
If you just take the GID from the hardware address why not do that from userspace?
Why do you need a new ioctl to do that for you?
> + *sgid = be64_to_cpu(gid->global.interface_id);
> + *subnet_prefix = be64_to_cpu(gid->global.subnet_prefix);
> +
> + neigh_release(neigh);
> +
> + return 0;
> +}
Regards,
Haggai
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
