[PATCH rdma-rc 00/15] RDMA fixes for v4.16

[PATCH rdma-rc 00/15] RDMA fixes for v4.16

[Leon Romanovsky]

Hi,

This is mostly fixes for uverbs layers with two extra patches: one from
Alaa, who removed annoying warning prints in IPoIB and another from Leon
who removed unimplented part of restrack code.

Thanks

Alaa Hleihel (1):
  IB/ipoib: Do not warn if IPoIB debugfs doesn't exist

Jason Gunthorpe (7):
  IB/uverbs: Use inline data transfer for UHW_IN
  IB/uverbs: Use u64_to_user_ptr() not a union
  IB/uverbs: Use __aligned_u64 for uapi headers
  IB/uverbs: Hold the uobj write lock after allocate
  IB/uverbs: Tidy lockdep_check
  IB/uverbs: Tidy uverbs_uobject_add
  IB/uverbs: Fix unbalanced unlock on error path for
    rdma_explicit_destroy

Leon Romanovsky (3):
  RDMA/restrack: Remove unimplemented XRCD object
  RDMA/uverbs: Protect from races between lookup and destroy of uobjects
  RDMA/uverbs: Protect from command mask overflow

Matan Barak (4):
  IB/uverbs: Always the attribute size provided by the user
  IB/uverbs: Fix method merging in uverbs_ioctl_merge
  IB/uverbs: Add ioctl support for 32bit processes
  IB/uverbs: Fix possible oops with duplicate ioctl attributes

 drivers/infiniband/core/rdma_core.c          | 51 +++++++++++++++++-----------
 drivers/infiniband/core/restrack.c           |  5 ---
 drivers/infiniband/core/uverbs_ioctl.c       |  3 ++
 drivers/infiniband/core/uverbs_ioctl_merge.c | 18 +++++-----
 drivers/infiniband/core/uverbs_main.c        | 29 ++++++++++++----
 drivers/infiniband/core/uverbs_std_types.c   | 12 ++++---
 drivers/infiniband/ulp/ipoib/ipoib_fs.c      |  2 --
 include/rdma/restrack.h                      |  4 ---
 include/rdma/uverbs_ioctl.h                  | 43 ++++++++++++++++-------
 include/uapi/rdma/rdma_user_ioctl.h          |  4 +--
 10 files changed, 107 insertions(+), 64 deletions(-)

--
2.16.1

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH rdma-rc 01/15] IB/ipoib: Do not warn if IPoIB debugfs doesn't exist

[Leon Romanovsky]

From: Alaa Hleihel <alaa-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>

netdev_wait_allrefs() could rebroadcast NETDEV_UNREGISTER event
multiple times until all refs are gone, which will result in calling
ipoib_delete_debug_files multiple times and printing a warning.

Remove the WARN_ONCE since checks of NULL pointers before calling
debugfs_remove are not needed.

Fixes: 771a52584096 ("IB/IPoIB: ibX: failed to create mcg debug file")
Signed-off-by: Alaa Hleihel <alaa-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Signed-off-by: Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
 drivers/infiniband/ulp/ipoib/ipoib_fs.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/infiniband/ulp/ipoib/ipoib_fs.c b/drivers/infiniband/ulp/ipoib/ipoib_fs.c
index 11f74cbe6660..ea302b054601 100644
--- a/drivers/infiniband/ulp/ipoib/ipoib_fs.c
+++ b/drivers/infiniband/ulp/ipoib/ipoib_fs.c
@@ -281,8 +281,6 @@ void ipoib_delete_debug_files(struct net_device *dev)
 {
 	struct ipoib_dev_priv *priv = ipoib_priv(dev);
 
-	WARN_ONCE(!priv->mcg_dentry, "null mcg debug file\n");
-	WARN_ONCE(!priv->path_dentry, "null path debug file\n");
 	debugfs_remove(priv->mcg_dentry);
 	debugfs_remove(priv->path_dentry);
 	priv->mcg_dentry = priv->path_dentry = NULL;
-- 
2.16.1

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH rdma-rc 02/15] RDMA/restrack: Remove unimplemented XRCD object

[Leon Romanovsky]

From: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>

Resource tracking of XRCD objects is not implemented in current
version of restrack and hence can be removed.

Fixes: 02d8883f520e ("RDMA/restrack: Add general infrastructure to track RDMA resources")
Signed-off-by: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
---
 drivers/infiniband/core/restrack.c | 5 -----
 include/rdma/restrack.h            | 4 ----
 2 files changed, 9 deletions(-)

diff --git a/drivers/infiniband/core/restrack.c b/drivers/infiniband/core/restrack.c
index 857637bf46da..d8dc709a3715 100644
--- a/drivers/infiniband/core/restrack.c
+++ b/drivers/infiniband/core/restrack.c
@@ -63,7 +63,6 @@ static struct ib_device *res_to_dev(struct rdma_restrack_entry *res)
 {
 	enum rdma_restrack_type type = res->type;
 	struct ib_device *dev;
-	struct ib_xrcd *xrcd;
 	struct ib_pd *pd;
 	struct ib_cq *cq;
 	struct ib_qp *qp;
@@ -81,10 +80,6 @@ static struct ib_device *res_to_dev(struct rdma_restrack_entry *res)
 		qp = container_of(res, struct ib_qp, res);
 		dev = qp->device;
 		break;
-	case RDMA_RESTRACK_XRCD:
-		xrcd = container_of(res, struct ib_xrcd, res);
-		dev = xrcd->device;
-		break;
 	default:
 		WARN_ONCE(true, "Wrong resource tracking type %u\n", type);
 		return NULL;
diff --git a/include/rdma/restrack.h b/include/rdma/restrack.h
index c2d81167c858..2cdf8dcf4bdc 100644
--- a/include/rdma/restrack.h
+++ b/include/rdma/restrack.h
@@ -28,10 +28,6 @@ enum rdma_restrack_type {
 	 * @RDMA_RESTRACK_QP: Queue pair (QP)
 	 */
 	RDMA_RESTRACK_QP,
-	/**
-	 * @RDMA_RESTRACK_XRCD: XRC domain (XRCD)
-	 */
-	RDMA_RESTRACK_XRCD,
 	/**
 	 * @RDMA_RESTRACK_MAX: Last entry, used for array dclarations
 	 */
-- 
2.16.1

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH rdma-rc 03/15] IB/uverbs: Always the attribute size provided by the user

[Leon Romanovsky]

From: Matan Barak <matanb-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>

This fixes several bugs around the copy_to/from user path:
 - copy_to used the user provided size of the attribute
   and could copy data beyond the end of the kernel buffer into
   userspace.
 - copy_from didn't know the size of the kernel buffer and
   could have left kernel memory unexpectedly un-initialized.
 - copy_from did not use the user length to determine if the
   attribute data is inlined or not.

Signed-off-by: Matan Barak <matanb-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Signed-off-by: Jason Gunthorpe <jgg-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Signed-off-by: Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
 drivers/infiniband/core/uverbs_std_types.c |  5 +++--
 include/rdma/uverbs_ioctl.h                | 35 ++++++++++++++++++++++++------
 2 files changed, 31 insertions(+), 9 deletions(-)

diff --git a/drivers/infiniband/core/uverbs_std_types.c b/drivers/infiniband/core/uverbs_std_types.c
index cab0ac3556eb..c6502c7b7c46 100644
--- a/drivers/infiniband/core/uverbs_std_types.c
+++ b/drivers/infiniband/core/uverbs_std_types.c
@@ -323,7 +323,8 @@ static int uverbs_create_cq_handler(struct ib_device *ib_dev,
 	cq->res.type = RDMA_RESTRACK_CQ;
 	rdma_restrack_add(&cq->res);
 
-	ret = uverbs_copy_to(attrs, CREATE_CQ_RESP_CQE, &cq->cqe);
+	ret = uverbs_copy_to(attrs, CREATE_CQ_RESP_CQE, &cq->cqe,
+			     sizeof(cq->cqe));
 	if (ret)
 		goto err_cq;
 
@@ -375,7 +376,7 @@ static int uverbs_destroy_cq_handler(struct ib_device *ib_dev,
 	resp.comp_events_reported  = obj->comp_events_reported;
 	resp.async_events_reported = obj->async_events_reported;
 
-	return uverbs_copy_to(attrs, DESTROY_CQ_RESP, &resp);
+	return uverbs_copy_to(attrs, DESTROY_CQ_RESP, &resp, sizeof(resp));
 }
 
 static DECLARE_UVERBS_METHOD(
diff --git a/include/rdma/uverbs_ioctl.h b/include/rdma/uverbs_ioctl.h
index 6da44079aa58..32cb14703914 100644
--- a/include/rdma/uverbs_ioctl.h
+++ b/include/rdma/uverbs_ioctl.h
@@ -351,29 +351,50 @@ static inline const struct uverbs_attr *uverbs_attr_get(const struct uverbs_attr
 }
 
 static inline int uverbs_copy_to(const struct uverbs_attr_bundle *attrs_bundle,
-				 size_t idx, const void *from)
+				 size_t idx, const void *from, size_t size)
 {
 	const struct uverbs_attr *attr = uverbs_attr_get(attrs_bundle, idx);
 	u16 flags;
+	size_t min_size;
 
 	if (IS_ERR(attr))
 		return PTR_ERR(attr);
 
+	min_size = min_t(size_t, attr->ptr_attr.len, size);
+	if (copy_to_user(attr->ptr_attr.ptr, from, min_size))
+		return -EFAULT;
+
 	flags = attr->ptr_attr.flags | UVERBS_ATTR_F_VALID_OUTPUT;
-	return (!copy_to_user(attr->ptr_attr.ptr, from, attr->ptr_attr.len) &&
-		!put_user(flags, &attr->uattr->flags)) ? 0 : -EFAULT;
+	if (put_user(flags, &attr->uattr->flags))
+		return -EFAULT;
+
+	return 0;
 }
 
-static inline int _uverbs_copy_from(void *to, size_t to_size,
+static inline bool uverbs_attr_ptr_is_inline(const struct uverbs_attr *attr)
+{
+	return attr->ptr_attr.len <= sizeof(attr->ptr_attr.data);
+}
+
+static inline int _uverbs_copy_from(void *to,
 				    const struct uverbs_attr_bundle *attrs_bundle,
-				    size_t idx)
+				    size_t idx,
+				    size_t size)
 {
 	const struct uverbs_attr *attr = uverbs_attr_get(attrs_bundle, idx);
 
 	if (IS_ERR(attr))
 		return PTR_ERR(attr);
 
-	if (to_size <= sizeof(((struct ib_uverbs_attr *)0)->data))
+	/*
+	 * Validation ensures attr->ptr_attr.len >= size. If the caller is
+	 * using UVERBS_ATTR_SPEC_F_MIN_SZ then it must call copy_from with
+	 * the right size.
+	 */
+	if (unlikely(size < attr->ptr_attr.len))
+		return -EINVAL;
+
+	if (uverbs_attr_ptr_is_inline(attr))
 		memcpy(to, &attr->ptr_attr.data, attr->ptr_attr.len);
 	else if (copy_from_user(to, attr->ptr_attr.ptr, attr->ptr_attr.len))
 		return -EFAULT;
@@ -382,7 +403,7 @@ static inline int _uverbs_copy_from(void *to, size_t to_size,
 }
 
 #define uverbs_copy_from(to, attrs_bundle, idx)				      \
-	_uverbs_copy_from(to, sizeof(*(to)), attrs_bundle, idx)
+	_uverbs_copy_from(to, attrs_bundle, idx, sizeof(*to))
 
 /* =================================================
  *	 Definitions -> Specs infrastructure
-- 
2.16.1

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH rdma-rc 04/15] IB/uverbs: Use inline data transfer for UHW_IN

[Leon Romanovsky]

From: Jason Gunthorpe <jgg-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>

The rule for the API is pointers less than 8 bytes are inlined into
the .data field of the attribute. Fix the creation of the driver udata
struct to follow this rule and point to the .data itself when the size
is less than 8 bytes.

Otherwise if the UHW struct is less than 8 bytes the driver will get
EFAULT during copy_from_user.

Signed-off-by: Jason Gunthorpe <jgg-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Signed-off-by: Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
 drivers/infiniband/core/uverbs_std_types.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/infiniband/core/uverbs_std_types.c b/drivers/infiniband/core/uverbs_std_types.c
index c6502c7b7c46..7b0e4d778d79 100644
--- a/drivers/infiniband/core/uverbs_std_types.c
+++ b/drivers/infiniband/core/uverbs_std_types.c
@@ -234,8 +234,11 @@ static void create_udata(struct uverbs_attr_bundle *ctx,
 		uverbs_attr_get(ctx, UVERBS_UHW_OUT);
 
 	if (!IS_ERR(uhw_in)) {
-		udata->inbuf = uhw_in->ptr_attr.ptr;
 		udata->inlen = uhw_in->ptr_attr.len;
+		if (uverbs_attr_ptr_is_inline(uhw_in))
+			udata->inbuf = &uhw_in->uattr->data;
+		else
+			udata->inbuf = uhw_in->ptr_attr.ptr;
 	} else {
 		udata->inbuf = NULL;
 		udata->inlen = 0;
-- 
2.16.1

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH rdma-rc 05/15] IB/uverbs: Use u64_to_user_ptr() not a union

[Leon Romanovsky]

From: Jason Gunthorpe <jgg-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>

The union approach will get the endianness wrong sometimes if the kernel's
pointer size is 32 bits resulting in EFAULTs when trying to copy to/from
user.

Signed-off-by: Jason Gunthorpe <jgg-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Signed-off-by: Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
 drivers/infiniband/core/uverbs_std_types.c |  4 ++--
 include/rdma/uverbs_ioctl.h                | 10 ++++------
 2 files changed, 6 insertions(+), 8 deletions(-)

diff --git a/drivers/infiniband/core/uverbs_std_types.c b/drivers/infiniband/core/uverbs_std_types.c
index 7b0e4d778d79..df1360e6774f 100644
--- a/drivers/infiniband/core/uverbs_std_types.c
+++ b/drivers/infiniband/core/uverbs_std_types.c
@@ -238,14 +238,14 @@ static void create_udata(struct uverbs_attr_bundle *ctx,
 		if (uverbs_attr_ptr_is_inline(uhw_in))
 			udata->inbuf = &uhw_in->uattr->data;
 		else
-			udata->inbuf = uhw_in->ptr_attr.ptr;
+			udata->inbuf = u64_to_user_ptr(uhw_in->ptr_attr.data);
 	} else {
 		udata->inbuf = NULL;
 		udata->inlen = 0;
 	}
 
 	if (!IS_ERR(uhw_out)) {
-		udata->outbuf = uhw_out->ptr_attr.ptr;
+		udata->outbuf = u64_to_user_ptr(uhw_out->ptr_attr.data);
 		udata->outlen = uhw_out->ptr_attr.len;
 	} else {
 		udata->outbuf = NULL;
diff --git a/include/rdma/uverbs_ioctl.h b/include/rdma/uverbs_ioctl.h
index 32cb14703914..38287d9d23a1 100644
--- a/include/rdma/uverbs_ioctl.h
+++ b/include/rdma/uverbs_ioctl.h
@@ -276,10 +276,7 @@ struct uverbs_object_tree_def {
  */
 
 struct uverbs_ptr_attr {
-	union {
-		u64		data;
-		void	__user *ptr;
-	};
+	u64		data;
 	u16		len;
 	/* Combination of bits from enum UVERBS_ATTR_F_XXXX */
 	u16		flags;
@@ -361,7 +358,7 @@ static inline int uverbs_copy_to(const struct uverbs_attr_bundle *attrs_bundle,
 		return PTR_ERR(attr);
 
 	min_size = min_t(size_t, attr->ptr_attr.len, size);
-	if (copy_to_user(attr->ptr_attr.ptr, from, min_size))
+	if (copy_to_user(u64_to_user_ptr(attr->ptr_attr.data), from, min_size))
 		return -EFAULT;
 
 	flags = attr->ptr_attr.flags | UVERBS_ATTR_F_VALID_OUTPUT;
@@ -396,7 +393,8 @@ static inline int _uverbs_copy_from(void *to,
 
 	if (uverbs_attr_ptr_is_inline(attr))
 		memcpy(to, &attr->ptr_attr.data, attr->ptr_attr.len);
-	else if (copy_from_user(to, attr->ptr_attr.ptr, attr->ptr_attr.len))
+	else if (copy_from_user(to, u64_to_user_ptr(attr->ptr_attr.data),
+				attr->ptr_attr.len))
 		return -EFAULT;
 
 	return 0;
-- 
2.16.1

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH rdma-rc 06/15] IB/uverbs: Fix method merging in uverbs_ioctl_merge

[Leon Romanovsky]

From: Matan Barak <matanb-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>

Fix a bug in uverbs_ioctl_merge that looked at objects iterators
number instead of methods iterators number when merging methods.

While we're at it, make the uverbs_ioctl_merge code a bit more clear
and faster.

Fixes: 118620d3686b ('IB/core: Add uverbs merge trees functionality')
Signed-off-by: Matan Barak <matanb-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Signed-off-by: Jason Gunthorpe <jgg-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Signed-off-by: Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
 drivers/infiniband/core/uverbs_ioctl_merge.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/infiniband/core/uverbs_ioctl_merge.c b/drivers/infiniband/core/uverbs_ioctl_merge.c
index 062485f9300d..62e1eb1d2a28 100644
--- a/drivers/infiniband/core/uverbs_ioctl_merge.c
+++ b/drivers/infiniband/core/uverbs_ioctl_merge.c
@@ -114,6 +114,7 @@ static size_t get_elements_above_id(const void **iters,
 	short min = SHRT_MAX;
 	const void *elem;
 	int i, j, last_stored = -1;
+	unsigned int equal_min = 0;
 
 	for_each_element(elem, i, j, elements, num_elements, num_offset,
 			 data_offset) {
@@ -136,6 +137,10 @@ static size_t get_elements_above_id(const void **iters,
 		 */
 		iters[last_stored == i ? num_iters - 1 : num_iters++] = elem;
 		last_stored = i;
+		if (min == GET_ID(id))
+			equal_min++;
+		else
+			equal_min = 1;
 		min = GET_ID(id);
 	}
 
@@ -146,15 +151,10 @@ static size_t get_elements_above_id(const void **iters,
 	 * Therefore, we need to clean the beginning of the array to make sure
 	 * all ids of final elements are equal to min.
 	 */
-	for (i = num_iters - 1; i >= 0 &&
-	     GET_ID(*(u16 *)(iters[i] + id_offset)) == min; i--)
-		;
-
-	num_iters -= i + 1;
-	memmove(iters, iters + i + 1, sizeof(*iters) * num_iters);
+	memmove(iters, iters + num_iters - equal_min, sizeof(*iters) * equal_min);
 
 	*min_id = min;
-	return num_iters;
+	return equal_min;
 }
 
 #define find_max_element_entry_id(num_elements, elements, num_objects_fld, \
@@ -322,7 +322,7 @@ static struct uverbs_method_spec *build_method_with_attrs(const struct uverbs_me
 		hash = kzalloc(sizeof(*hash) +
 			       ALIGN(sizeof(*hash->attrs) * (attr_max_bucket + 1),
 				     sizeof(long)) +
-			       BITS_TO_LONGS(attr_max_bucket) * sizeof(long),
+			       BITS_TO_LONGS(attr_max_bucket + 1) * sizeof(long),
 			       GFP_KERNEL);
 		if (!hash) {
 			res = -ENOMEM;
@@ -509,7 +509,7 @@ static struct uverbs_object_spec *build_object_with_methods(const struct uverbs_
 			 * first handler which != NULL. This also defines the
 			 * set of flags used for this handler.
 			 */
-			for (i = num_object_defs - 1;
+			for (i = num_method_defs - 1;
 			     i >= 0 && !method_defs[i]->handler; i--)
 				;
 			hash->methods[min_id++] = method;
-- 
2.16.1

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH rdma-rc 07/15] IB/uverbs: Use __aligned_u64 for uapi headers

[Leon Romanovsky]

From: Jason Gunthorpe <jgg-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>

This has no impact on the structure layout since these structs already
have their u64s already properly aligned, but it does document that we
have this requirement for 32 bit compatibility.

Signed-off-by: Jason Gunthorpe <jgg-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Signed-off-by: Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
 include/uapi/rdma/rdma_user_ioctl.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/uapi/rdma/rdma_user_ioctl.h b/include/uapi/rdma/rdma_user_ioctl.h
index 03557b5f9aa6..46de0885e800 100644
--- a/include/uapi/rdma/rdma_user_ioctl.h
+++ b/include/uapi/rdma/rdma_user_ioctl.h
@@ -65,7 +65,7 @@ struct ib_uverbs_attr {
 	__u16 len;		/* only for pointers */
 	__u16 flags;		/* combination of UVERBS_ATTR_F_XXXX */
 	__u16 reserved;
-	__u64 data;		/* ptr to command, inline data or idr/fd */
+	__aligned_u64 data;	/* ptr to command, inline data or idr/fd */
 };
 
 struct ib_uverbs_ioctl_hdr {
@@ -73,7 +73,7 @@ struct ib_uverbs_ioctl_hdr {
 	__u16 object_id;
 	__u16 method_id;
 	__u16 num_attrs;
-	__u64 reserved;
+	__aligned_u64 reserved;
 	struct ib_uverbs_attr  attrs[0];
 };
 
-- 
2.16.1

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH rdma-rc 08/15] IB/uverbs: Add ioctl support for 32bit processes

[Leon Romanovsky]

From: Matan Barak <matanb-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>

32 bit processes running on a 64 bit kernel call compat_ioctl so that
implementations can revise any structure layout issues. Point compat_ioctl
at our normal ioctl because:

- All our structures are designed to be the same on 32 and 64 bit, ie we
  use __aligned_u64 when required and are careful to manage padding.

- Any pointers are stored in u64's and userspace is expected
  to prepare them properly.

Signed-off-by: Matan Barak <matanb-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Signed-off-by: Jason Gunthorpe <jgg-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Signed-off-by: Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
 drivers/infiniband/core/uverbs_main.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/infiniband/core/uverbs_main.c b/drivers/infiniband/core/uverbs_main.c
index 395a3b091229..cd72555ad457 100644
--- a/drivers/infiniband/core/uverbs_main.c
+++ b/drivers/infiniband/core/uverbs_main.c
@@ -942,6 +942,7 @@ static const struct file_operations uverbs_fops = {
 	.llseek	 = no_llseek,
 #if IS_ENABLED(CONFIG_INFINIBAND_EXP_USER_ACCESS)
 	.unlocked_ioctl = ib_uverbs_ioctl,
+	.compat_ioctl = ib_uverbs_ioctl,
 #endif
 };
 
@@ -954,6 +955,7 @@ static const struct file_operations uverbs_mmap_fops = {
 	.llseek	 = no_llseek,
 #if IS_ENABLED(CONFIG_INFINIBAND_EXP_USER_ACCESS)
 	.unlocked_ioctl = ib_uverbs_ioctl,
+	.compat_ioctl = ib_uverbs_ioctl,
 #endif
 };
 
-- 
2.16.1

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH rdma-rc 09/15] IB/uverbs: Fix possible oops with duplicate ioctl attributes

[Leon Romanovsky]

From: Matan Barak <matanb-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>

If the same attribute is listed twice by the user in the ioctl attribute
list then error unwind can cause the kernel to deref garbage.

This happens when an object with WRITE access is sent twice. The second
parse properly fails but corrupts the state required for the error unwind
it triggers.

Fixing this by making duplicates in the attribute list invalid. This is
not something we need to support.

The ioctl interface is currently recommended to be disabled in kConfig.

Signed-off-by: Matan Barak <matanb-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Signed-off-by: Jason Gunthorpe <jgg-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Signed-off-by: Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
 drivers/infiniband/core/uverbs_ioctl.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/infiniband/core/uverbs_ioctl.c b/drivers/infiniband/core/uverbs_ioctl.c
index d96dc1d17be1..339b85145044 100644
--- a/drivers/infiniband/core/uverbs_ioctl.c
+++ b/drivers/infiniband/core/uverbs_ioctl.c
@@ -59,6 +59,9 @@ static int uverbs_process_attr(struct ib_device *ibdev,
 			return 0;
 	}
 
+	if (test_bit(attr_id, attr_bundle_h->valid_bitmap))
+		return -EINVAL;
+
 	spec = &attr_spec_bucket->attrs[attr_id];
 	e = &elements[attr_id];
 	e->uattr = uattr_ptr;
-- 
2.16.1

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH rdma-rc 10/15] IB/uverbs: Hold the uobj write lock after allocate

[Leon Romanovsky]

From: Jason Gunthorpe <jgg-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>

This clarifies the design intention that time between allocate and
commit has the uobj exclusive to the caller. We already guarantee
this by delaying publishing the uobj pointer via idr_insert,
fd_install, list_add, etc.

Additionally holding the usecnt lock during this period provides
extra clarity and more protection against future mistakes.

Fixes: 3832125624b7 ("IB/core: Add support for idr types")
Signed-off-by: Jason Gunthorpe <jgg-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Signed-off-by: Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
 drivers/infiniband/core/rdma_core.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/infiniband/core/rdma_core.c b/drivers/infiniband/core/rdma_core.c
index 85b5ee4defa4..82afb0cdcca3 100644
--- a/drivers/infiniband/core/rdma_core.c
+++ b/drivers/infiniband/core/rdma_core.c
@@ -141,7 +141,12 @@ static struct ib_uobject *alloc_uobj(struct ib_ucontext *context,
 	 */
 	uobj->context = context;
 	uobj->type = type;
-	atomic_set(&uobj->usecnt, 0);
+	/*
+	 * Allocated objects start out as write locked to deny any other
+	 * syscalls from accessing them until they are commited. See
+	 * rdma_alloc_commit_uobject
+	 */
+	atomic_set(&uobj->usecnt, -1);
 	kref_init(&uobj->ref);
 
 	return uobj;
@@ -527,6 +532,10 @@ int rdma_alloc_commit_uobject(struct ib_uobject *uobj)
 		return ret;
 	}
 
+	/* matches atomic_set(-1) in alloc_uobj */
+	lockdep_check(uobj, true);
+	atomic_set(&uobj->usecnt, 0);
+
 	uobj->type->type_class->alloc_commit(uobj);
 	up_read(&uobj->context->cleanup_rwsem);
 
-- 
2.16.1

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH rdma-rc 11/15] RDMA/uverbs: Protect from races between lookup and destroy of uobjects

[Leon Romanovsky]

From: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>

The race is between lookup_get_idr_uobject and
uverbs_idr_remove_uobj -> uverbs_uobject_put.

We deliberately do not call sychronize_rcu after the idr_remove in
uverbs_idr_remove_uobj for performance reasons, instead we call
kfree_rcu() during uverbs_uobject_put.

However, this means we can obtain pointers to uobj's that have
already been released and must protect against krefing them
using kref_get_unless_zero.

==================================================================
BUG: KASAN: use-after-free in copy_ah_attr_from_uverbs.isra.2+0x860/0xa00
Read of size 4 at addr ffff88005fda1ac8 by task syz-executor2/441

CPU: 1 PID: 441 Comm: syz-executor2 Not tainted 4.15.0-rc2+ #56
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS
rel-1.7.5-0-ge51488c-20140602_164612-nilsson.home.kraxel.org 04/01/2014
Call Trace:
dump_stack+0x8d/0xd4
print_address_description+0x73/0x290
kasan_report+0x25c/0x370
? copy_ah_attr_from_uverbs.isra.2+0x860/0xa00
copy_ah_attr_from_uverbs.isra.2+0x860/0xa00
? uverbs_try_lock_object+0x68/0xc0
? modify_qp.isra.7+0xdc4/0x10e0
modify_qp.isra.7+0xdc4/0x10e0
ib_uverbs_modify_qp+0xfe/0x170
? ib_uverbs_query_qp+0x970/0x970
? __lock_acquire+0xa11/0x1da0
ib_uverbs_write+0x55a/0xad0
? ib_uverbs_query_qp+0x970/0x970
? ib_uverbs_query_qp+0x970/0x970
? ib_uverbs_open+0x760/0x760
? futex_wake+0x147/0x410
? sched_clock_cpu+0x18/0x180
? check_prev_add+0x1680/0x1680
? do_futex+0x3b6/0xa30
? sched_clock_cpu+0x18/0x180
__vfs_write+0xf7/0x5c0
? ib_uverbs_open+0x760/0x760
? kernel_read+0x110/0x110
? lock_acquire+0x370/0x370
? __fget+0x264/0x3b0
vfs_write+0x18a/0x460
SyS_write+0xc7/0x1a0
? SyS_read+0x1a0/0x1a0
? trace_hardirqs_on_thunk+0x1a/0x1c
entry_SYSCALL_64_fastpath+0x18/0x85
RIP: 0033:0x448e29
RSP: 002b:00007f443fee0c58 EFLAGS: 00000246 ORIG_RAX: 0000000000000001
RAX: ffffffffffffffda RBX: 00007f443fee16bc RCX: 0000000000448e29
RDX: 0000000000000078 RSI: 00000000209f8000 RDI: 0000000000000012
RBP: 000000000070bea0 R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000246 R12: 00000000ffffffff
R13: 0000000000008e98 R14: 00000000006ebf38 R15: 0000000000000000

Allocated by task 1:
kmem_cache_alloc_trace+0x16c/0x2f0
mlx5_alloc_cmd_msg+0x12e/0x670
cmd_exec+0x419/0x1810
mlx5_cmd_exec+0x40/0x70
mlx5_core_mad_ifc+0x187/0x220
mlx5_MAD_IFC+0xd7/0x1b0
mlx5_query_mad_ifc_gids+0x1f3/0x650
mlx5_ib_query_gid+0xa4/0xc0
ib_query_gid+0x152/0x1a0
ib_query_port+0x21e/0x290
mlx5_port_immutable+0x30f/0x490
ib_register_device+0x5dd/0x1130
mlx5_ib_add+0x3e7/0x700
mlx5_add_device+0x124/0x510
mlx5_register_interface+0x11f/0x1c0
mlx5_ib_init+0x56/0x61
do_one_initcall+0xa3/0x250
kernel_init_freeable+0x309/0x3b8
kernel_init+0x14/0x180
ret_from_fork+0x24/0x30

Freed by task 1:
kfree+0xeb/0x2f0
mlx5_free_cmd_msg+0xcd/0x140
cmd_exec+0xeba/0x1810
mlx5_cmd_exec+0x40/0x70
mlx5_core_mad_ifc+0x187/0x220
mlx5_MAD_IFC+0xd7/0x1b0
mlx5_query_mad_ifc_gids+0x1f3/0x650
mlx5_ib_query_gid+0xa4/0xc0
ib_query_gid+0x152/0x1a0
ib_query_port+0x21e/0x290
mlx5_port_immutable+0x30f/0x490
ib_register_device+0x5dd/0x1130
mlx5_ib_add+0x3e7/0x700
mlx5_add_device+0x124/0x510
mlx5_register_interface+0x11f/0x1c0
mlx5_ib_init+0x56/0x61
do_one_initcall+0xa3/0x250
kernel_init_freeable+0x309/0x3b8
kernel_init+0x14/0x180
ret_from_fork+0x24/0x30

The buggy address belongs to the object at ffff88005fda1ab0
which belongs to the cache kmalloc-32 of size 32
The buggy address is located 24 bytes inside of
32-byte region [ffff88005fda1ab0, ffff88005fda1ad0)
The buggy address belongs to the page:
page:00000000d5655c19 count:1 mapcount:0 mapping: (null)
index:0xffff88005fda1fc0
flags: 0x4000000000000100(slab)
raw: 4000000000000100 0000000000000000 ffff88005fda1fc0 0000000180550008
raw: ffffea00017f6780 0000000400000004 ffff88006c803980 0000000000000000
page dumped because: kasan: bad access detected

Memory state around the buggy address:
ffff88005fda1980: fc fc fb fb fb fb fc fc fb fb fb fb fc fc fb fb
ffff88005fda1a00: fb fb fc fc fb fb fb fb fc fc 00 00 00 00 fc fc
ffff88005fda1a80: fb fb fb fb fc fc fb fb fb fb fc fc fb fb fb fb
ffff88005fda1b00: fc fc 00 00 00 00 fc fc fb fb fb fb fc fc fb fb
ffff88005fda1b80: fb fb fc fc fb fb fb fb fc fc fb fb fb fb fc fc
==================================================================@

Cc: syzkaller <syzkaller-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org>
Cc: <stable-u79uwXL29TY76Z2rM5mHXA@public.gmane.org> # 4.11
Fixes: 3832125624b7 ("IB/core: Add support for idr types")
Reported-by: Noa Osherovich <noaos-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Signed-off-by: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
---
 drivers/infiniband/core/rdma_core.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/infiniband/core/rdma_core.c b/drivers/infiniband/core/rdma_core.c
index 82afb0cdcca3..3ec5c7926f1c 100644
--- a/drivers/infiniband/core/rdma_core.c
+++ b/drivers/infiniband/core/rdma_core.c
@@ -201,7 +201,15 @@ static struct ib_uobject *lookup_get_idr_uobject(const struct uverbs_obj_type *t
 		goto free;
 	}
 
-	uverbs_uobject_get(uobj);
+	/*
+	 * The idr_find is guaranteed to return a pointer to something that
+	 * isn't freed yet, or NULL, as the free after idr_remove goes through
+	 * kfree_rcu(). However the object may still have been released and
+	 * kfree() could be called at any time.
+	 */
+	if (!kref_get_unless_zero(&uobj->ref))
+		uobj = ERR_PTR(-ENOENT);
+
 free:
 	rcu_read_unlock();
 	return uobj;
-- 
2.16.1

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH rdma-rc 12/15] IB/uverbs: Tidy lockdep_check

[Leon Romanovsky]

From: Jason Gunthorpe <jgg-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>

This is really being used as an asssert that the expected usecnt
is being held and implicitly that the usecnt is valid. Rename it to
assert_uverbs_usecnt and tighten the checks to only accept valid
values of usecnt (eg 0 and < -1 are invalid).

Fixes: 3832125624b7 ("IB/core: Add support for idr types")
Signed-off-by: Jason Gunthorpe <jgg-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Signed-off-by: Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
 drivers/infiniband/core/rdma_core.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/infiniband/core/rdma_core.c b/drivers/infiniband/core/rdma_core.c
index 3ec5c7926f1c..7aee1c0b9884 100644
--- a/drivers/infiniband/core/rdma_core.c
+++ b/drivers/infiniband/core/rdma_core.c
@@ -412,13 +412,13 @@ static int __must_check remove_commit_fd_uobject(struct ib_uobject *uobj,
 	return ret;
 }
 
-static void lockdep_check(struct ib_uobject *uobj, bool exclusive)
+static void assert_uverbs_usecnt(struct ib_uobject *uobj, bool exclusive)
 {
 #ifdef CONFIG_LOCKDEP
 	if (exclusive)
-		WARN_ON(atomic_read(&uobj->usecnt) > 0);
+		WARN_ON(atomic_read(&uobj->usecnt) != -1);
 	else
-		WARN_ON(atomic_read(&uobj->usecnt) == -1);
+		WARN_ON(atomic_read(&uobj->usecnt) <= 0);
 #endif
 }
 
@@ -457,7 +457,7 @@ int __must_check rdma_remove_commit_uobject(struct ib_uobject *uobj)
 		WARN(true, "ib_uverbs: Cleanup is running while removing an uobject\n");
 		return 0;
 	}
-	lockdep_check(uobj, true);
+	assert_uverbs_usecnt(uobj, true);
 	ret = _rdma_remove_commit_uobject(uobj, RDMA_REMOVE_DESTROY);
 
 	up_read(&ucontext->cleanup_rwsem);
@@ -487,7 +487,7 @@ int rdma_explicit_destroy(struct ib_uobject *uobject)
 		WARN(true, "ib_uverbs: Cleanup is running while removing an uobject\n");
 		return 0;
 	}
-	lockdep_check(uobject, true);
+	assert_uverbs_usecnt(uobject, true);
 	ret = uobject->type->type_class->remove_commit(uobject,
 						       RDMA_REMOVE_DESTROY);
 	if (ret)
@@ -541,7 +541,7 @@ int rdma_alloc_commit_uobject(struct ib_uobject *uobj)
 	}
 
 	/* matches atomic_set(-1) in alloc_uobj */
-	lockdep_check(uobj, true);
+	assert_uverbs_usecnt(uobj, true);
 	atomic_set(&uobj->usecnt, 0);
 
 	uobj->type->type_class->alloc_commit(uobj);
@@ -578,7 +578,7 @@ static void lookup_put_fd_uobject(struct ib_uobject *uobj, bool exclusive)
 
 void rdma_lookup_put_uobject(struct ib_uobject *uobj, bool exclusive)
 {
-	lockdep_check(uobj, exclusive);
+	assert_uverbs_usecnt(uobj, exclusive);
 	uobj->type->type_class->lookup_put(uobj, exclusive);
 	/*
 	 * In order to unlock an object, either decrease its usecnt for
-- 
2.16.1

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH rdma-rc 13/15] IB/uverbs: Tidy uverbs_uobject_add

[Leon Romanovsky]

From: Jason Gunthorpe <jgg-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>

Maintaining the uobjects list is mandatory, hoist it into the common
rdma_alloc_commit_uobject() function and inline it as there is now
only one caller.

Signed-off-by: Jason Gunthorpe <jgg-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Signed-off-by: Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
 drivers/infiniband/core/rdma_core.c | 13 ++++---------
 1 file changed, 4 insertions(+), 9 deletions(-)

diff --git a/drivers/infiniband/core/rdma_core.c b/drivers/infiniband/core/rdma_core.c
index 7aee1c0b9884..1cb6849b2231 100644
--- a/drivers/infiniband/core/rdma_core.c
+++ b/drivers/infiniband/core/rdma_core.c
@@ -350,13 +350,6 @@ struct ib_uobject *rdma_alloc_begin_uobject(const struct uverbs_obj_type *type,
 	return type->type_class->alloc_begin(type, ucontext);
 }
 
-static void uverbs_uobject_add(struct ib_uobject *uobject)
-{
-	mutex_lock(&uobject->context->uobjects_lock);
-	list_add(&uobject->list, &uobject->context->uobjects);
-	mutex_unlock(&uobject->context->uobjects_lock);
-}
-
 static int __must_check remove_commit_idr_uobject(struct ib_uobject *uobj,
 						  enum rdma_remove_reason why)
 {
@@ -501,7 +494,6 @@ int rdma_explicit_destroy(struct ib_uobject *uobject)
 
 static void alloc_commit_idr_uobject(struct ib_uobject *uobj)
 {
-	uverbs_uobject_add(uobj);
 	spin_lock(&uobj->context->ufile->idr_lock);
 	/*
 	 * We already allocated this IDR with a NULL object, so
@@ -517,7 +509,6 @@ static void alloc_commit_fd_uobject(struct ib_uobject *uobj)
 	struct ib_uobject_file *uobj_file =
 		container_of(uobj, struct ib_uobject_file, uobj);
 
-	uverbs_uobject_add(&uobj_file->uobj);
 	fd_install(uobj_file->uobj.id, uobj->object);
 	/* This shouldn't be used anymore. Use the file object instead */
 	uobj_file->uobj.id = 0;
@@ -544,6 +535,10 @@ int rdma_alloc_commit_uobject(struct ib_uobject *uobj)
 	assert_uverbs_usecnt(uobj, true);
 	atomic_set(&uobj->usecnt, 0);
 
+	mutex_lock(&uobj->context->uobjects_lock);
+	list_add(&uobj->list, &uobj->context->uobjects);
+	mutex_unlock(&uobj->context->uobjects_lock);
+
 	uobj->type->type_class->alloc_commit(uobj);
 	up_read(&uobj->context->cleanup_rwsem);
 
-- 
2.16.1

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH rdma-rc 14/15] IB/uverbs: Fix unbalanced unlock on error path for rdma_explicit_destroy

[Leon Romanovsky]

From: Jason Gunthorpe <jgg-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>

If remove_commit fails then the lock is left locked while the uobj still
exists. Eventually the kernel will deadlock.

lockdep detects this and says:

 test/4221 is leaving the kernel with locks still held!
 1 lock held by test/4221:
  #0:  (&ucontext->cleanup_rwsem){.+.+}, at: [<000000001e5c7523>] rdma_explicit_destroy+0x37/0x120 [ib_uverbs]

Fixes: 4da70da23e9b ("IB/core: Explicitly destroy an object while keeping uobject")
Signed-off-by: Jason Gunthorpe <jgg-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Signed-off-by: Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
 drivers/infiniband/core/rdma_core.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/infiniband/core/rdma_core.c b/drivers/infiniband/core/rdma_core.c
index 1cb6849b2231..e2c2aa424e4d 100644
--- a/drivers/infiniband/core/rdma_core.c
+++ b/drivers/infiniband/core/rdma_core.c
@@ -484,12 +484,13 @@ int rdma_explicit_destroy(struct ib_uobject *uobject)
 	ret = uobject->type->type_class->remove_commit(uobject,
 						       RDMA_REMOVE_DESTROY);
 	if (ret)
-		return ret;
+		goto out;
 
 	uobject->type = &null_obj_type;
 
+out:
 	up_read(&ucontext->cleanup_rwsem);
-	return 0;
+	return ret;
 }
 
 static void alloc_commit_idr_uobject(struct ib_uobject *uobj)
-- 
2.16.1

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH rdma-rc 15/15] RDMA/uverbs: Protect from command mask overflow

[Leon Romanovsky]

From: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>

The number of commands supported by the uverbs is less than 64, so let's
check it in advance that the supplied command is below that limit.

================================================================================
UBSAN: Undefined behaviour in
drivers/infiniband/core/uverbs_main.c:647:21
shift exponent 207 is too large for 64-bit type 'long long unsigned int'
CPU: 0 PID: 446 Comm: syz-executor3 Not tainted 4.15.0-rc2+ #61
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS
rel-1.7.5-0-ge51488c-20140602_164612-nilsson.home.kraxel.org 04/01/2014
Call Trace:
dump_stack+0xde/0x164
? dma_virt_map_sg+0x22c/0x22c
ubsan_epilogue+0xe/0x81
__ubsan_handle_shift_out_of_bounds+0x293/0x2f7
? debug_check_no_locks_freed+0x340/0x340
? __ubsan_handle_load_invalid_value+0x19b/0x19b
? lock_acquire+0x440/0x440
? lock_acquire+0x19d/0x440
? __might_fault+0xf4/0x240
? ib_uverbs_write+0x68d/0xe20
ib_uverbs_write+0x68d/0xe20
? __lock_acquire+0xcf7/0x3940
? uverbs_devnode+0x110/0x110
? cyc2ns_read_end+0x10/0x10
? sched_clock_cpu+0x18/0x200
? sched_clock_cpu+0x18/0x200
__vfs_write+0x10d/0x700
? uverbs_devnode+0x110/0x110
? kernel_read+0x170/0x170
? __fget+0x35b/0x5d0
? security_file_permission+0x93/0x260
vfs_write+0x1b0/0x550
SyS_write+0xc7/0x1a0
? SyS_read+0x1a0/0x1a0
? trace_hardirqs_on_thunk+0x1a/0x1c
entry_SYSCALL_64_fastpath+0x18/0x85
RIP: 0033:0x448e29
RSP: 002b:00007f033f567c58 EFLAGS: 00000246 ORIG_RAX: 0000000000000001
RAX: ffffffffffffffda RBX: 00007f033f5686bc RCX: 0000000000448e29
RDX: 0000000000000060 RSI: 0000000020001000 RDI: 0000000000000012
RBP: 000000000070bea0 R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000246 R12: 00000000ffffffff
R13: 00000000000056a0 R14: 00000000006e8740 R15: 0000000000000000
================================================================================

Cc: syzkaller <syzkaller-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org>
Cc: <stable-u79uwXL29TY76Z2rM5mHXA@public.gmane.org> # 4.5
Fixes: 2dbd5186a39c ("IB/core: IB/core: Allow legacy verbs through extended interfaces")
Reported-by: Noa Osherovich <noaos-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Reviewed-by: Matan Barak <matanb-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Signed-off-by: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
---
 drivers/infiniband/core/uverbs_main.c | 27 ++++++++++++++++++++-------
 1 file changed, 20 insertions(+), 7 deletions(-)

diff --git a/drivers/infiniband/core/uverbs_main.c b/drivers/infiniband/core/uverbs_main.c
index cd72555ad457..b36cb12b3f38 100644
--- a/drivers/infiniband/core/uverbs_main.c
+++ b/drivers/infiniband/core/uverbs_main.c
@@ -650,12 +650,21 @@ static int verify_command_mask(struct ib_device *ib_dev, __u32 command)
 	return -1;
 }
 
+static bool verify_command_idx(__u32 command, bool extended)
+{
+	if (extended)
+		return command < ARRAY_SIZE(uverbs_ex_cmd_table);
+
+	return command < ARRAY_SIZE(uverbs_cmd_table);
+}
+
 static ssize_t ib_uverbs_write(struct file *filp, const char __user *buf,
 			     size_t count, loff_t *pos)
 {
 	struct ib_uverbs_file *file = filp->private_data;
 	struct ib_device *ib_dev;
 	struct ib_uverbs_cmd_hdr hdr;
+	bool extended_command;
 	__u32 command;
 	__u32 flags;
 	int srcu_key;
@@ -688,6 +697,15 @@ static ssize_t ib_uverbs_write(struct file *filp, const char __user *buf,
 	}
 
 	command = hdr.command & IB_USER_VERBS_CMD_COMMAND_MASK;
+	flags = (hdr.command &
+		 IB_USER_VERBS_CMD_FLAGS_MASK) >> IB_USER_VERBS_CMD_FLAGS_SHIFT;
+
+	extended_command = flags & IB_USER_VERBS_CMD_FLAG_EXTENDED;
+	if (!verify_command_idx(command, extended_command)) {
+		ret = -EINVAL;
+		goto out;
+	}
+
 	if (verify_command_mask(ib_dev, command)) {
 		ret = -EOPNOTSUPP;
 		goto out;
@@ -699,12 +717,8 @@ static ssize_t ib_uverbs_write(struct file *filp, const char __user *buf,
 		goto out;
 	}
 
-	flags = (hdr.command &
-		 IB_USER_VERBS_CMD_FLAGS_MASK) >> IB_USER_VERBS_CMD_FLAGS_SHIFT;
-
 	if (!flags) {
-		if (command >= ARRAY_SIZE(uverbs_cmd_table) ||
-		    !uverbs_cmd_table[command]) {
+		if (!uverbs_cmd_table[command]) {
 			ret = -EINVAL;
 			goto out;
 		}
@@ -725,8 +739,7 @@ static ssize_t ib_uverbs_write(struct file *filp, const char __user *buf,
 		struct ib_udata uhw;
 		size_t written_count = count;
 
-		if (command >= ARRAY_SIZE(uverbs_ex_cmd_table) ||
-		    !uverbs_ex_cmd_table[command]) {
+		if (!uverbs_ex_cmd_table[command]) {
 			ret = -ENOSYS;
 			goto out;
 		}
-- 
2.16.1

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH rdma-rc 01/15] IB/ipoib: Do not warn if IPoIB debugfs doesn't exist

[Dennis Dalessandro]

On 2/13/2018 5:18 AM, Leon Romanovsky wrote:
>   	struct ipoib_dev_priv *priv = ipoib_priv(dev);
>   
> -	WARN_ONCE(!priv->mcg_dentry, "null mcg debug file\n");
> -	WARN_ONCE(!priv->path_dentry, "null path debug file\n");
>   	debugfs_remove(priv->mcg_dentry);
>   	debugfs_remove(priv->path_dentry);
>   	priv->mcg_dentry = priv->path_dentry = NULL;
> 

Do we really want to keep sending stuff like this for RC? I get that 
it's a trivial patch and all, but it's not actually fixing anything. 
Just a console spam that only happens once.

-Denny
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH rdma-rc 05/15] IB/uverbs: Use u64_to_user_ptr() not a union

[Dennis Dalessandro]

On 2/13/2018 5:18 AM, Leon Romanovsky wrote:
> From: Jason Gunthorpe <jgg-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> 
> The union approach will get the endianness wrong sometimes if the kernel's
> pointer size is 32 bits resulting in EFAULTs when trying to copy to/from
> user.
> 
> Signed-off-by: Jason Gunthorpe <jgg-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> Signed-off-by: Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> ---
>   drivers/infiniband/core/uverbs_std_types.c |  4 ++--
>   include/rdma/uverbs_ioctl.h                | 10 ++++------
>   2 files changed, 6 insertions(+), 8 deletions(-)

Reviewed-by: Dennis Dalessandro <dennis.dalessandro-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH rdma-rc 07/15] IB/uverbs: Use __aligned_u64 for uapi headers

[Dennis Dalessandro]

On 2/13/2018 5:18 AM, Leon Romanovsky wrote:
> From: Jason Gunthorpe <jgg-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> 
> This has no impact on the structure layout since these structs already
> have their u64s already properly aligned, but it does document that we
> have this requirement for 32 bit compatibility.
> 
> Signed-off-by: Jason Gunthorpe <jgg-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> Signed-off-by: Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>

Reviewed-by: Dennis Dalessandro <dennis.dalessandro-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH rdma-rc 08/15] IB/uverbs: Add ioctl support for 32bit processes

[Dennis Dalessandro]

On 2/13/2018 5:18 AM, Leon Romanovsky wrote:
> From: Matan Barak <matanb-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> 
> 32 bit processes running on a 64 bit kernel call compat_ioctl so that
> implementations can revise any structure layout issues. Point compat_ioctl
> at our normal ioctl because:
> 
> - All our structures are designed to be the same on 32 and 64 bit, ie we
>    use __aligned_u64 when required and are careful to manage padding.
> 
> - Any pointers are stored in u64's and userspace is expected
>    to prepare them properly.
> 
> Signed-off-by: Matan Barak <matanb-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> Signed-off-by: Jason Gunthorpe <jgg-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> Signed-off-by: Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> ---
>   drivers/infiniband/core/uverbs_main.c | 2 ++
>   1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/infiniband/core/uverbs_main.c b/drivers/infiniband/core/uverbs_main.c
> index 395a3b091229..cd72555ad457 100644
> --- a/drivers/infiniband/core/uverbs_main.c
> +++ b/drivers/infiniband/core/uverbs_main.c
> @@ -942,6 +942,7 @@ static const struct file_operations uverbs_fops = {
>   	.llseek	 = no_llseek,
>   #if IS_ENABLED(CONFIG_INFINIBAND_EXP_USER_ACCESS)
>   	.unlocked_ioctl = ib_uverbs_ioctl,
> +	.compat_ioctl = ib_uverbs_ioctl,
>   #endif
>   };
>   
> @@ -954,6 +955,7 @@ static const struct file_operations uverbs_mmap_fops = {
>   	.llseek	 = no_llseek,
>   #if IS_ENABLED(CONFIG_INFINIBAND_EXP_USER_ACCESS)
>   	.unlocked_ioctl = ib_uverbs_ioctl,
> +	.compat_ioctl = ib_uverbs_ioctl,
>   #endif
>   };
>   
> 

Is this patch really necessary? Doesn't sys_ioctl() just end up calling 
unlocked_ioctl() if .compat_ioctl is not set? I guess nothing wrong with 
being explicit here, but this is just a clean-up then, and not an -rc patch.

-Denny
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH rdma-rc 02/15] RDMA/restrack: Remove unimplemented XRCD object

[Dennis Dalessandro]

On 2/13/2018 5:18 AM, Leon Romanovsky wrote:
> From: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> 
> Resource tracking of XRCD objects is not implemented in current
> version of restrack and hence can be removed.
> 
> Fixes: 02d8883f520e ("RDMA/restrack: Add general infrastructure to track RDMA resources")
> Signed-off-by: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> ---
>   drivers/infiniband/core/restrack.c | 5 -----
>   include/rdma/restrack.h            | 4 ----
>   2 files changed, 9 deletions(-)
> 
> diff --git a/drivers/infiniband/core/restrack.c b/drivers/infiniband/core/restrack.c
> index 857637bf46da..d8dc709a3715 100644
> --- a/drivers/infiniband/core/restrack.c
> +++ b/drivers/infiniband/core/restrack.c
> @@ -63,7 +63,6 @@ static struct ib_device *res_to_dev(struct rdma_restrack_entry *res)
>   {
>   	enum rdma_restrack_type type = res->type;
>   	struct ib_device *dev;
> -	struct ib_xrcd *xrcd;
>   	struct ib_pd *pd;
>   	struct ib_cq *cq;
>   	struct ib_qp *qp;
> @@ -81,10 +80,6 @@ static struct ib_device *res_to_dev(struct rdma_restrack_entry *res)
>   		qp = container_of(res, struct ib_qp, res);
>   		dev = qp->device;
>   		break;
> -	case RDMA_RESTRACK_XRCD:
> -		xrcd = container_of(res, struct ib_xrcd, res);
> -		dev = xrcd->device;
> -		break;
>   	default:
>   		WARN_ONCE(true, "Wrong resource tracking type %u\n", type);
>   		return NULL;
> diff --git a/include/rdma/restrack.h b/include/rdma/restrack.h
> index c2d81167c858..2cdf8dcf4bdc 100644
> --- a/include/rdma/restrack.h
> +++ b/include/rdma/restrack.h
> @@ -28,10 +28,6 @@ enum rdma_restrack_type {
>   	 * @RDMA_RESTRACK_QP: Queue pair (QP)
>   	 */
>   	RDMA_RESTRACK_QP,
> -	/**
> -	 * @RDMA_RESTRACK_XRCD: XRC domain (XRCD)
> -	 */
> -	RDMA_RESTRACK_XRCD,
>   	/**
>   	 * @RDMA_RESTRACK_MAX: Last entry, used for array dclarations
>   	 */
> 

Not -rc material in my opinion.

-Denny
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH rdma-rc 13/15] IB/uverbs: Tidy uverbs_uobject_add

[Dennis Dalessandro]

On 2/13/2018 5:18 AM, Leon Romanovsky wrote:
> From: Jason Gunthorpe <jgg-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> 
> Maintaining the uobjects list is mandatory, hoist it into the common
> rdma_alloc_commit_uobject() function and inline it as there is now
> only one caller.
> 
> Signed-off-by: Jason Gunthorpe <jgg-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> Signed-off-by: Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> ---
>   drivers/infiniband/core/rdma_core.c | 13 ++++---------
>   1 file changed, 4 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/infiniband/core/rdma_core.c b/drivers/infiniband/core/rdma_core.c
> index 7aee1c0b9884..1cb6849b2231 100644
> --- a/drivers/infiniband/core/rdma_core.c
> +++ b/drivers/infiniband/core/rdma_core.c
> @@ -350,13 +350,6 @@ struct ib_uobject *rdma_alloc_begin_uobject(const struct uverbs_obj_type *type,
>   	return type->type_class->alloc_begin(type, ucontext);
>   }
>   
> -static void uverbs_uobject_add(struct ib_uobject *uobject)
> -{
> -	mutex_lock(&uobject->context->uobjects_lock);
> -	list_add(&uobject->list, &uobject->context->uobjects);
> -	mutex_unlock(&uobject->context->uobjects_lock);
> -}
> -
>   static int __must_check remove_commit_idr_uobject(struct ib_uobject *uobj,
>   						  enum rdma_remove_reason why)
>   {
> @@ -501,7 +494,6 @@ int rdma_explicit_destroy(struct ib_uobject *uobject)
>   
>   static void alloc_commit_idr_uobject(struct ib_uobject *uobj)
>   {
> -	uverbs_uobject_add(uobj);
>   	spin_lock(&uobj->context->ufile->idr_lock);
>   	/*
>   	 * We already allocated this IDR with a NULL object, so
> @@ -517,7 +509,6 @@ static void alloc_commit_fd_uobject(struct ib_uobject *uobj)
>   	struct ib_uobject_file *uobj_file =
>   		container_of(uobj, struct ib_uobject_file, uobj);
>   
> -	uverbs_uobject_add(&uobj_file->uobj);
>   	fd_install(uobj_file->uobj.id, uobj->object);
>   	/* This shouldn't be used anymore. Use the file object instead */
>   	uobj_file->uobj.id = 0;
> @@ -544,6 +535,10 @@ int rdma_alloc_commit_uobject(struct ib_uobject *uobj)
>   	assert_uverbs_usecnt(uobj, true);
>   	atomic_set(&uobj->usecnt, 0);
>   
> +	mutex_lock(&uobj->context->uobjects_lock);
> +	list_add(&uobj->list, &uobj->context->uobjects);
> +	mutex_unlock(&uobj->context->uobjects_lock);
> +
>   	uobj->type->type_class->alloc_commit(uobj);
>   	up_read(&uobj->context->cleanup_rwsem);
>   
> 

 From the subject it sounds like you are doing a cosmetic clean-up, but 
this is doing a bit more. You are ensuring the list_add is always going 
to be called by putting it in the common path. What about making that be 
the subject?

-Denny
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH rdma-rc 12/15] IB/uverbs: Tidy lockdep_check

[Dennis Dalessandro]

On 2/13/2018 5:18 AM, Leon Romanovsky wrote:
> From: Jason Gunthorpe <jgg-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> 
> This is really being used as an asssert that the expected usecnt
> is being held and implicitly that the usecnt is valid. Rename it to
> assert_uverbs_usecnt and tighten the checks to only accept valid
> values of usecnt (eg 0 and < -1 are invalid).
> 
> Fixes: 3832125624b7 ("IB/core: Add support for idr types")
> Signed-off-by: Jason Gunthorpe <jgg-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> Signed-off-by: Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> ---
>   drivers/infiniband/core/rdma_core.c | 14 +++++++-------
>   1 file changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/infiniband/core/rdma_core.c b/drivers/infiniband/core/rdma_core.c
> index 3ec5c7926f1c..7aee1c0b9884 100644
> --- a/drivers/infiniband/core/rdma_core.c
> +++ b/drivers/infiniband/core/rdma_core.c
> @@ -412,13 +412,13 @@ static int __must_check remove_commit_fd_uobject(struct ib_uobject *uobj,
>   	return ret;
>   }
>   
> -static void lockdep_check(struct ib_uobject *uobj, bool exclusive)
> +static void assert_uverbs_usecnt(struct ib_uobject *uobj, bool exclusive)
>   {
>   #ifdef CONFIG_LOCKDEP
>   	if (exclusive)
> -		WARN_ON(atomic_read(&uobj->usecnt) > 0);
> +		WARN_ON(atomic_read(&uobj->usecnt) != -1);
>   	else
> -		WARN_ON(atomic_read(&uobj->usecnt) == -1);
> +		WARN_ON(atomic_read(&uobj->usecnt) <= 0);
>   #endif
>   }
>   
> @@ -457,7 +457,7 @@ int __must_check rdma_remove_commit_uobject(struct ib_uobject *uobj)
>   		WARN(true, "ib_uverbs: Cleanup is running while removing an uobject\n");
>   		return 0;
>   	}
> -	lockdep_check(uobj, true);
> +	assert_uverbs_usecnt(uobj, true);
>   	ret = _rdma_remove_commit_uobject(uobj, RDMA_REMOVE_DESTROY);
>   
>   	up_read(&ucontext->cleanup_rwsem);
> @@ -487,7 +487,7 @@ int rdma_explicit_destroy(struct ib_uobject *uobject)
>   		WARN(true, "ib_uverbs: Cleanup is running while removing an uobject\n");
>   		return 0;
>   	}
> -	lockdep_check(uobject, true);
> +	assert_uverbs_usecnt(uobject, true);
>   	ret = uobject->type->type_class->remove_commit(uobject,
>   						       RDMA_REMOVE_DESTROY);
>   	if (ret)
> @@ -541,7 +541,7 @@ int rdma_alloc_commit_uobject(struct ib_uobject *uobj)
>   	}
>   
>   	/* matches atomic_set(-1) in alloc_uobj */
> -	lockdep_check(uobj, true);
> +	assert_uverbs_usecnt(uobj, true);
>   	atomic_set(&uobj->usecnt, 0);
>   
>   	uobj->type->type_class->alloc_commit(uobj);
> @@ -578,7 +578,7 @@ static void lookup_put_fd_uobject(struct ib_uobject *uobj, bool exclusive)
>   
>   void rdma_lookup_put_uobject(struct ib_uobject *uobj, bool exclusive)
>   {
> -	lockdep_check(uobj, exclusive);
> +	assert_uverbs_usecnt(uobj, exclusive);
>   	uobj->type->type_class->lookup_put(uobj, exclusive);
>   	/*
>   	 * In order to unlock an object, either decrease its usecnt for
> 

-rc material?
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH rdma-rc 14/15] IB/uverbs: Fix unbalanced unlock on error path for rdma_explicit_destroy

[Dennis Dalessandro]

On 2/13/2018 5:18 AM, Leon Romanovsky wrote:
> From: Jason Gunthorpe <jgg-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> 
> If remove_commit fails then the lock is left locked while the uobj still
> exists. Eventually the kernel will deadlock.
> 
> lockdep detects this and says:
> 
>   test/4221 is leaving the kernel with locks still held!
>   1 lock held by test/4221:
>    #0:  (&ucontext->cleanup_rwsem){.+.+}, at: [<000000001e5c7523>] rdma_explicit_destroy+0x37/0x120 [ib_uverbs]
> 
> Fixes: 4da70da23e9b ("IB/core: Explicitly destroy an object while keeping uobject")
> Signed-off-by: Jason Gunthorpe <jgg-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> Signed-off-by: Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>

Reviewed-by: Dennis Dalessandro <dennis.dalessandro-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH rdma-rc 08/15] IB/uverbs: Add ioctl support for 32bit processes

[Jason Gunthorpe]

On Tue, Feb 13, 2018 at 11:56:47AM -0500, Dennis Dalessandro wrote:
> On 2/13/2018 5:18 AM, Leon Romanovsky wrote:
> >From: Matan Barak <matanb-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> >
> >32 bit processes running on a 64 bit kernel call compat_ioctl so that
> >implementations can revise any structure layout issues. Point compat_ioctl
> >at our normal ioctl because:
> >
> >- All our structures are designed to be the same on 32 and 64 bit, ie we
> >   use __aligned_u64 when required and are careful to manage padding.
> >
> >- Any pointers are stored in u64's and userspace is expected
> >   to prepare them properly.
> >
> >Signed-off-by: Matan Barak <matanb-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> >Signed-off-by: Jason Gunthorpe <jgg-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> >Signed-off-by: Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> >  drivers/infiniband/core/uverbs_main.c | 2 ++
> >  1 file changed, 2 insertions(+)
> >
> >diff --git a/drivers/infiniband/core/uverbs_main.c b/drivers/infiniband/core/uverbs_main.c
> >index 395a3b091229..cd72555ad457 100644
> >+++ b/drivers/infiniband/core/uverbs_main.c
> >@@ -942,6 +942,7 @@ static const struct file_operations uverbs_fops = {
> >  	.llseek	 = no_llseek,
> >  #if IS_ENABLED(CONFIG_INFINIBAND_EXP_USER_ACCESS)
> >  	.unlocked_ioctl = ib_uverbs_ioctl,
> >+	.compat_ioctl = ib_uverbs_ioctl,
> >  #endif
> >  };
> >@@ -954,6 +955,7 @@ static const struct file_operations uverbs_mmap_fops = {
> >  	.llseek	 = no_llseek,
> >  #if IS_ENABLED(CONFIG_INFINIBAND_EXP_USER_ACCESS)
> >  	.unlocked_ioctl = ib_uverbs_ioctl,
> >+	.compat_ioctl = ib_uverbs_ioctl,
> >  #endif
> >  };
> >
> 
> Is this patch really necessary?

Yes. Without it 32 bit gets:

ioctl(3, _IOC(_IOC_READ|_IOC_WRITE, 0x1b, 0x1, 0x10), 0xffcaf5f8) = -1 ENOTTY (Inappropriate ioctl for device)

> Doesn't sys_ioctl() just end up calling unlocked_ioctl() if
> .compat_ioctl is not set? I guess nothing wrong with being explicit
> here, but this is just a clean-up then, and not an -rc patch.

No, the control flow is really tortured in compat_ioctl.c, but it
will only call compat_ioctl for our case.

Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH rdma-rc 13/15] IB/uverbs: Tidy uverbs_uobject_add

[Jason Gunthorpe]

On Tue, Feb 13, 2018 at 12:09:55PM -0500, Dennis Dalessandro wrote:
> On 2/13/2018 5:18 AM, Leon Romanovsky wrote:
> >From: Jason Gunthorpe <jgg-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> >
> >Maintaining the uobjects list is mandatory, hoist it into the common
> >rdma_alloc_commit_uobject() function and inline it as there is now
> >only one caller.
> >
> >Signed-off-by: Jason Gunthorpe <jgg-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> >Signed-off-by: Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> >  drivers/infiniband/core/rdma_core.c | 13 ++++---------
> >  1 file changed, 4 insertions(+), 9 deletions(-)
> >
> >diff --git a/drivers/infiniband/core/rdma_core.c b/drivers/infiniband/core/rdma_core.c
> >index 7aee1c0b9884..1cb6849b2231 100644
> >+++ b/drivers/infiniband/core/rdma_core.c
> >@@ -350,13 +350,6 @@ struct ib_uobject *rdma_alloc_begin_uobject(const struct uverbs_obj_type *type,
> >  	return type->type_class->alloc_begin(type, ucontext);
> >  }
> >-static void uverbs_uobject_add(struct ib_uobject *uobject)
> >-{
> >-	mutex_lock(&uobject->context->uobjects_lock);
> >-	list_add(&uobject->list, &uobject->context->uobjects);
> >-	mutex_unlock(&uobject->context->uobjects_lock);
> >-}
> >-
> >  static int __must_check remove_commit_idr_uobject(struct ib_uobject *uobj,
> >  						  enum rdma_remove_reason why)
> >  {
> >@@ -501,7 +494,6 @@ int rdma_explicit_destroy(struct ib_uobject *uobject)
> >  static void alloc_commit_idr_uobject(struct ib_uobject *uobj)
> >  {
> >-	uverbs_uobject_add(uobj);
> >  	spin_lock(&uobj->context->ufile->idr_lock);
> >  	/*
> >  	 * We already allocated this IDR with a NULL object, so
> >@@ -517,7 +509,6 @@ static void alloc_commit_fd_uobject(struct ib_uobject *uobj)
> >  	struct ib_uobject_file *uobj_file =
> >  		container_of(uobj, struct ib_uobject_file, uobj);
> >-	uverbs_uobject_add(&uobj_file->uobj);
> >  	fd_install(uobj_file->uobj.id, uobj->object);
> >  	/* This shouldn't be used anymore. Use the file object instead */
> >  	uobj_file->uobj.id = 0;
> >@@ -544,6 +535,10 @@ int rdma_alloc_commit_uobject(struct ib_uobject *uobj)
> >  	assert_uverbs_usecnt(uobj, true);
> >  	atomic_set(&uobj->usecnt, 0);
> >+	mutex_lock(&uobj->context->uobjects_lock);
> >+	list_add(&uobj->list, &uobj->context->uobjects);
> >+	mutex_unlock(&uobj->context->uobjects_lock);
> >+
> >  	uobj->type->type_class->alloc_commit(uobj);
> >  	up_read(&uobj->context->cleanup_rwsem);
> >
> 
> From the subject it sounds like you are doing a cosmetic clean-up, but this
> is doing a bit more.

The behavior is identical before/after.

There are only two implementations of alloc_commit and both of them do
the list_add as the first operation.

Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH rdma-rc 12/15] IB/uverbs: Tidy lockdep_check

