* [bug report] io_uring: add support for sqe links
@ 2019-05-27 10:08 Dan Carpenter
2019-05-27 13:36 ` Jens Axboe
0 siblings, 1 reply; 5+ messages in thread
From: Dan Carpenter @ 2019-05-27 10:08 UTC (permalink / raw)
To: axboe; +Cc: linux-fsdevel
Hello Jens Axboe,
The patch f3fafe4103bd: "io_uring: add support for sqe links" from
May 10, 2019, leads to the following static checker warning:
fs/io_uring.c:623 io_req_link_next()
error: potential NULL dereference 'nxt'.
fs/io_uring.c
614 static void io_req_link_next(struct io_kiocb *req)
615 {
616 struct io_kiocb *nxt;
617
618 nxt = list_first_entry_or_null(&req->link_list, struct io_kiocb, list);
619 list_del(&nxt->list);
^^^^^^^^^
The warning is a false positive but this is a NULL dereference.
620 if (!list_empty(&req->link_list)) {
621 INIT_LIST_HEAD(&nxt->link_list);
^^^^^
False positive.
622 list_splice(&req->link_list, &nxt->link_list);
623 nxt->flags |= REQ_F_LINK;
624 }
625
626 INIT_WORK(&nxt->work, io_sq_wq_submit_work);
^^^^^^^^^^
627 queue_work(req->ctx->sqo_wq, &nxt->work);
^^^^^^^^^^
Other bugs.
628 }
regards,
dan carpenter
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [bug report] io_uring: add support for sqe links
2019-05-27 10:08 [bug report] io_uring: add support for sqe links Dan Carpenter
@ 2019-05-27 13:36 ` Jens Axboe
2019-05-27 14:10 ` Dan Carpenter
0 siblings, 1 reply; 5+ messages in thread
From: Jens Axboe @ 2019-05-27 13:36 UTC (permalink / raw)
To: Dan Carpenter; +Cc: linux-fsdevel
On 5/27/19 4:08 AM, Dan Carpenter wrote:
> Hello Jens Axboe,
>
> The patch f3fafe4103bd: "io_uring: add support for sqe links" from
> May 10, 2019, leads to the following static checker warning:
>
> fs/io_uring.c:623 io_req_link_next()
> error: potential NULL dereference 'nxt'.
>
> fs/io_uring.c
> 614 static void io_req_link_next(struct io_kiocb *req)
> 615 {
> 616 struct io_kiocb *nxt;
> 617
> 618 nxt = list_first_entry_or_null(&req->link_list, struct io_kiocb, list);
> 619 list_del(&nxt->list);
> ^^^^^^^^^
> The warning is a false positive but this is a NULL dereference.
>
> 620 if (!list_empty(&req->link_list)) {
> 621 INIT_LIST_HEAD(&nxt->link_list);
> ^^^^^
> False positive.
Both of them are false positives. I can work around them though, as it
probably makes it a bit cleaner, too.
>
> 622 list_splice(&req->link_list, &nxt->link_list);
> 623 nxt->flags |= REQ_F_LINK;
> 624 }
> 625
> 626 INIT_WORK(&nxt->work, io_sq_wq_submit_work);
> ^^^^^^^^^^
> 627 queue_work(req->ctx->sqo_wq, &nxt->work);
> ^^^^^^^^^^
> Other bugs.
Not sure what that means?
--
Jens Axboe
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [bug report] io_uring: add support for sqe links
2019-05-27 13:36 ` Jens Axboe
@ 2019-05-27 14:10 ` Dan Carpenter
2019-05-27 14:34 ` Jens Axboe
0 siblings, 1 reply; 5+ messages in thread
From: Dan Carpenter @ 2019-05-27 14:10 UTC (permalink / raw)
To: Jens Axboe; +Cc: linux-fsdevel
On Mon, May 27, 2019 at 07:36:22AM -0600, Jens Axboe wrote:
> On 5/27/19 4:08 AM, Dan Carpenter wrote:
> > Hello Jens Axboe,
> >
> > The patch f3fafe4103bd: "io_uring: add support for sqe links" from
> > May 10, 2019, leads to the following static checker warning:
> >
> > fs/io_uring.c:623 io_req_link_next()
> > error: potential NULL dereference 'nxt'.
> >
> > fs/io_uring.c
> > 614 static void io_req_link_next(struct io_kiocb *req)
> > 615 {
> > 616 struct io_kiocb *nxt;
> > 617
> > 618 nxt = list_first_entry_or_null(&req->link_list, struct io_kiocb, list);
^^^^^^^^^^^^^^^
If this list is empty then "nxt" is NULL.
> > 619 list_del(&nxt->list);
> > ^^^^^^^^^
> > The warning is a false positive but this is a NULL dereference.
> >
> > 620 if (!list_empty(&req->link_list)) {
^^^^^^^^^^^^^^^^^^^^^^^^^^^^
We're checking for list_empty() here.
> > 621 INIT_LIST_HEAD(&nxt->link_list);
> > ^^^^^
> > False positive.
>
> Both of them are false positives. I can work around them though, as it
> probably makes it a bit cleaner, too.
>
> >
> > 622 list_splice(&req->link_list, &nxt->link_list);
> > 623 nxt->flags |= REQ_F_LINK;
> > 624 }
> > 625
> > 626 INIT_WORK(&nxt->work, io_sq_wq_submit_work);
> > ^^^^^^^^^^
> > 627 queue_work(req->ctx->sqo_wq, &nxt->work);
> > ^^^^^^^^^^
> > Other bugs.
>
> Not sure what that means?
All these dereferences outside the if not empty check are a problem.
regards,
dan carpenter
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [bug report] io_uring: add support for sqe links
2019-05-27 14:10 ` Dan Carpenter
@ 2019-05-27 14:34 ` Jens Axboe
2019-05-27 15:23 ` Dan Carpenter
0 siblings, 1 reply; 5+ messages in thread
From: Jens Axboe @ 2019-05-27 14:34 UTC (permalink / raw)
To: Dan Carpenter; +Cc: linux-fsdevel
On 5/27/19 8:10 AM, Dan Carpenter wrote:
> On Mon, May 27, 2019 at 07:36:22AM -0600, Jens Axboe wrote:
>> On 5/27/19 4:08 AM, Dan Carpenter wrote:
>>> Hello Jens Axboe,
>>>
>>> The patch f3fafe4103bd: "io_uring: add support for sqe links" from
>>> May 10, 2019, leads to the following static checker warning:
>>>
>>> fs/io_uring.c:623 io_req_link_next()
>>> error: potential NULL dereference 'nxt'.
>>>
>>> fs/io_uring.c
>>> 614 static void io_req_link_next(struct io_kiocb *req)
>>> 615 {
>>> 616 struct io_kiocb *nxt;
>>> 617
>>> 618 nxt = list_first_entry_or_null(&req->link_list, struct io_kiocb, list);
> ^^^^^^^^^^^^^^^
> If this list is empty then "nxt" is NULL.
Right...
>>> 619 list_del(&nxt->list);
>>> ^^^^^^^^^
>>> The warning is a false positive but this is a NULL dereference.
>>>
>>> 620 if (!list_empty(&req->link_list)) {
> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>
> We're checking for list_empty() here.
After deleting an entry from it.
>>> 621 INIT_LIST_HEAD(&nxt->link_list);
>>> ^^^^^
>>> False positive.
>>
>> Both of them are false positives. I can work around them though, as it
>> probably makes it a bit cleaner, too.
>>
>>>
>>> 622 list_splice(&req->link_list, &nxt->link_list);
>>> 623 nxt->flags |= REQ_F_LINK;
>>> 624 }
>>> 625
>>> 626 INIT_WORK(&nxt->work, io_sq_wq_submit_work);
>>> ^^^^^^^^^^
>>> 627 queue_work(req->ctx->sqo_wq, &nxt->work);
>>> ^^^^^^^^^^
>>> Other bugs.
>>
>> Not sure what that means?
>
> All these dereferences outside the if not empty check are a problem.
But this is the same thing If 'nxt' can ever be NULL, it's a problem.
If it cannot, then it's a false positive.
I'll just add a check. It should not happen, but it probably can if
the chain is messed up.
--
Jens Axboe
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [bug report] io_uring: add support for sqe links
2019-05-27 14:34 ` Jens Axboe
@ 2019-05-27 15:23 ` Dan Carpenter
0 siblings, 0 replies; 5+ messages in thread
From: Dan Carpenter @ 2019-05-27 15:23 UTC (permalink / raw)
To: Jens Axboe; +Cc: linux-fsdevel
On Mon, May 27, 2019 at 08:34:18AM -0600, Jens Axboe wrote:
> On 5/27/19 8:10 AM, Dan Carpenter wrote:
> > On Mon, May 27, 2019 at 07:36:22AM -0600, Jens Axboe wrote:
> >> On 5/27/19 4:08 AM, Dan Carpenter wrote:
> >>> Hello Jens Axboe,
> >>>
> >>> The patch f3fafe4103bd: "io_uring: add support for sqe links" from
> >>> May 10, 2019, leads to the following static checker warning:
> >>>
> >>> fs/io_uring.c:623 io_req_link_next()
> >>> error: potential NULL dereference 'nxt'.
> >>>
> >>> fs/io_uring.c
> >>> 614 static void io_req_link_next(struct io_kiocb *req)
> >>> 615 {
> >>> 616 struct io_kiocb *nxt;
> >>> 617
> >>> 618 nxt = list_first_entry_or_null(&req->link_list, struct io_kiocb, list);
> > ^^^^^^^^^^^^^^^
> > If this list is empty then "nxt" is NULL.
>
> Right...
>
> >>> 619 list_del(&nxt->list);
> >>> ^^^^^^^^^
> >>> The warning is a false positive but this is a NULL dereference.
> >>>
> >>> 620 if (!list_empty(&req->link_list)) {
> > ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> >
> > We're checking for list_empty() here.
>
> After deleting an entry from it.
>
Ah... Right. Sorry.
regards,
dan carpenter
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2019-05-27 15:23 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-05-27 10:08 [bug report] io_uring: add support for sqe links Dan Carpenter
2019-05-27 13:36 ` Jens Axboe
2019-05-27 14:10 ` Dan Carpenter
2019-05-27 14:34 ` Jens Axboe
2019-05-27 15:23 ` Dan Carpenter
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).