I was operating under the assumption that kernel code is trusted code (so as 
not to burden
the kernel will too many tests of correctness.

Below is a patch for verbs.c, which will accomplish the same check for both 
kernel
user space applications.  In addition, since the fix is on the same line, the 
patch
also checks that the QP we are trying to attach/detach is UD only (per IB Spec 
11.3.1).

Signed-off-by: Jack Morgenstein <[EMAIL PROTECTED]>

Index: linux-kernel/infiniband/core/verbs.c
===================================================================
--- linux-kernel/infiniband/core/verbs.c        (revision 3532)
+++ linux-kernel/infiniband/core/verbs.c        (working copy)
@@ -523,16 +523,16 @@
 
 int ib_attach_mcast(struct ib_qp *qp, union ib_gid *gid, u16 lid)
 {
-       return qp->device->attach_mcast ?
-               qp->device->attach_mcast(qp, gid, lid) :
-               -ENOSYS;
+       return !qp->device->attach_mcast ? -ENOSYS :
+               (gid->raw[0] != 0xFF || qp->qp_type != IB_QPT_UD) ? -EINVAL :
+               qp->device->attach_mcast(qp, gid, lid);
 }
 EXPORT_SYMBOL(ib_attach_mcast);
 
 int ib_detach_mcast(struct ib_qp *qp, union ib_gid *gid, u16 lid)
 {
-       return qp->device->detach_mcast ?
-               qp->device->detach_mcast(qp, gid, lid) :
-               -ENOSYS;
+       return !qp->device->detach_mcast ? -ENOSYS :
+               (gid->raw[0] != 0xFF || qp->qp_type != IB_QPT_UD) ? -EINVAL :
+               qp->device->detach_mcast(qp, gid, lid);
 }
 EXPORT_SYMBOL(ib_detach_mcast);


On Sun, Sep 25, 2005 at 12:10:38AM +0300, Roland Dreier wrote:
>     Jack> The following patch checks validity of MGID when
>     Jack> attaching/detaching a QP to/from a multicast group (for
>     Jack> user-space only). IB spec demands that multicast gids start
>     Jack> with 0xFF in 0-th byte. (IB Spec v1.2,section 4.1.1 (page
>     Jack> 144)).
> 
> This seems like a strange place to check the condition.  Why do we
> want to allow kernel callers to use invalid MGIDs?  In other words it
> would seem more appropriate to me to put the check in verbs.c.
> 
> Also no Signed-off-by: line for your patch...
> 
>  - R.
_______________________________________________
openib-general mailing list
[email protected]
http://openib.org/mailman/listinfo/openib-general

To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general

Reply via email to