Date: Mon, 12 Jul 2021 13:09:49 GMT From: Hans Petter Selasky <hselasky@FreeBSD.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org Subject: git: 05f4691919d6 - main - ibcore: Clean up INIT_UDATA() and INIT_UDATA_BUF_OR_NULL() macro usage. Message-ID: <202107121309.16CD9nRi095303@gitrepo.freebsd.org>
next in thread | raw e-mail | index | archive | help
The branch main has been updated by hselasky: URL: https://cgit.FreeBSD.org/src/commit/?id=05f4691919d6d0219795a1ca8ad84dd82d87b1cf commit 05f4691919d6d0219795a1ca8ad84dd82d87b1cf Author: Hans Petter Selasky <hselasky@FreeBSD.org> AuthorDate: 2021-06-16 13:01:51 +0000 Commit: Hans Petter Selasky <hselasky@FreeBSD.org> CommitDate: 2021-07-12 12:22:32 +0000 ibcore: Clean up INIT_UDATA() and INIT_UDATA_BUF_OR_NULL() macro usage. We get a harmless warning about the fact that we use the result of a multiplication as a condition in INIT_UDATA_BUF_OR_NULL(): uverbs_main.c: In function 'ib_uverbs_write': 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. After changing INIT_UDATA_BUF_OR_NULL() to an inline function, do the same change to INIT_UDATA() for consistency. 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. Linux commit: 12f727721eee61b3d19dedb95cb893b2baa9fe41 40a203396cc1c239f2e71c47c66ed03097123d2c MFC after: 1 week Reviewed by: kib Sponsored by: Mellanox Technologies // NVIDIA Networking --- sys/ofed/drivers/infiniband/core/ib_uverbs_cmd.c | 63 ++++++++++++----------- sys/ofed/drivers/infiniband/core/ib_uverbs_main.c | 7 +-- sys/ofed/drivers/infiniband/core/uverbs.h | 37 +++++++------ 3 files changed, 57 insertions(+), 50 deletions(-) diff --git a/sys/ofed/drivers/infiniband/core/ib_uverbs_cmd.c b/sys/ofed/drivers/infiniband/core/ib_uverbs_cmd.c index 46edd7b28580..dcc73dc5848e 100644 --- a/sys/ofed/drivers/infiniband/core/ib_uverbs_cmd.c +++ b/sys/ofed/drivers/infiniband/core/ib_uverbs_cmd.c @@ -339,8 +339,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, out_len - sizeof resp); ucontext = ib_dev->alloc_ucontext(ib_dev, &udata); @@ -560,8 +560,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, out_len - sizeof resp); uobj = kmalloc(sizeof *uobj, GFP_KERNEL); @@ -768,8 +768,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, out_len - sizeof resp); mutex_lock(&file->device->xrcd_tree_mutex); @@ -978,8 +978,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, out_len - sizeof resp); if ((cmd.start & ~PAGE_MASK) != (cmd.hca_va & ~PAGE_MASK)) @@ -1085,8 +1085,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), out_len - sizeof(resp)); if (cmd.flags & ~IB_MR_REREG_SUPPORTED || !cmd.flags) @@ -1225,8 +1225,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)); @@ -1494,10 +1494,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), out_len - sizeof(resp)); memset(&cmd_ex, 0, sizeof(cmd_ex)); @@ -1579,8 +1580,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, out_len - sizeof resp); cq = idr_read_cq(cmd.cq_handle, file->ucontext, 0); @@ -2050,10 +2051,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); @@ -2150,8 +2151,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, out_len - sizeof resp); obj = kmalloc(sizeof *obj, GFP_KERNEL); @@ -2354,7 +2355,7 @@ ssize_t ib_uverbs_modify_qp(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); attr = kmalloc(sizeof *attr, GFP_KERNEL); @@ -2919,8 +2920,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), out_len - sizeof(resp)); uobj = kmalloc(sizeof *uobj, GFP_KERNEL); @@ -4045,8 +4046,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); @@ -4072,8 +4073,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); @@ -4098,7 +4099,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 = idr_read_srq(cmd.srq_handle, file->ucontext); diff --git a/sys/ofed/drivers/infiniband/core/ib_uverbs_main.c b/sys/ofed/drivers/infiniband/core/ib_uverbs_main.c index e1b560774fbc..dff566e71f99 100644 --- a/sys/ofed/drivers/infiniband/core/ib_uverbs_main.c +++ b/sys/ofed/drivers/infiniband/core/ib_uverbs_main.c @@ -868,12 +868,13 @@ 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, + 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, + ib_uverbs_init_udata_buf_or_null(&uhw, buf + ucore.inlen, - (unsigned long) ex_hdr.response + ucore.outlen, + u64_to_user_ptr(ex_hdr.response + ucore.outlen), ex_hdr.provider_in_words * 8, ex_hdr.provider_out_words * 8); diff --git a/sys/ofed/drivers/infiniband/core/uverbs.h b/sys/ofed/drivers/infiniband/core/uverbs.h index 0db0d47cc691..9f2a519fa259 100644 --- a/sys/ofed/drivers/infiniband/core/uverbs.h +++ b/sys/ofed/drivers/infiniband/core/uverbs.h @@ -54,23 +54,28 @@ #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; +} -#define INIT_UDATA_BUF_OR_NULL(udata, ibuf, obuf, ilen, olen) \ - do { \ - (udata)->inbuf = ((ilen) != 0) ? \ - (const void __user *) (ibuf) : NULL; \ - (udata)->outbuf = ((olen) != 0) ? \ - (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) +{ + ib_uverbs_init_udata(udata, + ilen ? ibuf : NULL, olen ? obuf : NULL, + ilen, olen); +} /* * Our lifetime rules for these structs are the following:
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202107121309.16CD9nRi095303>