[Jason Gunthorpe]

On Tue, Feb 13, 2018 at 12:10:35PM -0500, Dennis Dalessandro wrote:
> On 2/13/2018 5:18 AM, Leon Romanovsky wrote:
> >From: Jason Gunthorpe <jgg-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> >
> >This is really being used as an asssert that the expected usecnt
> >is being held and implicitly that the usecnt is valid. Rename it to
> >assert_uverbs_usecnt and tighten the checks to only accept valid
> >values of usecnt (eg 0 and < -1 are invalid).
> >
> >Fixes: 3832125624b7 ("IB/core: Add support for idr types")
> >Signed-off-by: Jason Gunthorpe <jgg-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> >Signed-off-by: Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> >  drivers/infiniband/core/rdma_core.c | 14 +++++++-------
> >  1 file changed, 7 insertions(+), 7 deletions(-)
> >
> >diff --git a/drivers/infiniband/core/rdma_core.c b/drivers/infiniband/core/rdma_core.c
> >index 3ec5c7926f1c..7aee1c0b9884 100644
> >+++ b/drivers/infiniband/core/rdma_core.c
> >@@ -412,13 +412,13 @@ static int __must_check remove_commit_fd_uobject(struct ib_uobject *uobj,
> >  	return ret;
> >  }
> >-static void lockdep_check(struct ib_uobject *uobj, bool exclusive)
> >+static void assert_uverbs_usecnt(struct ib_uobject *uobj, bool exclusive)
> >  {
> >  #ifdef CONFIG_LOCKDEP
> >  	if (exclusive)
> >-		WARN_ON(atomic_read(&uobj->usecnt) > 0);
> >+		WARN_ON(atomic_read(&uobj->usecnt) != -1);
> >  	else
> >-		WARN_ON(atomic_read(&uobj->usecnt) == -1);
> >+		WARN_ON(atomic_read(&uobj->usecnt) <= 0);
> >  #endif
> >  }
> >@@ -457,7 +457,7 @@ int __must_check rdma_remove_commit_uobject(struct ib_uobject *uobj)
> >  		WARN(true, "ib_uverbs: Cleanup is running while removing an uobject\n");
> >  		return 0;
> >  	}
> >-	lockdep_check(uobj, true);
> >+	assert_uverbs_usecnt(uobj, true);
> >  	ret = _rdma_remove_commit_uobject(uobj, RDMA_REMOVE_DESTROY);
> >  	up_read(&ucontext->cleanup_rwsem);
> >@@ -487,7 +487,7 @@ int rdma_explicit_destroy(struct ib_uobject *uobject)
> >  		WARN(true, "ib_uverbs: Cleanup is running while removing an uobject\n");
> >  		return 0;
> >  	}
> >-	lockdep_check(uobject, true);
> >+	assert_uverbs_usecnt(uobject, true);
> >  	ret = uobject->type->type_class->remove_commit(uobject,
> >  						       RDMA_REMOVE_DESTROY);
> >  	if (ret)
> >@@ -541,7 +541,7 @@ int rdma_alloc_commit_uobject(struct ib_uobject *uobj)
> >  	}
> >  	/* matches atomic_set(-1) in alloc_uobj */
> >-	lockdep_check(uobj, true);
> >+	assert_uverbs_usecnt(uobj, true);
> >  	atomic_set(&uobj->usecnt, 0);
> >  	uobj->type->type_class->alloc_commit(uobj);
> >@@ -578,7 +578,7 @@ static void lookup_put_fd_uobject(struct ib_uobject *uobj, bool exclusive)
> >  void rdma_lookup_put_uobject(struct ib_uobject *uobj, bool exclusive)
> >  {
> >-	lockdep_check(uobj, exclusive);
> >+	assert_uverbs_usecnt(uobj, exclusive);
> >  	uobj->type->type_class->lookup_put(uobj, exclusive);
> >  	/*
> >  	 * In order to unlock an object, either decrease its usecnt for
> >
> 
> -rc material?

This one is a bit border line - but as coded the asserts were not
working right, eg:

 -		WARN_ON(atomic_read(&uobj->usecnt) > 0);
 +		WARN_ON(atomic_read(&uobj->usecnt) != -1);

Didn't trigger if usecnt == 0, which is wrong.

Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH rdma-rc 02/15] RDMA/restrack: Remove unimplemented XRCD object

[Leon Romanovsky]

[-- Attachment #1: Type: text/plain, Size: 2218 bytes --]

On Tue, Feb 13, 2018 at 12:00:25PM -0500, Dennis Dalessandro wrote:
> On 2/13/2018 5:18 AM, Leon Romanovsky wrote:
> > From: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> >
> > Resource tracking of XRCD objects is not implemented in current
> > version of restrack and hence can be removed.
> >
> > Fixes: 02d8883f520e ("RDMA/restrack: Add general infrastructure to track RDMA resources")
> > Signed-off-by: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> > ---
> >   drivers/infiniband/core/restrack.c | 5 -----
> >   include/rdma/restrack.h            | 4 ----
> >   2 files changed, 9 deletions(-)
> >
> > diff --git a/drivers/infiniband/core/restrack.c b/drivers/infiniband/core/restrack.c
> > index 857637bf46da..d8dc709a3715 100644
> > --- a/drivers/infiniband/core/restrack.c
> > +++ b/drivers/infiniband/core/restrack.c
> > @@ -63,7 +63,6 @@ static struct ib_device *res_to_dev(struct rdma_restrack_entry *res)
> >   {
> >   	enum rdma_restrack_type type = res->type;
> >   	struct ib_device *dev;
> > -	struct ib_xrcd *xrcd;
> >   	struct ib_pd *pd;
> >   	struct ib_cq *cq;
> >   	struct ib_qp *qp;
> > @@ -81,10 +80,6 @@ static struct ib_device *res_to_dev(struct rdma_restrack_entry *res)
> >   		qp = container_of(res, struct ib_qp, res);
> >   		dev = qp->device;
> >   		break;
> > -	case RDMA_RESTRACK_XRCD:
> > -		xrcd = container_of(res, struct ib_xrcd, res);
> > -		dev = xrcd->device;
> > -		break;
> >   	default:
> >   		WARN_ONCE(true, "Wrong resource tracking type %u\n", type);
> >   		return NULL;
> > diff --git a/include/rdma/restrack.h b/include/rdma/restrack.h
> > index c2d81167c858..2cdf8dcf4bdc 100644
> > --- a/include/rdma/restrack.h
> > +++ b/include/rdma/restrack.h
> > @@ -28,10 +28,6 @@ enum rdma_restrack_type {
> >   	 * @RDMA_RESTRACK_QP: Queue pair (QP)
> >   	 */
> >   	RDMA_RESTRACK_QP,
> > -	/**
> > -	 * @RDMA_RESTRACK_XRCD: XRC domain (XRCD)
> > -	 */
> > -	RDMA_RESTRACK_XRCD,
> >   	/**
> >   	 * @RDMA_RESTRACK_MAX: Last entry, used for array dclarations
> >   	 */
> >
>
> Not -rc material in my opinion.

We are in -rc1 and it is exactly the time to clean up the code which was
entered a couple of weeks before.

>
> -Denny

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH rdma-rc 01/15] IB/ipoib: Do not warn if IPoIB debugfs doesn't exist

