* [PATCH 1/2] [v2] IB/uverbs: clean up INIT_UDATA_BUF_OR_NULL usage
@ 2017-09-06 21:34 Arnd Bergmann
2017-09-06 21:34 ` [PATCH 2/2] IB/uverbs: clean up INIT_UDATA() macro usage Arnd Bergmann
2017-09-27 13:09 ` [PATCH 1/2] [v2] IB/uverbs: clean up INIT_UDATA_BUF_OR_NULL usage Doug Ledford
0 siblings, 2 replies; 5+ messages in thread
From: Arnd Bergmann @ 2017-09-06 21:34 UTC (permalink / raw)
To: Doug Ledford, Sean Hefty, Hal Rosenstock
Cc: Christoph Hellwig, Arnd Bergmann, Matan Barak, Yishai Hadas,
Leon Romanovsky, linux-rdma, linux-kernel
We get a harmless warning about the fact that we use the result of a
multiplication as a condition:
drivers/infiniband/core/uverbs_main.c: In function 'ib_uverbs_write':
drivers/infiniband/core/uverbs_main.c:787:40: error: '*' in boolean context, suggest '&&' instead [-Werror=int-in-bool-context]
drivers/infiniband/core/uverbs_main.c:787:117: error: '*' in boolean context, suggest '&&' instead [-Werror=int-in-bool-context]
drivers/infiniband/core/uverbs_main.c:790:50: error: '*' in boolean context, suggest '&&' instead [-Werror=int-in-bool-context]
drivers/infiniband/core/uverbs_main.c:790:151: error: '*' in boolean context, suggest '&&' instead [-Werror=int-in-bool-context]
This avoids the problem by using an inline function in place of
the macro.
Fixes: a96e4e2ffe43 ("IB/uverbs: New macro to set pointers to NULL if length is 0 in INIT_UDATA()")
Suggested-by: Christoph Hellwig <hch@infradead.org>
Link: https://patchwork.kernel.org/patch/9940777/
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
v2: completely rewrite the patch to a larger cleanup.
---
drivers/infiniband/core/uverbs.h | 15 ++++++++-------
drivers/infiniband/core/uverbs_main.c | 22 ++++++++++------------
drivers/infiniband/core/uverbs_std_types.c | 3 ++-
3 files changed, 20 insertions(+), 20 deletions(-)
diff --git a/drivers/infiniband/core/uverbs.h b/drivers/infiniband/core/uverbs.h
index 37c8903e7fd0..a8f104a4a91b 100644
--- a/drivers/infiniband/core/uverbs.h
+++ b/drivers/infiniband/core/uverbs.h
@@ -55,13 +55,14 @@
(udata)->outlen = (olen); \
} while (0)
-#define INIT_UDATA_BUF_OR_NULL(udata, ibuf, obuf, ilen, olen) \
- do { \
- (udata)->inbuf = (ilen) ? (const void __user *) (ibuf) : NULL; \
- (udata)->outbuf = (olen) ? (void __user *) (obuf) : NULL; \
- (udata)->inlen = (ilen); \
- (udata)->outlen = (olen); \
- } while (0)
+static inline void
+ib_uverbs_init_udata_buf_or_null(struct ib_udata *udata,
+ const void __user *ibuf,
+ void __user *obuf,
+ size_t ilen, size_t olen)
+{
+ INIT_UDATA(udata, ilen ? ibuf : NULL, olen ? obuf : NULL, ilen, olen);
+}
/*
* Our lifetime rules for these structs are the following:
diff --git a/drivers/infiniband/core/uverbs_main.c b/drivers/infiniband/core/uverbs_main.c
index dc2aed6fb21b..b5febfd84ee5 100644
--- a/drivers/infiniband/core/uverbs_main.c
+++ b/drivers/infiniband/core/uverbs_main.c
@@ -763,7 +763,7 @@ static ssize_t ib_uverbs_write(struct file *filp, const char __user *buf,
}
if (!access_ok(VERIFY_WRITE,
- (void __user *) (unsigned long) ex_hdr.response,
+ u64_to_user_ptr(ex_hdr.response),
(hdr.out_words + ex_hdr.provider_out_words) * 8)) {
ret = -EFAULT;
goto out;
@@ -775,19 +775,17 @@ static ssize_t ib_uverbs_write(struct file *filp, const char __user *buf,
}
}
- INIT_UDATA_BUF_OR_NULL(&ucore, buf, (unsigned long) ex_hdr.response,
- hdr.in_words * 8, hdr.out_words * 8);
+ ib_uverbs_init_udata_buf_or_null(&ucore, buf,
+ u64_to_user_ptr(ex_hdr.response),
+ hdr.in_words * 8, hdr.out_words * 8);
- INIT_UDATA_BUF_OR_NULL(&uhw,
- buf + ucore.inlen,
- (unsigned long) ex_hdr.response + ucore.outlen,
- ex_hdr.provider_in_words * 8,
- ex_hdr.provider_out_words * 8);
+ ib_uverbs_init_udata_buf_or_null(&uhw,
+ buf + ucore.inlen,
+ u64_to_user_ptr(ex_hdr.response) + ucore.outlen,
+ ex_hdr.provider_in_words * 8,
+ ex_hdr.provider_out_words * 8);
- ret = uverbs_ex_cmd_table[command](file,
- ib_dev,
- &ucore,
- &uhw);
+ ret = uverbs_ex_cmd_table[command](file, ib_dev, &ucore, &uhw);
if (!ret)
ret = written_count;
} else {
diff --git a/drivers/infiniband/core/uverbs_std_types.c b/drivers/infiniband/core/uverbs_std_types.c
index 0a98579700ec..b095bce7f238 100644
--- a/drivers/infiniband/core/uverbs_std_types.c
+++ b/drivers/infiniband/core/uverbs_std_types.c
@@ -246,7 +246,8 @@ static void create_udata(struct uverbs_attr_bundle *ctx,
outbuf_len = uhw_out->ptr_attr.len;
}
- INIT_UDATA_BUF_OR_NULL(udata, inbuf, outbuf, inbuf_len, outbuf_len);
+ ib_uverbs_init_udata_buf_or_null(udata, inbuf, outbuf, inbuf_len,
+ outbuf_len);
}
static int uverbs_create_cq_handler(struct ib_device *ib_dev,
--
2.9.0
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH 2/2] IB/uverbs: clean up INIT_UDATA() macro usage
2017-09-06 21:34 [PATCH 1/2] [v2] IB/uverbs: clean up INIT_UDATA_BUF_OR_NULL usage Arnd Bergmann
@ 2017-09-06 21:34 ` Arnd Bergmann
2017-09-11 14:27 ` Leon Romanovsky
2017-09-27 13:10 ` Doug Ledford
2017-09-27 13:09 ` [PATCH 1/2] [v2] IB/uverbs: clean up INIT_UDATA_BUF_OR_NULL usage Doug Ledford
1 sibling, 2 replies; 5+ messages in thread
From: Arnd Bergmann @ 2017-09-06 21:34 UTC (permalink / raw)
To: Doug Ledford, Sean Hefty, Hal Rosenstock
Cc: Christoph Hellwig, Arnd Bergmann, Matan Barak, Yishai Hadas,
Leon Romanovsky, Moses Reuben, linux-rdma, linux-kernel
After changing INIT_UDATA_BUF_OR_NULL() to an inline function,
this does the same change to INIT_UDATA for consistency.
I'm keeping it separate as this part is much larger and
we wouldn't want to backport this to stable kernels if we
ever want to address the gcc warnings by backporting the
first patch.
Again, using an inline function gives us better type
safety here among other issues with macros. I'm using
u64_to_user_ptr() to convert the user pointer to simplify
the logic rather than adding lots of new type casts.
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
drivers/infiniband/core/uverbs.h | 22 +++---
drivers/infiniband/core/uverbs_cmd.c | 125 +++++++++++++++--------------------
2 files changed, 67 insertions(+), 80 deletions(-)
diff --git a/drivers/infiniband/core/uverbs.h b/drivers/infiniband/core/uverbs.h
index a8f104a4a91b..ee2739ae4305 100644
--- a/drivers/infiniband/core/uverbs.h
+++ b/drivers/infiniband/core/uverbs.h
@@ -47,13 +47,17 @@
#include <rdma/ib_umem.h>
#include <rdma/ib_user_verbs.h>
-#define INIT_UDATA(udata, ibuf, obuf, ilen, olen) \
- do { \
- (udata)->inbuf = (const void __user *) (ibuf); \
- (udata)->outbuf = (void __user *) (obuf); \
- (udata)->inlen = (ilen); \
- (udata)->outlen = (olen); \
- } while (0)
+static inline void
+ib_uverbs_init_udata(struct ib_udata *udata,
+ const void __user *ibuf,
+ void __user *obuf,
+ size_t ilen, size_t olen)
+{
+ udata->inbuf = ibuf;
+ udata->outbuf = obuf;
+ udata->inlen = ilen;
+ udata->outlen = olen;
+}
static inline void
ib_uverbs_init_udata_buf_or_null(struct ib_udata *udata,
@@ -61,7 +65,9 @@ ib_uverbs_init_udata_buf_or_null(struct ib_udata *udata,
void __user *obuf,
size_t ilen, size_t olen)
{
- INIT_UDATA(udata, ilen ? ibuf : NULL, olen ? obuf : NULL, ilen, olen);
+ ib_uverbs_init_udata(udata,
+ ilen ? ibuf : NULL, olen ? obuf : NULL,
+ ilen, olen);
}
/*
diff --git a/drivers/infiniband/core/uverbs_cmd.c b/drivers/infiniband/core/uverbs_cmd.c
index 4ab30d832ac5..cf787fabd88c 100644
--- a/drivers/infiniband/core/uverbs_cmd.c
+++ b/drivers/infiniband/core/uverbs_cmd.c
@@ -91,8 +91,8 @@ ssize_t ib_uverbs_get_context(struct ib_uverbs_file *file,
goto err;
}
- INIT_UDATA(&udata, buf + sizeof(cmd),
- (unsigned long) cmd.response + sizeof(resp),
+ ib_uverbs_init_udata(&udata, buf + sizeof(cmd),
+ u64_to_user_ptr(cmd.response) + sizeof(resp),
in_len - sizeof(cmd) - sizeof(struct ib_uverbs_cmd_hdr),
out_len - sizeof(resp));
@@ -141,8 +141,7 @@ ssize_t ib_uverbs_get_context(struct ib_uverbs_file *file,
goto err_fd;
}
- if (copy_to_user((void __user *) (unsigned long) cmd.response,
- &resp, sizeof resp)) {
+ if (copy_to_user(u64_to_user_ptr(cmd.response), &resp, sizeof resp)) {
ret = -EFAULT;
goto err_file;
}
@@ -238,8 +237,7 @@ ssize_t ib_uverbs_query_device(struct ib_uverbs_file *file,
memset(&resp, 0, sizeof resp);
copy_query_dev_fields(file, ib_dev, &resp, &ib_dev->attrs);
- if (copy_to_user((void __user *) (unsigned long) cmd.response,
- &resp, sizeof resp))
+ if (copy_to_user(u64_to_user_ptr(cmd.response), &resp, sizeof resp))
return -EFAULT;
return in_len;
@@ -295,8 +293,7 @@ ssize_t ib_uverbs_query_port(struct ib_uverbs_file *file,
resp.link_layer = rdma_port_get_link_layer(ib_dev,
cmd.port_num);
- if (copy_to_user((void __user *) (unsigned long) cmd.response,
- &resp, sizeof resp))
+ if (copy_to_user(u64_to_user_ptr(cmd.response), &resp, sizeof resp))
return -EFAULT;
return in_len;
@@ -320,8 +317,8 @@ ssize_t ib_uverbs_alloc_pd(struct ib_uverbs_file *file,
if (copy_from_user(&cmd, buf, sizeof cmd))
return -EFAULT;
- INIT_UDATA(&udata, buf + sizeof(cmd),
- (unsigned long) cmd.response + sizeof(resp),
+ ib_uverbs_init_udata(&udata, buf + sizeof(cmd),
+ u64_to_user_ptr(cmd.response) + sizeof(resp),
in_len - sizeof(cmd) - sizeof(struct ib_uverbs_cmd_hdr),
out_len - sizeof(resp));
@@ -344,8 +341,7 @@ ssize_t ib_uverbs_alloc_pd(struct ib_uverbs_file *file,
memset(&resp, 0, sizeof resp);
resp.pd_handle = uobj->id;
- if (copy_to_user((void __user *) (unsigned long) cmd.response,
- &resp, sizeof resp)) {
+ if (copy_to_user(u64_to_user_ptr(cmd.response), &resp, sizeof resp)) {
ret = -EFAULT;
goto err_copy;
}
@@ -490,8 +486,8 @@ ssize_t ib_uverbs_open_xrcd(struct ib_uverbs_file *file,
if (copy_from_user(&cmd, buf, sizeof cmd))
return -EFAULT;
- INIT_UDATA(&udata, buf + sizeof(cmd),
- (unsigned long) cmd.response + sizeof(resp),
+ ib_uverbs_init_udata(&udata, buf + sizeof(cmd),
+ u64_to_user_ptr(cmd.response) + sizeof(resp),
in_len - sizeof(cmd) - sizeof(struct ib_uverbs_cmd_hdr),
out_len - sizeof(resp));
@@ -556,8 +552,7 @@ ssize_t ib_uverbs_open_xrcd(struct ib_uverbs_file *file,
atomic_inc(&xrcd->usecnt);
}
- if (copy_to_user((void __user *) (unsigned long) cmd.response,
- &resp, sizeof resp)) {
+ if (copy_to_user(u64_to_user_ptr(cmd.response), &resp, sizeof resp)) {
ret = -EFAULT;
goto err_copy;
}
@@ -655,8 +650,8 @@ ssize_t ib_uverbs_reg_mr(struct ib_uverbs_file *file,
if (copy_from_user(&cmd, buf, sizeof cmd))
return -EFAULT;
- INIT_UDATA(&udata, buf + sizeof(cmd),
- (unsigned long) cmd.response + sizeof(resp),
+ ib_uverbs_init_udata(&udata, buf + sizeof(cmd),
+ u64_to_user_ptr(cmd.response) + sizeof(resp),
in_len - sizeof(cmd) - sizeof(struct ib_uverbs_cmd_hdr),
out_len - sizeof(resp));
@@ -705,8 +700,7 @@ ssize_t ib_uverbs_reg_mr(struct ib_uverbs_file *file,
resp.rkey = mr->rkey;
resp.mr_handle = uobj->id;
- if (copy_to_user((void __user *) (unsigned long) cmd.response,
- &resp, sizeof resp)) {
+ if (copy_to_user(u64_to_user_ptr(cmd.response), &resp, sizeof resp)) {
ret = -EFAULT;
goto err_copy;
}
@@ -748,8 +742,8 @@ ssize_t ib_uverbs_rereg_mr(struct ib_uverbs_file *file,
if (copy_from_user(&cmd, buf, sizeof(cmd)))
return -EFAULT;
- INIT_UDATA(&udata, buf + sizeof(cmd),
- (unsigned long) cmd.response + sizeof(resp),
+ ib_uverbs_init_udata(&udata, buf + sizeof(cmd),
+ u64_to_user_ptr(cmd.response) + sizeof(resp),
in_len - sizeof(cmd) - sizeof(struct ib_uverbs_cmd_hdr),
out_len - sizeof(resp));
@@ -800,8 +794,7 @@ ssize_t ib_uverbs_rereg_mr(struct ib_uverbs_file *file,
resp.lkey = mr->lkey;
resp.rkey = mr->rkey;
- if (copy_to_user((void __user *)(unsigned long)cmd.response,
- &resp, sizeof(resp)))
+ if (copy_to_user(u64_to_user_ptr(cmd.response), &resp, sizeof(resp)))
ret = -EFAULT;
else
ret = in_len;
@@ -867,8 +860,8 @@ ssize_t ib_uverbs_alloc_mw(struct ib_uverbs_file *file,
goto err_free;
}
- INIT_UDATA(&udata, buf + sizeof(cmd),
- (unsigned long)cmd.response + sizeof(resp),
+ ib_uverbs_init_udata(&udata, buf + sizeof(cmd),
+ u64_to_user_ptr(cmd.response) + sizeof(resp),
in_len - sizeof(cmd) - sizeof(struct ib_uverbs_cmd_hdr),
out_len - sizeof(resp));
@@ -889,8 +882,7 @@ ssize_t ib_uverbs_alloc_mw(struct ib_uverbs_file *file,
resp.rkey = mw->rkey;
resp.mw_handle = uobj->id;
- if (copy_to_user((void __user *)(unsigned long)cmd.response,
- &resp, sizeof(resp))) {
+ if (copy_to_user(u64_to_user_ptr(cmd.response), &resp, sizeof(resp))) {
ret = -EFAULT;
goto err_copy;
}
@@ -956,8 +948,7 @@ ssize_t ib_uverbs_create_comp_channel(struct ib_uverbs_file *file,
uobj_file.uobj);
ib_uverbs_init_event_queue(&ev_file->ev_queue);
- if (copy_to_user((void __user *) (unsigned long) cmd.response,
- &resp, sizeof resp)) {
+ if (copy_to_user(u64_to_user_ptr(cmd.response), &resp, sizeof resp)) {
uobj_alloc_abort(uobj);
return -EFAULT;
}
@@ -1087,10 +1078,11 @@ ssize_t ib_uverbs_create_cq(struct ib_uverbs_file *file,
if (copy_from_user(&cmd, buf, sizeof(cmd)))
return -EFAULT;
- INIT_UDATA(&ucore, buf, (unsigned long)cmd.response, sizeof(cmd), sizeof(resp));
+ ib_uverbs_init_udata(&ucore, buf, u64_to_user_ptr(cmd.response),
+ sizeof(cmd), sizeof(resp));
- INIT_UDATA(&uhw, buf + sizeof(cmd),
- (unsigned long)cmd.response + sizeof(resp),
+ ib_uverbs_init_udata(&uhw, buf + sizeof(cmd),
+ u64_to_user_ptr(cmd.response) + sizeof(resp),
in_len - sizeof(cmd) - sizeof(struct ib_uverbs_cmd_hdr),
out_len - sizeof(resp));
@@ -1173,8 +1165,8 @@ ssize_t ib_uverbs_resize_cq(struct ib_uverbs_file *file,
if (copy_from_user(&cmd, buf, sizeof cmd))
return -EFAULT;
- INIT_UDATA(&udata, buf + sizeof(cmd),
- (unsigned long) cmd.response + sizeof(resp),
+ ib_uverbs_init_udata(&udata, buf + sizeof(cmd),
+ u64_to_user_ptr(cmd.response) + sizeof(resp),
in_len - sizeof(cmd) - sizeof(struct ib_uverbs_cmd_hdr),
out_len - sizeof(resp));
@@ -1188,8 +1180,7 @@ ssize_t ib_uverbs_resize_cq(struct ib_uverbs_file *file,
resp.cqe = cq->cqe;
- if (copy_to_user((void __user *) (unsigned long) cmd.response,
- &resp, sizeof resp.cqe))
+ if (copy_to_user(u64_to_user_ptr(cmd.response), &resp, sizeof resp.cqe))
ret = -EFAULT;
out:
@@ -1249,7 +1240,7 @@ ssize_t ib_uverbs_poll_cq(struct ib_uverbs_file *file,
return -EINVAL;
/* we copy a struct ib_uverbs_poll_cq_resp to user space */
- header_ptr = (void __user *)(unsigned long) cmd.response;
+ header_ptr = u64_to_user_ptr(cmd.response);
data_ptr = header_ptr + sizeof resp;
memset(&resp, 0, sizeof resp);
@@ -1343,8 +1334,7 @@ ssize_t ib_uverbs_destroy_cq(struct ib_uverbs_file *file,
resp.async_events_reported = obj->async_events_reported;
uverbs_uobject_put(uobj);
- if (copy_to_user((void __user *) (unsigned long) cmd.response,
- &resp, sizeof resp))
+ if (copy_to_user(u64_to_user_ptr(cmd.response), &resp, sizeof resp))
return -EFAULT;
return in_len;
@@ -1650,10 +1640,10 @@ ssize_t ib_uverbs_create_qp(struct ib_uverbs_file *file,
if (copy_from_user(&cmd, buf, sizeof(cmd)))
return -EFAULT;
- INIT_UDATA(&ucore, buf, (unsigned long)cmd.response, sizeof(cmd),
- resp_size);
- INIT_UDATA(&uhw, buf + sizeof(cmd),
- (unsigned long)cmd.response + resp_size,
+ ib_uverbs_init_udata(&ucore, buf, u64_to_user_ptr(cmd.response),
+ sizeof(cmd), resp_size);
+ ib_uverbs_init_udata(&uhw, buf + sizeof(cmd),
+ u64_to_user_ptr(cmd.response) + resp_size,
in_len - sizeof(cmd) - sizeof(struct ib_uverbs_cmd_hdr),
out_len - resp_size);
@@ -1750,8 +1740,8 @@ ssize_t ib_uverbs_open_qp(struct ib_uverbs_file *file,
if (copy_from_user(&cmd, buf, sizeof cmd))
return -EFAULT;
- INIT_UDATA(&udata, buf + sizeof(cmd),
- (unsigned long) cmd.response + sizeof(resp),
+ ib_uverbs_init_udata(&udata, buf + sizeof(cmd),
+ u64_to_user_ptr(cmd.response) + sizeof(resp),
in_len - sizeof(cmd) - sizeof(struct ib_uverbs_cmd_hdr),
out_len - sizeof(resp));
@@ -1795,8 +1785,7 @@ ssize_t ib_uverbs_open_qp(struct ib_uverbs_file *file,
resp.qpn = qp->qp_num;
resp.qp_handle = obj->uevent.uobject.id;
- if (copy_to_user((void __user *) (unsigned long) cmd.response,
- &resp, sizeof resp)) {
+ if (copy_to_user(u64_to_user_ptr(cmd.response), &resp, sizeof resp)) {
ret = -EFAULT;
goto err_destroy;
}
@@ -1911,8 +1900,7 @@ ssize_t ib_uverbs_query_qp(struct ib_uverbs_file *file,
resp.max_inline_data = init_attr->cap.max_inline_data;
resp.sq_sig_all = init_attr->sq_sig_type == IB_SIGNAL_ALL_WR;
- if (copy_to_user((void __user *) (unsigned long) cmd.response,
- &resp, sizeof resp))
+ if (copy_to_user(u64_to_user_ptr(cmd.response), &resp, sizeof resp))
ret = -EFAULT;
out:
@@ -2042,7 +2030,7 @@ ssize_t ib_uverbs_modify_qp(struct ib_uverbs_file *file,
~((IB_USER_LEGACY_LAST_QP_ATTR_MASK << 1) - 1))
return -EOPNOTSUPP;
- INIT_UDATA(&udata, buf + sizeof(cmd.base), NULL,
+ ib_uverbs_init_udata(&udata, buf + sizeof(cmd.base), NULL,
in_len - sizeof(cmd.base) - sizeof(struct ib_uverbs_cmd_hdr),
out_len);
@@ -2126,8 +2114,7 @@ ssize_t ib_uverbs_destroy_qp(struct ib_uverbs_file *file,
resp.events_reported = obj->uevent.events_reported;
uverbs_uobject_put(uobj);
- if (copy_to_user((void __user *) (unsigned long) cmd.response,
- &resp, sizeof resp))
+ if (copy_to_user(u64_to_user_ptr(cmd.response), &resp, sizeof resp))
return -EFAULT;
return in_len;
@@ -2311,8 +2298,7 @@ ssize_t ib_uverbs_post_send(struct ib_uverbs_file *file,
break;
}
- if (copy_to_user((void __user *) (unsigned long) cmd.response,
- &resp, sizeof resp))
+ if (copy_to_user(u64_to_user_ptr(cmd.response), &resp, sizeof resp))
ret = -EFAULT;
out_put:
@@ -2460,8 +2446,7 @@ ssize_t ib_uverbs_post_recv(struct ib_uverbs_file *file,
}
}
- if (copy_to_user((void __user *) (unsigned long) cmd.response,
- &resp, sizeof resp))
+ if (copy_to_user(u64_to_user_ptr(cmd.response), &resp, sizeof resp))
ret = -EFAULT;
out:
@@ -2510,8 +2495,7 @@ ssize_t ib_uverbs_post_srq_recv(struct ib_uverbs_file *file,
break;
}
- if (copy_to_user((void __user *) (unsigned long) cmd.response,
- &resp, sizeof resp))
+ if (copy_to_user(u64_to_user_ptr(cmd.response), &resp, sizeof resp))
ret = -EFAULT;
out:
@@ -2548,8 +2532,8 @@ ssize_t ib_uverbs_create_ah(struct ib_uverbs_file *file,
if (!rdma_is_port_valid(ib_dev, cmd.attr.port_num))
return -EINVAL;
- INIT_UDATA(&udata, buf + sizeof(cmd),
- (unsigned long)cmd.response + sizeof(resp),
+ ib_uverbs_init_udata(&udata, buf + sizeof(cmd),
+ u64_to_user_ptr(cmd.response) + sizeof(resp),
in_len - sizeof(cmd) - sizeof(struct ib_uverbs_cmd_hdr),
out_len - sizeof(resp));
@@ -2600,8 +2584,7 @@ ssize_t ib_uverbs_create_ah(struct ib_uverbs_file *file,
resp.ah_handle = uobj->id;
- if (copy_to_user((void __user *) (unsigned long) cmd.response,
- &resp, sizeof resp)) {
+ if (copy_to_user(u64_to_user_ptr(cmd.response), &resp, sizeof resp)) {
ret = -EFAULT;
goto err_copy;
}
@@ -3627,8 +3610,8 @@ ssize_t ib_uverbs_create_srq(struct ib_uverbs_file *file,
xcmd.max_sge = cmd.max_sge;
xcmd.srq_limit = cmd.srq_limit;
- INIT_UDATA(&udata, buf + sizeof(cmd),
- (unsigned long) cmd.response + sizeof(resp),
+ ib_uverbs_init_udata(&udata, buf + sizeof(cmd),
+ u64_to_user_ptr(cmd.response) + sizeof(resp),
in_len - sizeof(cmd) - sizeof(struct ib_uverbs_cmd_hdr),
out_len - sizeof(resp));
@@ -3654,8 +3637,8 @@ ssize_t ib_uverbs_create_xsrq(struct ib_uverbs_file *file,
if (copy_from_user(&cmd, buf, sizeof cmd))
return -EFAULT;
- INIT_UDATA(&udata, buf + sizeof(cmd),
- (unsigned long) cmd.response + sizeof(resp),
+ ib_uverbs_init_udata(&udata, buf + sizeof(cmd),
+ u64_to_user_ptr(cmd.response) + sizeof(resp),
in_len - sizeof(cmd) - sizeof(struct ib_uverbs_cmd_hdr),
out_len - sizeof(resp));
@@ -3680,7 +3663,7 @@ ssize_t ib_uverbs_modify_srq(struct ib_uverbs_file *file,
if (copy_from_user(&cmd, buf, sizeof cmd))
return -EFAULT;
- INIT_UDATA(&udata, buf + sizeof cmd, NULL, in_len - sizeof cmd,
+ ib_uverbs_init_udata(&udata, buf + sizeof cmd, NULL, in_len - sizeof cmd,
out_len);
srq = uobj_get_obj_read(srq, cmd.srq_handle, file->ucontext);
@@ -3731,8 +3714,7 @@ ssize_t ib_uverbs_query_srq(struct ib_uverbs_file *file,
resp.max_sge = attr.max_sge;
resp.srq_limit = attr.srq_limit;
- if (copy_to_user((void __user *) (unsigned long) cmd.response,
- &resp, sizeof resp))
+ if (copy_to_user(u64_to_user_ptr(cmd.response), &resp, sizeof resp))
return -EFAULT;
return in_len;
@@ -3773,8 +3755,7 @@ ssize_t ib_uverbs_destroy_srq(struct ib_uverbs_file *file,
}
resp.events_reported = obj->events_reported;
uverbs_uobject_put(uobj);
- if (copy_to_user((void __user *)(unsigned long)cmd.response,
- &resp, sizeof(resp)))
+ if (copy_to_user(u64_to_user_ptr(cmd.response), &resp, sizeof(resp)))
return -EFAULT;
return in_len;
--
2.9.0
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 2/2] IB/uverbs: clean up INIT_UDATA() macro usage
2017-09-06 21:34 ` [PATCH 2/2] IB/uverbs: clean up INIT_UDATA() macro usage Arnd Bergmann
@ 2017-09-11 14:27 ` Leon Romanovsky
2017-09-27 13:10 ` Doug Ledford
1 sibling, 0 replies; 5+ messages in thread
From: Leon Romanovsky @ 2017-09-11 14:27 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Doug Ledford, Sean Hefty, Hal Rosenstock, Christoph Hellwig,
Matan Barak, Yishai Hadas, Moses Reuben, linux-rdma, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 920 bytes --]
On Wed, Sep 06, 2017 at 11:34:26PM +0200, Arnd Bergmann wrote:
> After changing INIT_UDATA_BUF_OR_NULL() to an inline function,
> this does the same change to INIT_UDATA for consistency.
> I'm keeping it separate as this part is much larger and
> we wouldn't want to backport this to stable kernels if we
> ever want to address the gcc warnings by backporting the
> first patch.
>
> Again, using an inline function gives us better type
> safety here among other issues with macros. I'm using
> u64_to_user_ptr() to convert the user pointer to simplify
> the logic rather than adding lots of new type casts.
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
> drivers/infiniband/core/uverbs.h | 22 +++---
> drivers/infiniband/core/uverbs_cmd.c | 125 +++++++++++++++--------------------
> 2 files changed, 67 insertions(+), 80 deletions(-)
>
Thanks, for doing it and for spotting light on u64_to_user_ptr().
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 1/2] [v2] IB/uverbs: clean up INIT_UDATA_BUF_OR_NULL usage
2017-09-06 21:34 [PATCH 1/2] [v2] IB/uverbs: clean up INIT_UDATA_BUF_OR_NULL usage Arnd Bergmann
2017-09-06 21:34 ` [PATCH 2/2] IB/uverbs: clean up INIT_UDATA() macro usage Arnd Bergmann
@ 2017-09-27 13:09 ` Doug Ledford
1 sibling, 0 replies; 5+ messages in thread
From: Doug Ledford @ 2017-09-27 13:09 UTC (permalink / raw)
To: Arnd Bergmann, Sean Hefty, Hal Rosenstock
Cc: Christoph Hellwig, Matan Barak, Yishai Hadas, Leon Romanovsky,
linux-rdma, linux-kernel
On Wed, 2017-09-06 at 23:34 +0200, Arnd Bergmann wrote:
> We get a harmless warning about the fact that we use the result of a
> multiplication as a condition:
>
> drivers/infiniband/core/uverbs_main.c: In function 'ib_uverbs_write':
> drivers/infiniband/core/uverbs_main.c:787:40: error: '*' in boolean
> context, suggest '&&' instead [-Werror=int-in-bool-context]
> drivers/infiniband/core/uverbs_main.c:787:117: error: '*' in boolean
> context, suggest '&&' instead [-Werror=int-in-bool-context]
> drivers/infiniband/core/uverbs_main.c:790:50: error: '*' in boolean
> context, suggest '&&' instead [-Werror=int-in-bool-context]
> drivers/infiniband/core/uverbs_main.c:790:151: error: '*' in boolean
> context, suggest '&&' instead [-Werror=int-in-bool-context]
>
> This avoids the problem by using an inline function in place of
> the macro.
>
> Fixes: a96e4e2ffe43 ("IB/uverbs: New macro to set pointers to NULL if
> length is 0 in INIT_UDATA()")
> Suggested-by: Christoph Hellwig <hch@infradead.org>
> Link: https://patchwork.kernel.org/patch/9940777/
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Thanks, applied.
--
Doug Ledford <dledford@redhat.com>
GPG KeyID: B826A3330E572FDD
Key fingerprint = AE6B 1BDA 122B 23B4 265B 1274 B826 A333 0E57 2FDD
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 2/2] IB/uverbs: clean up INIT_UDATA() macro usage
2017-09-06 21:34 ` [PATCH 2/2] IB/uverbs: clean up INIT_UDATA() macro usage Arnd Bergmann
2017-09-11 14:27 ` Leon Romanovsky
@ 2017-09-27 13:10 ` Doug Ledford
1 sibling, 0 replies; 5+ messages in thread
From: Doug Ledford @ 2017-09-27 13:10 UTC (permalink / raw)
To: Arnd Bergmann, Sean Hefty, Hal Rosenstock
Cc: Christoph Hellwig, Matan Barak, Yishai Hadas, Leon Romanovsky,
Moses Reuben, linux-rdma, linux-kernel
On Wed, 2017-09-06 at 23:34 +0200, Arnd Bergmann wrote:
> After changing INIT_UDATA_BUF_OR_NULL() to an inline function,
> this does the same change to INIT_UDATA for consistency.
> I'm keeping it separate as this part is much larger and
> we wouldn't want to backport this to stable kernels if we
> ever want to address the gcc warnings by backporting the
> first patch.
>
> Again, using an inline function gives us better type
> safety here among other issues with macros. I'm using
> u64_to_user_ptr() to convert the user pointer to simplify
> the logic rather than adding lots of new type casts.
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Thanks, applied.
--
Doug Ledford <dledford@redhat.com>
GPG KeyID: B826A3330E572FDD
Key fingerprint = AE6B 1BDA 122B 23B4 265B 1274 B826 A333 0E57 2FDD
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2017-09-27 13:10 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-09-06 21:34 [PATCH 1/2] [v2] IB/uverbs: clean up INIT_UDATA_BUF_OR_NULL usage Arnd Bergmann
2017-09-06 21:34 ` [PATCH 2/2] IB/uverbs: clean up INIT_UDATA() macro usage Arnd Bergmann
2017-09-11 14:27 ` Leon Romanovsky
2017-09-27 13:10 ` Doug Ledford
2017-09-27 13:09 ` [PATCH 1/2] [v2] IB/uverbs: clean up INIT_UDATA_BUF_OR_NULL usage Doug Ledford
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox