On 8/9/2018 5:10 PM, Jason Gunthorpe wrote:
On Thu, Aug 09, 2018 at 12:54:00PM +0000, Ruhl, Michael J wrote:
From: linux-rdma-owner@xxxxxxxxxxxxxxx [mailto:linux-rdma-
owner@xxxxxxxxxxxxxxx] On Behalf Of Jason Gunthorpe
Sent: Thursday, July 26, 2018 6:37 PM
To: linux-rdma@xxxxxxxxxxxxxxx; Leon Romanovsky <leonro@xxxxxxxxxxxx>
Subject: [PATCH v3] IB/uverbs: Add UVERBS_ATTR_FLAGS_IN to the specs
language
This clearly indicates that the input is a bitwise combination of values
in an enum, and identifies which enum contains the definition of the bits.
Special accessors are provided that handle the mandatory validation of the
allowed bits and enforce the correct type for bitwise flags.
If we had introduced this at the start then the kabi would have uniformly
used u64 data to pass flags, however today there is a mixture of u64 and
u32 flags. All places are converted to accept both sizes and the accessor
fixes it. This allows all existing flags to grow to u64 in future without
any hassle.
Finally all flags are, by definition, optional. If flags are not passed
the accessor does not fail, but provides a value of zero.
Signed-off-by: Jason Gunthorpe <jgg@xxxxxxxxxxxx>
drivers/infiniband/core/uverbs_ioctl.c | 51 +++++++++++++++++++
.../core/uverbs_std_types_counters.c | 10 ++--
drivers/infiniband/core/uverbs_std_types_cq.c | 13 +++--
drivers/infiniband/core/uverbs_std_types_mr.c | 10 ++--
drivers/infiniband/hw/mlx5/devx.c | 16 +++---
drivers/infiniband/hw/mlx5/main.c | 16 +++---
include/rdma/uverbs_ioctl.h | 33 ++++++++++++
7 files changed, 119 insertions(+), 30 deletions(-)
- v2 fixes compilation when !CONFIG_INFINIBAND_USER_ACCESS by provding
dummy static inline's.
- v3 uses BUILD_BUG_ON_ZERO to wrapper the spelling checking sizeof
instead of *0
diff --git a/drivers/infiniband/core/uverbs_ioctl.c
b/drivers/infiniband/core/uverbs_ioctl.c
index db7a92ea5dbe87..23a1777f26e278 100644
+++ b/drivers/infiniband/core/uverbs_ioctl.c
@@ -486,3 +486,54 @@ long ib_uverbs_ioctl(struct file *filp, unsigned int
cmd, unsigned long arg)
return err;
}
+
+int uverbs_get_flags64(u64 *to, const struct uverbs_attr_bundle
*attrs_bundle,
+ size_t idx, u64 allowed_bits)
+{
+ const struct uverbs_attr *attr;
+ u64 flags;
+
+ attr = uverbs_attr_get(attrs_bundle, idx);
+ /* Missing attribute means 0 flags */
+ if (IS_ERR(attr)) {
+ *to = 0;
+ return 0;
+ }
+
+ /*
+ * New userspace code should use 8 bytes to pass flags, but we
+ * transparently support old userspaces that were using 4 bytes as
+ * well.
+ */
+ if (attr->ptr_attr.len == 8)
+ flags = attr->ptr_attr.data;
+ else if (attr->ptr_attr.len == 4)
+ memcpy(&flags, &attr->ptr_attr.data, 4);
If you are doing this, can't the top 4 bytes have garbage from the stack?
Indeed, this was missed during testing apparently. It should read
- memcpy(&flags, &attr->ptr_attr.data, 4);
+ flags = *(u32 *)&attr->ptr_attr.data;
Hi Jason,
It looks that 'for-next' doesn't include yet this fix, let's please not
miss it. I just hit this problem in some test which uses this code as
part of DEVX.
Yishai
