* [bug report] IB/uverbs: clean up INIT_UDATA_BUF_OR_NULL usage
@ 2017-10-02 10:13 Dan Carpenter
2017-10-02 11:00 ` Arnd Bergmann
0 siblings, 1 reply; 4+ messages in thread
From: Dan Carpenter @ 2017-10-02 10:13 UTC (permalink / raw)
To: arnd-r2nGTMty4D4; +Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA
Hello Arnd Bergmann,
The patch 12f727721eee: "IB/uverbs: clean up INIT_UDATA_BUF_OR_NULL
usage" from Sep 6, 2017, leads to the following static checker
warning:
drivers/infiniband/core/uverbs_std_types.c:249 create_udata()
error: uninitialized symbol 'inbuf'.
drivers/infiniband/core/uverbs_std_types.c
222 static void create_udata(struct uverbs_attr_bundle *ctx,
223 struct ib_udata *udata)
224 {
225 /*
226 * This is for ease of conversion. The purpose is to convert all drivers
227 * to use uverbs_attr_bundle instead of ib_udata.
228 * Assume attr == 0 is input and attr == 1 is output.
229 */
230 void __user *inbuf;
231 size_t inbuf_len = 0;
232 void __user *outbuf;
233 size_t outbuf_len = 0;
234 const struct uverbs_attr *uhw_in =
235 uverbs_attr_get(ctx, UVERBS_UHW_IN);
236 const struct uverbs_attr *uhw_out =
237 uverbs_attr_get(ctx, UVERBS_UHW_OUT);
238
239 if (!IS_ERR(uhw_in)) {
240 inbuf = uhw_in->ptr_attr.ptr;
241 inbuf_len = uhw_in->ptr_attr.len;
242 }
243
244 if (!IS_ERR(uhw_out)) {
245 outbuf = uhw_out->ptr_attr.ptr;
246 outbuf_len = uhw_out->ptr_attr.len;
247 }
248
249 ib_uverbs_init_udata_buf_or_null(udata, inbuf, outbuf, inbuf_len,
250 outbuf_len);
In the original code, this was a macro so smatch understood that we
don't use inbuf and outbuf if they aren't initialized. Now it's a
function and smatch generates a warning if you pass uninitialized
variables to a function.
It's not a bug, but it's really ugly.
251 }
regards,
dan carpenter
--
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
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [bug report] IB/uverbs: clean up INIT_UDATA_BUF_OR_NULL usage 2017-10-02 10:13 [bug report] IB/uverbs: clean up INIT_UDATA_BUF_OR_NULL usage Dan Carpenter @ 2017-10-02 11:00 ` Arnd Bergmann [not found] ` <CAK8P3a1t6zyQXtfov6HguOb9V7E4i54pK8+y8BjPqGqvsc9mpQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 0 siblings, 1 reply; 4+ messages in thread From: Arnd Bergmann @ 2017-10-02 11:00 UTC (permalink / raw) To: Dan Carpenter; +Cc: linux-rdma, Christoph Hellwig On Mon, Oct 2, 2017 at 12:13 PM, Dan Carpenter <dan.carpenter-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org> wrote: > Hello Arnd Bergmann, > > The patch 12f727721eee: "IB/uverbs: clean up INIT_UDATA_BUF_OR_NULL > usage" from Sep 6, 2017, leads to the following static checker > warning: > > drivers/infiniband/core/uverbs_std_types.c:249 create_udata() > error: uninitialized symbol 'inbuf'. > > drivers/infiniband/core/uverbs_std_types.c > 222 static void create_udata(struct uverbs_attr_bundle *ctx, > 223 struct ib_udata *udata) > 224 { > 225 /* > 226 * This is for ease of conversion. The purpose is to convert all drivers > 227 * to use uverbs_attr_bundle instead of ib_udata. > 228 * Assume attr == 0 is input and attr == 1 is output. > 229 */ > 230 void __user *inbuf; > 231 size_t inbuf_len = 0; > 232 void __user *outbuf; > 233 size_t outbuf_len = 0; > 234 const struct uverbs_attr *uhw_in = > 235 uverbs_attr_get(ctx, UVERBS_UHW_IN); > 236 const struct uverbs_attr *uhw_out = > 237 uverbs_attr_get(ctx, UVERBS_UHW_OUT); > 238 > 239 if (!IS_ERR(uhw_in)) { > 240 inbuf = uhw_in->ptr_attr.ptr; > 241 inbuf_len = uhw_in->ptr_attr.len; > 242 } > 243 > 244 if (!IS_ERR(uhw_out)) { > 245 outbuf = uhw_out->ptr_attr.ptr; > 246 outbuf_len = uhw_out->ptr_attr.len; > 247 } > 248 > 249 ib_uverbs_init_udata_buf_or_null(udata, inbuf, outbuf, inbuf_len, > 250 outbuf_len); > > In the original code, this was a macro so smatch understood that we > don't use inbuf and outbuf if they aren't initialized. Now it's a > function and smatch generates a warning if you pass uninitialized > variables to a function. > > It's not a bug, but it's really ugly. I see. Should we maybe open-code the ib_uverbs_init_udata_buf_or_null() call in this function? diff --git a/drivers/infiniband/core/uverbs_std_types.c b/drivers/infiniband/core/uverbs_std_types.c index b095bce7f238..039802ae7332 100644 --- a/drivers/infiniband/core/uverbs_std_types.c +++ b/drivers/infiniband/core/uverbs_std_types.c @@ -227,27 +227,26 @@ static void create_udata(struct uverbs_attr_bundle *ctx, * to use uverbs_attr_bundle instead of ib_udata. * Assume attr == 0 is input and attr == 1 is output. */ - void __user *inbuf; - size_t inbuf_len = 0; - void __user *outbuf; - size_t outbuf_len = 0; const struct uverbs_attr *uhw_in = uverbs_attr_get(ctx, UVERBS_UHW_IN); const struct uverbs_attr *uhw_out = uverbs_attr_get(ctx, UVERBS_UHW_OUT); if (!IS_ERR(uhw_in)) { - inbuf = uhw_in->ptr_attr.ptr; - inbuf_len = uhw_in->ptr_attr.len; + udata->inbuf = uhw_in->ptr_attr.ptr; + udata->inbuf_len = uhw_in->ptr_attr.len; + } else { + udata->inbuf = NULL; + udata->inbuf_len = 0; } if (!IS_ERR(uhw_out)) { - outbuf = uhw_out->ptr_attr.ptr; - outbuf_len = uhw_out->ptr_attr.len; + udata->outbuf = uhw_out->ptr_attr.ptr; + udata->outbuf_len = uhw_out->ptr_attr.len; + } else { + udata->outbuf = NULL; + udata->outbuf_len = 0; } - - 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, Arnd -- 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 ^ permalink raw reply related [flat|nested] 4+ messages in thread
[parent not found: <CAK8P3a1t6zyQXtfov6HguOb9V7E4i54pK8+y8BjPqGqvsc9mpQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>]
* Re: [bug report] IB/uverbs: clean up INIT_UDATA_BUF_OR_NULL usage [not found] ` <CAK8P3a1t6zyQXtfov6HguOb9V7E4i54pK8+y8BjPqGqvsc9mpQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> @ 2017-11-10 17:58 ` Doug Ledford [not found] ` <1510336682.3735.3.camel-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> 0 siblings, 1 reply; 4+ messages in thread From: Doug Ledford @ 2017-11-10 17:58 UTC (permalink / raw) To: Arnd Bergmann, Dan Carpenter; +Cc: linux-rdma, Christoph Hellwig [-- Attachment #1: Type: text/plain, Size: 4675 bytes --] On Mon, 2017-10-02 at 13:00 +0200, Arnd Bergmann wrote: > On Mon, Oct 2, 2017 at 12:13 PM, Dan Carpenter <dan.carpenter-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org> wrote: > > Hello Arnd Bergmann, > > > > The patch 12f727721eee: "IB/uverbs: clean up INIT_UDATA_BUF_OR_NULL > > usage" from Sep 6, 2017, leads to the following static checker > > warning: > > > > drivers/infiniband/core/uverbs_std_types.c:249 create_udata() > > error: uninitialized symbol 'inbuf'. > > > > drivers/infiniband/core/uverbs_std_types.c > > 222 static void create_udata(struct uverbs_attr_bundle *ctx, > > 223 struct ib_udata *udata) > > 224 { > > 225 /* > > 226 * This is for ease of conversion. The purpose is to convert all drivers > > 227 * to use uverbs_attr_bundle instead of ib_udata. > > 228 * Assume attr == 0 is input and attr == 1 is output. > > 229 */ > > 230 void __user *inbuf; > > 231 size_t inbuf_len = 0; > > 232 void __user *outbuf; > > 233 size_t outbuf_len = 0; > > 234 const struct uverbs_attr *uhw_in = > > 235 uverbs_attr_get(ctx, UVERBS_UHW_IN); > > 236 const struct uverbs_attr *uhw_out = > > 237 uverbs_attr_get(ctx, UVERBS_UHW_OUT); > > 238 > > 239 if (!IS_ERR(uhw_in)) { > > 240 inbuf = uhw_in->ptr_attr.ptr; > > 241 inbuf_len = uhw_in->ptr_attr.len; > > 242 } > > 243 > > 244 if (!IS_ERR(uhw_out)) { > > 245 outbuf = uhw_out->ptr_attr.ptr; > > 246 outbuf_len = uhw_out->ptr_attr.len; > > 247 } > > 248 > > 249 ib_uverbs_init_udata_buf_or_null(udata, inbuf, outbuf, inbuf_len, > > 250 outbuf_len); > > > > In the original code, this was a macro so smatch understood that we > > don't use inbuf and outbuf if they aren't initialized. Now it's a > > function and smatch generates a warning if you pass uninitialized > > variables to a function. > > > > It's not a bug, but it's really ugly. > > I see. Should we maybe open-code the ib_uverbs_init_udata_buf_or_null() > call in this function? > > diff --git a/drivers/infiniband/core/uverbs_std_types.c > b/drivers/infiniband/core/uverbs_std_types.c > index b095bce7f238..039802ae7332 100644 > --- a/drivers/infiniband/core/uverbs_std_types.c > +++ b/drivers/infiniband/core/uverbs_std_types.c > @@ -227,27 +227,26 @@ static void create_udata(struct uverbs_attr_bundle *ctx, > * to use uverbs_attr_bundle instead of ib_udata. > * Assume attr == 0 is input and attr == 1 is output. > */ > - void __user *inbuf; > - size_t inbuf_len = 0; > - void __user *outbuf; > - size_t outbuf_len = 0; > const struct uverbs_attr *uhw_in = > uverbs_attr_get(ctx, UVERBS_UHW_IN); > const struct uverbs_attr *uhw_out = > uverbs_attr_get(ctx, UVERBS_UHW_OUT); > > if (!IS_ERR(uhw_in)) { > - inbuf = uhw_in->ptr_attr.ptr; > - inbuf_len = uhw_in->ptr_attr.len; > + udata->inbuf = uhw_in->ptr_attr.ptr; > + udata->inbuf_len = uhw_in->ptr_attr.len; > + } else { > + udata->inbuf = NULL; > + udata->inbuf_len = 0; > } > > if (!IS_ERR(uhw_out)) { > - outbuf = uhw_out->ptr_attr.ptr; > - outbuf_len = uhw_out->ptr_attr.len; > + udata->outbuf = uhw_out->ptr_attr.ptr; > + udata->outbuf_len = uhw_out->ptr_attr.len; > + } else { > + udata->outbuf = NULL; > + udata->outbuf_len = 0; > } > - > - 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, > > Arnd > -- > 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 Arnd, can you send this as a proper patch with attribution and a Signed- off-by: please. -- Doug Ledford <dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> GPG KeyID: B826A3330E572FDD Key fingerprint = AE6B 1BDA 122B 23B4 265B 1274 B826 A333 0E57 2FDD [-- Attachment #2: This is a digitally signed message part --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 4+ messages in thread
[parent not found: <1510336682.3735.3.camel-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>]
* Re: [bug report] IB/uverbs: clean up INIT_UDATA_BUF_OR_NULL usage [not found] ` <1510336682.3735.3.camel-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> @ 2017-11-10 22:12 ` Arnd Bergmann 0 siblings, 0 replies; 4+ messages in thread From: Arnd Bergmann @ 2017-11-10 22:12 UTC (permalink / raw) To: Doug Ledford; +Cc: Dan Carpenter, linux-rdma, Christoph Hellwig On Fri, Nov 10, 2017 at 6:58 PM, Doug Ledford <dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote: > On Mon, 2017-10-02 at 13:00 +0200, Arnd Bergmann wrote: >> On Mon, Oct 2, 2017 at 12:13 PM, Dan Carpenter <dan.carpenter-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org> wrote: >> >> static int uverbs_create_cq_handler(struct ib_device *ib_dev, >> > Arnd, can you send this as a proper patch with attribution and a Signed- > off-by: please. > Submitted now as "infiniband: avoid uninitialized variable warning in create_udata", Message-Id: <20171110221053.3549147-1-arnd-r2nGTMty4D4@public.gmane.org>. Arnd -- 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 ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2017-11-10 22:12 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-10-02 10:13 [bug report] IB/uverbs: clean up INIT_UDATA_BUF_OR_NULL usage Dan Carpenter
2017-10-02 11:00 ` Arnd Bergmann
[not found] ` <CAK8P3a1t6zyQXtfov6HguOb9V7E4i54pK8+y8BjPqGqvsc9mpQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2017-11-10 17:58 ` Doug Ledford
[not found] ` <1510336682.3735.3.camel-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
2017-11-10 22:12 ` Arnd Bergmann
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox