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; Jason -- 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