[Leon Romanovsky]

[-- Attachment #1: Type: text/plain, Size: 734 bytes --]

On Tue, Feb 13, 2018 at 11:06:02AM -0500, Dennis Dalessandro wrote:
> On 2/13/2018 5:18 AM, Leon Romanovsky wrote:
> >   	struct ipoib_dev_priv *priv = ipoib_priv(dev);
> > -	WARN_ONCE(!priv->mcg_dentry, "null mcg debug file\n");
> > -	WARN_ONCE(!priv->path_dentry, "null path debug file\n");
> >   	debugfs_remove(priv->mcg_dentry);
> >   	debugfs_remove(priv->path_dentry);
> >   	priv->mcg_dentry = priv->path_dentry = NULL;
> >
>
> Do we really want to keep sending stuff like this for RC? I get that it's a
> trivial patch and all, but it's not actually fixing anything. Just a console
> spam that only happens once.

It is more than simple console spam, but actual crash on the system with
warning == panic.

Thanks

>
> -Denny

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH rdma-rc 02/15] RDMA/restrack: Remove unimplemented XRCD object

[Dennis Dalessandro]

On 2/13/2018 1:09 PM, Leon Romanovsky wrote:

>>
>> Not -rc material in my opinion.
> 
> We are in -rc1 and it is exactly the time to clean up the code which was
> entered a couple of weeks before.

