* [PATCH][next] io_uring: ensure variable ret is initialized to zero
@ 2019-09-26 9:50 Colin King
2019-09-26 9:56 ` Jens Axboe
0 siblings, 1 reply; 6+ messages in thread
From: Colin King @ 2019-09-26 9:50 UTC (permalink / raw)
To: Jens Axboe, Alexander Viro, linux-block, linux-fsdevel
Cc: kernel-janitors, linux-kernel
From: Colin Ian King <colin.king@canonical.com>
In the case where sig is NULL the error variable ret is not initialized
and may contain a garbage value on the final checks to see if ret is
-ERESTARTSYS. Best to initialize ret to zero before the do loop to
ensure the ret does not accidentially contain -ERESTARTSYS before the
loop.
Addresses-Coverity: ("Uninitialized scalar variable")
Fixes: dd671c79e40b ("io_uring: make CQ ring wakeups be more efficient")
Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
fs/io_uring.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/fs/io_uring.c b/fs/io_uring.c
index 7b5710e3a18c..aa8ac557493c 100644
--- a/fs/io_uring.c
+++ b/fs/io_uring.c
@@ -2835,6 +2835,7 @@ static int io_cqring_wait(struct io_ring_ctx *ctx, int min_events,
return ret;
}
+ ret = 0;
iowq.nr_timeouts = atomic_read(&ctx->cq_timeouts);
do {
prepare_to_wait_exclusive(&ctx->wait, &iowq.wq,
--
2.20.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH][next] io_uring: ensure variable ret is initialized to zero
2019-09-26 9:50 [PATCH][next] io_uring: ensure variable ret is initialized to zero Colin King
@ 2019-09-26 9:56 ` Jens Axboe
2019-09-26 11:33 ` Dan Carpenter
0 siblings, 1 reply; 6+ messages in thread
From: Jens Axboe @ 2019-09-26 9:56 UTC (permalink / raw)
To: Colin King, Alexander Viro, linux-block, linux-fsdevel
Cc: kernel-janitors, linux-kernel
On 9/26/19 11:50 AM, Colin King wrote:
> From: Colin Ian King <colin.king@canonical.com>
>
> In the case where sig is NULL the error variable ret is not initialized
> and may contain a garbage value on the final checks to see if ret is
> -ERESTARTSYS. Best to initialize ret to zero before the do loop to
> ensure the ret does not accidentially contain -ERESTARTSYS before the
> loop.
Oops, weird it didn't complain. I've folded in this fix, as that commit
isn't upstream yet. Thanks!
--
Jens Axboe
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH][next] io_uring: ensure variable ret is initialized to zero
2019-09-26 9:56 ` Jens Axboe
@ 2019-09-26 11:33 ` Dan Carpenter
2019-09-26 11:42 ` Jens Axboe
0 siblings, 1 reply; 6+ messages in thread
From: Dan Carpenter @ 2019-09-26 11:33 UTC (permalink / raw)
To: Jens Axboe
Cc: Colin King, Alexander Viro, linux-block, linux-fsdevel,
kernel-janitors, linux-kernel
On Thu, Sep 26, 2019 at 11:56:30AM +0200, Jens Axboe wrote:
> On 9/26/19 11:50 AM, Colin King wrote:
> > From: Colin Ian King <colin.king@canonical.com>
> >
> > In the case where sig is NULL the error variable ret is not initialized
> > and may contain a garbage value on the final checks to see if ret is
> > -ERESTARTSYS. Best to initialize ret to zero before the do loop to
> > ensure the ret does not accidentially contain -ERESTARTSYS before the
> > loop.
>
> Oops, weird it didn't complain. I've folded in this fix, as that commit
> isn't upstream yet. Thanks!
There is a bug in GCC where at certain optimization levels, instead of
complaining, it initializes it to zero.
regards,
dan carpenter
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH][next] io_uring: ensure variable ret is initialized to zero
2019-09-26 11:33 ` Dan Carpenter
@ 2019-09-26 11:42 ` Jens Axboe
2019-09-26 12:14 ` Rasmus Villemoes
0 siblings, 1 reply; 6+ messages in thread
From: Jens Axboe @ 2019-09-26 11:42 UTC (permalink / raw)
To: Dan Carpenter
Cc: Colin King, Alexander Viro, linux-block, linux-fsdevel,
kernel-janitors, linux-kernel
On 9/26/19 1:33 PM, Dan Carpenter wrote:
> On Thu, Sep 26, 2019 at 11:56:30AM +0200, Jens Axboe wrote:
>> On 9/26/19 11:50 AM, Colin King wrote:
>>> From: Colin Ian King <colin.king@canonical.com>
>>>
>>> In the case where sig is NULL the error variable ret is not initialized
>>> and may contain a garbage value on the final checks to see if ret is
>>> -ERESTARTSYS. Best to initialize ret to zero before the do loop to
>>> ensure the ret does not accidentially contain -ERESTARTSYS before the
>>> loop.
>>
>> Oops, weird it didn't complain. I've folded in this fix, as that commit
>> isn't upstream yet. Thanks!
>
> There is a bug in GCC where at certain optimization levels, instead of
> complaining, it initializes it to zero.
That's awfully nice of it ;-)
Tried with -O0 and still didn't complain for me.
$ gcc --version
gcc (Ubuntu 9.1.0-2ubuntu2~18.04) 9.1.0
Tried gcc 5/6/7/8 as well. Might have to go look at what code it's
generating.
--
Jens Axboe
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH][next] io_uring: ensure variable ret is initialized to zero
2019-09-26 11:42 ` Jens Axboe
@ 2019-09-26 12:14 ` Rasmus Villemoes
2019-09-26 12:26 ` Jens Axboe
0 siblings, 1 reply; 6+ messages in thread
From: Rasmus Villemoes @ 2019-09-26 12:14 UTC (permalink / raw)
To: Jens Axboe, Dan Carpenter
Cc: Colin King, Alexander Viro, linux-block, linux-fsdevel,
kernel-janitors, linux-kernel
On 26/09/2019 13.42, Jens Axboe wrote:
> On 9/26/19 1:33 PM, Dan Carpenter wrote:
>> On Thu, Sep 26, 2019 at 11:56:30AM +0200, Jens Axboe wrote:
>>> On 9/26/19 11:50 AM, Colin King wrote:
>>>> From: Colin Ian King <colin.king@canonical.com>
>>>>
>>>> In the case where sig is NULL the error variable ret is not initialized
>>>> and may contain a garbage value on the final checks to see if ret is
>>>> -ERESTARTSYS. Best to initialize ret to zero before the do loop to
>>>> ensure the ret does not accidentially contain -ERESTARTSYS before the
>>>> loop.
>>>
>>> Oops, weird it didn't complain. I've folded in this fix, as that commit
>>> isn't upstream yet. Thanks!
>>
>> There is a bug in GCC where at certain optimization levels, instead of
>> complaining, it initializes it to zero.
>
> That's awfully nice of it ;-)
>
> Tried with -O0 and still didn't complain for me.
>
> $ gcc --version
> gcc (Ubuntu 9.1.0-2ubuntu2~18.04) 9.1.0
>
> Tried gcc 5/6/7/8 as well. Might have to go look at what code it's
> generating.
>
I think it's essentially the same as
https://lore.kernel.org/lkml/CAHk-=whP-9yPAWuJDwA6+rQ-9owuYZgmrMA9AqO3EGJVefe8vg@mail.gmail.com/
(thread "tmpfs: fix uninitialized return value in shmem_link").
Rasmus
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH][next] io_uring: ensure variable ret is initialized to zero
2019-09-26 12:14 ` Rasmus Villemoes
@ 2019-09-26 12:26 ` Jens Axboe
0 siblings, 0 replies; 6+ messages in thread
From: Jens Axboe @ 2019-09-26 12:26 UTC (permalink / raw)
To: Rasmus Villemoes, Dan Carpenter
Cc: Colin King, Alexander Viro, linux-block, linux-fsdevel,
kernel-janitors, linux-kernel
On 9/26/19 2:14 PM, Rasmus Villemoes wrote:
> On 26/09/2019 13.42, Jens Axboe wrote:
>> On 9/26/19 1:33 PM, Dan Carpenter wrote:
>>> On Thu, Sep 26, 2019 at 11:56:30AM +0200, Jens Axboe wrote:
>>>> On 9/26/19 11:50 AM, Colin King wrote:
>>>>> From: Colin Ian King <colin.king@canonical.com>
>>>>>
>>>>> In the case where sig is NULL the error variable ret is not initialized
>>>>> and may contain a garbage value on the final checks to see if ret is
>>>>> -ERESTARTSYS. Best to initialize ret to zero before the do loop to
>>>>> ensure the ret does not accidentially contain -ERESTARTSYS before the
>>>>> loop.
>>>>
>>>> Oops, weird it didn't complain. I've folded in this fix, as that commit
>>>> isn't upstream yet. Thanks!
>>>
>>> There is a bug in GCC where at certain optimization levels, instead of
>>> complaining, it initializes it to zero.
>>
>> That's awfully nice of it ;-)
>>
>> Tried with -O0 and still didn't complain for me.
>>
>> $ gcc --version
>> gcc (Ubuntu 9.1.0-2ubuntu2~18.04) 9.1.0
>>
>> Tried gcc 5/6/7/8 as well. Might have to go look at what code it's
>> generating.
>>
>
> I think it's essentially the same as
> https://lore.kernel.org/lkml/CAHk-=whP-9yPAWuJDwA6+rQ-9owuYZgmrMA9AqO3EGJVefe8vg@mail.gmail.com/
> (thread "tmpfs: fix uninitialized return value in shmem_link").
I think you're right, it's the same pattern. If I kill the:
if (ret)
return ret;
inside the if (sig) branch, then gcc does show the warning as it should.
--
Jens Axboe
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2019-09-26 12:26 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-09-26 9:50 [PATCH][next] io_uring: ensure variable ret is initialized to zero Colin King
2019-09-26 9:56 ` Jens Axboe
2019-09-26 11:33 ` Dan Carpenter
2019-09-26 11:42 ` Jens Axboe
2019-09-26 12:14 ` Rasmus Villemoes
2019-09-26 12:26 ` Jens Axboe
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).