On 28/8/2013 7:21 PM, Jason Gunthorpe wrote:
On Wed, Aug 28, 2013 at 03:47:54PM +0300, Matan Barak wrote:
+
+ if (cmd.comp_mask)
+ return -EINVAL;
So, how do you propose to interoperate with new user space/old
kernels?
How will user space know what comp_mask values the kernel will
support?
struct ib_uverbs_create_flow_resp contains a comp_mask value. This value
should contain the supported comp_masks fields.
Currently, we don't support any extensions, so we zero this field by
doing "memset(&resp, 0, sizeof(resp));"
I suggest returning an error and setting the response comp_mask field.
The notion that was established in the verbs patches is that extra
structure fields are ignored by old software.
I'm not aware of any such concrete example. Could you please point out ?
I think it's safer to return an error. Imagine that ibv_modify_qp was
extended for some crucial field, say IB_QP_AV2. Creating a QP without
indicating its AV (or AV2 in that case) is an invalid behavior. This
example is a bit artificial, though some future extensions could have
such mandatory fields.
The user should do ibv_query_device before requesting features that
might be unsupported.
+ if ((cmd.flow_attr.type == IB_FLOW_ATTR_SNIFFER &&
+ !capable(CAP_NET_ADMIN)) || !capable(CAP_NET_RAW))
+ return -EPERM;
+
+ if (cmd.flow_attr.num_of_specs < 0 ||
+ cmd.flow_attr.num_of_specs > IB_FLOW_SPEC_SUPPORT_LAYERS)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ > checked here
+ return -EINVAL;
+
+ kern_attr_size = cmd.flow_attr.size - sizeof(cmd) -
+ sizeof(struct ib_uverbs_cmd_hdr_ex);
+
+ if (cmd.flow_attr.size < 0 || cmd.flow_attr.size > in_len ||
+ kern_attr_size < 0 || kern_attr_size >
+ (cmd.flow_attr.num_of_specs * sizeof(struct ib_kern_spec)))
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Watch out for integer overflow here..
I don't think there's a risk of integer overflow since sizeof(struct
ib_kern_spec) is a constant of ~50 bytes long and
cmd.flow_attr.num_of_specs is checked above and is at most 4.
Jason
Since the code was already merged, I'll post an incremental patch and I
suggest that we'll continue things there.
- Matan
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