Ah if that's the case then yes I totally agree.

-Denny
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH rdma-rc 13/15] IB/uverbs: Tidy uverbs_uobject_add

[Dennis Dalessandro]

On 2/13/2018 12:20 PM, Jason Gunthorpe wrote:
> On Tue, Feb 13, 2018 at 12:09:55PM -0500, Dennis Dalessandro wrote:
>> On 2/13/2018 5:18 AM, Leon Romanovsky wrote:
>>> From: Jason Gunthorpe <jgg-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
>>>
>>> Maintaining the uobjects list is mandatory, hoist it into the common
>>> rdma_alloc_commit_uobject() function and inline it as there is now
>>> only one caller.
>>>
>>> Signed-off-by: Jason Gunthorpe <jgg-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
>>> Signed-off-by: Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
>>>   drivers/infiniband/core/rdma_core.c | 13 ++++---------
>>>   1 file changed, 4 insertions(+), 9 deletions(-)
>>>
>>> diff --git a/drivers/infiniband/core/rdma_core.c b/drivers/infiniband/core/rdma_core.c
>>> index 7aee1c0b9884..1cb6849b2231 100644
>>> +++ b/drivers/infiniband/core/rdma_core.c
>>> @@ -350,13 +350,6 @@ struct ib_uobject *rdma_alloc_begin_uobject(const struct uverbs_obj_type *type,
>>>   	return type->type_class->alloc_begin(type, ucontext);
>>>   }
>>> -static void uverbs_uobject_add(struct ib_uobject *uobject)
>>> -{
>>> -	mutex_lock(&uobject->context->uobjects_lock);
>>> -	list_add(&uobject->list, &uobject->context->uobjects);
>>> -	mutex_unlock(&uobject->context->uobjects_lock);
>>> -}
>>> -
>>>   static int __must_check remove_commit_idr_uobject(struct ib_uobject *uobj,
>>>   						  enum rdma_remove_reason why)
>>>   {
>>> @@ -501,7 +494,6 @@ int rdma_explicit_destroy(struct ib_uobject *uobject)
>>>   static void alloc_commit_idr_uobject(struct ib_uobject *uobj)
>>>   {
>>> -	uverbs_uobject_add(uobj);
>>>   	spin_lock(&uobj->context->ufile->idr_lock);
>>>   	/*
>>>   	 * We already allocated this IDR with a NULL object, so
>>> @@ -517,7 +509,6 @@ static void alloc_commit_fd_uobject(struct ib_uobject *uobj)
>>>   	struct ib_uobject_file *uobj_file =
>>>   		container_of(uobj, struct ib_uobject_file, uobj);
>>> -	uverbs_uobject_add(&uobj_file->uobj);
>>>   	fd_install(uobj_file->uobj.id, uobj->object);
>>>   	/* This shouldn't be used anymore. Use the file object instead */
>>>   	uobj_file->uobj.id = 0;
>>> @@ -544,6 +535,10 @@ int rdma_alloc_commit_uobject(struct ib_uobject *uobj)
>>>   	assert_uverbs_usecnt(uobj, true);
>>>   	atomic_set(&uobj->usecnt, 0);
>>> +	mutex_lock(&uobj->context->uobjects_lock);
>>> +	list_add(&uobj->list, &uobj->context->uobjects);
>>> +	mutex_unlock(&uobj->context->uobjects_lock);
>>> +
>>>   	uobj->type->type_class->alloc_commit(uobj);
>>>   	up_read(&uobj->context->cleanup_rwsem);
>>>
>>
>>  From the subject it sounds like you are doing a cosmetic clean-up, but this
>> is doing a bit more.
> 
> The behavior is identical before/after.
> 
> There are only two implementations of alloc_commit and both of them do
> the list_add as the first operation.

Agree. To me the value of this patch is that you remove two separate 
function calls and do the work in a common place. That's more important 
than to just "Tidy" up the code. Either way I'm totally fine with the patch.

Reviewed-by: Dennis Dalessandro <dennis.dalessandro-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH rdma-rc 01/15] IB/ipoib: Do not warn if IPoIB debugfs doesn't exist

[Dennis Dalessandro]

On 2/13/2018 1:12 PM, Leon Romanovsky wrote:
> On Tue, Feb 13, 2018 at 11:06:02AM -0500, Dennis Dalessandro wrote:
>> On 2/13/2018 5:18 AM, Leon Romanovsky wrote:
>>>    	struct ipoib_dev_priv *priv = ipoib_priv(dev);
>>> -	WARN_ONCE(!priv->mcg_dentry, "null mcg debug file\n");
>>> -	WARN_ONCE(!priv->path_dentry, "null path debug file\n");
>>>    	debugfs_remove(priv->mcg_dentry);
>>>    	debugfs_remove(priv->path_dentry);
>>>    	priv->mcg_dentry = priv->path_dentry = NULL;
>>>
>>
>> Do we really want to keep sending stuff like this for RC? I get that it's a
>> trivial patch and all, but it's not actually fixing anything. Just a console
>> spam that only happens once.
> 
> It is more than simple console spam, but actual crash on the system with
> warning == panic.

Ok. I'll buy that.

Reviewed-by: Dennis Dalessandro <dennis.dalessandro-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH rdma-rc 08/15] IB/uverbs: Add ioctl support for 32bit processes

[Dennis Dalessandro]

On 2/13/2018 12:16 PM, Jason Gunthorpe wrote:
> On Tue, Feb 13, 2018 at 11:56:47AM -0500, Dennis Dalessandro wrote:
>> On 2/13/2018 5:18 AM, Leon Romanovsky wrote:
>>> From: Matan Barak <matanb-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
>>>
>>> 32 bit processes running on a 64 bit kernel call compat_ioctl so that
>>> implementations can revise any structure layout issues. Point compat_ioctl
>>> at our normal ioctl because:
>>>
>>> - All our structures are designed to be the same on 32 and 64 bit, ie we
>>>    use __aligned_u64 when required and are careful to manage padding.
>>>
>>> - Any pointers are stored in u64's and userspace is expected
>>>    to prepare them properly.
>>>
>>> Signed-off-by: Matan Barak <matanb-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
>>> Signed-off-by: Jason Gunthorpe <jgg-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
>>> Signed-off-by: Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
>>>   drivers/infiniband/core/uverbs_main.c | 2 ++
>>>   1 file changed, 2 insertions(+)
>>>
>>> diff --git a/drivers/infiniband/core/uverbs_main.c b/drivers/infiniband/core/uverbs_main.c
>>> index 395a3b091229..cd72555ad457 100644
>>> +++ b/drivers/infiniband/core/uverbs_main.c
>>> @@ -942,6 +942,7 @@ static const struct file_operations uverbs_fops = {
>>>   	.llseek	 = no_llseek,
>>>   #if IS_ENABLED(CONFIG_INFINIBAND_EXP_USER_ACCESS)
>>>   	.unlocked_ioctl = ib_uverbs_ioctl,
>>> +	.compat_ioctl = ib_uverbs_ioctl,
>>>   #endif
>>>   };
>>> @@ -954,6 +955,7 @@ static const struct file_operations uverbs_mmap_fops = {
>>>   	.llseek	 = no_llseek,
>>>   #if IS_ENABLED(CONFIG_INFINIBAND_EXP_USER_ACCESS)
>>>   	.unlocked_ioctl = ib_uverbs_ioctl,
>>> +	.compat_ioctl = ib_uverbs_ioctl,
>>>   #endif
>>>   };
>>>
>>
>> Is this patch really necessary?
> 
> Yes. Without it 32 bit gets:
> 
> ioctl(3, _IOC(_IOC_READ|_IOC_WRITE, 0x1b, 0x1, 0x10), 0xffcaf5f8) = -1 ENOTTY (Inappropriate ioctl for device)
> 
>> Doesn't sys_ioctl() just end up calling unlocked_ioctl() if
>> .compat_ioctl is not set? I guess nothing wrong with being explicit
>> here, but this is just a clean-up then, and not an -rc patch.
> 
> No, the control flow is really tortured in compat_ioctl.c, but it
> will only call compat_ioctl for our case.

