* [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
* 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
* 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.