Ok, thanks, I'll go take another look.

-Denny

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH rdma-rc 00/15] RDMA fixes for v4.16

[Jason Gunthorpe]

On Tue, Feb 13, 2018 at 12:18:26PM +0200, Leon Romanovsky wrote:
> Hi,
> 
> This is mostly fixes for uverbs layers with two extra patches: one from
> Alaa, who removed annoying warning prints in IPoIB and another from Leon
> who removed unimplented part of restrack code.
> 
> Thanks
> 
> Alaa Hleihel (1):
>   IB/ipoib: Do not warn if IPoIB debugfs doesn't exist
> 
> Jason Gunthorpe (7):
>   IB/uverbs: Use inline data transfer for UHW_IN
>   IB/uverbs: Use u64_to_user_ptr() not a union
>   IB/uverbs: Use __aligned_u64 for uapi headers
>   IB/uverbs: Hold the uobj write lock after allocate
>   IB/uverbs: Tidy lockdep_check
>   IB/uverbs: Tidy uverbs_uobject_add
>   IB/uverbs: Fix unbalanced unlock on error path for
>     rdma_explicit_destroy
> 
> Leon Romanovsky (3):
>   RDMA/restrack: Remove unimplemented XRCD object
>   RDMA/uverbs: Protect from races between lookup and destroy of uobjects
>   RDMA/uverbs: Protect from command mask overflow
> 
> Matan Barak (4):
>   IB/uverbs: Always the attribute size provided by the user
>   IB/uverbs: Fix method merging in uverbs_ioctl_merge
>   IB/uverbs: Add ioctl support for 32bit processes
>   IB/uverbs: Fix possible oops with duplicate ioctl attributes

I applied all of these to for-rc, with some modifications to the
commit messages. Thanks

Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH rdma-rc 00/15] RDMA fixes for v4.16

[Jason Gunthorpe]

On Tue, Feb 13, 2018 at 12:18:26PM +0200, Leon Romanovsky wrote:
>   IB/uverbs: Tidy uverbs_uobject_add

Actually, I was writing my notes for the PR and I just can't defend
this one as -rc, especially since I wrote it, so to -next it goes.

Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html