* Re: [ammarfaizi2-block:testing/io_uring-sendto-recvfrom.v1] BUILD SUCCESS 68d110c39241b887ec388cd3316dbedb85b0cbcf
[not found] <61f5a2fa./zZ+BkSwdkmvxv7x%[email protected]>
@ 2022-01-30 9:11 ` Ammar Faizi
2022-02-07 6:13 ` Chen, Rong A
0 siblings, 1 reply; 9+ messages in thread
From: Ammar Faizi @ 2022-01-30 9:11 UTC (permalink / raw)
To: kernel test robot; +Cc: GNU/Weeb Mailing List, Tea Inside Mailing List
Hello,
On 1/30/22 3:26 AM, kernel test robot wrote:
> tree/branch: https://github.com/ammarfaizi2/linux-block testing/io_uring-sendto-recvfrom.v1
> branch HEAD: 68d110c39241b887ec388cd3316dbedb85b0cbcf io_uring: Add `sendto(2)` and `recvfrom(2)` support
>
> possible Warning in current branch (please contact us if interested):
>
> fs/io_uring.c:5238 io_recvfrom() error: uninitialized symbol 'flags'.
>
> Warning ids grouped by kconfigs:
>
> gcc_recent_errors
> `-- x86_64-randconfig-m001
> `-- fs-io_uring.c-io_recvfrom()-error:uninitialized-symbol-flags-.
Interesting...
Can I have the reproducer for this warning?
> elapsed time: 722m
>
> configs tested: 137
> configs skipped: 3
>
> The following configs have been built successfully.
> More configs may be tested in the coming days.
>
> gcc tested configs:
> arm defconfig
> arm64 allyesconfig
> arm64 defconfig
> arm allyesconfig
> arm allmodconfig
> i386 randconfig-c001-20220124
> sh sh03_defconfig
> arm axm55xx_defconfig
> powerpc pcm030_defconfig
> xtensa alldefconfig
> powerpc ep88xc_defconfig
> powerpc ep8248e_defconfig
> m68k stmark2_defconfig
> powerpc mpc834x_itx_defconfig
> arm shmobile_defconfig
> mips decstation_defconfig
> arm lubbock_defconfig
> arm iop32x_defconfig
> arc tb10x_defconfig
> s390 defconfig
> sh se7751_defconfig
> openrisc alldefconfig
> arc nsimosci_hs_smp_defconfig
> powerpc sequoia_defconfig
> m68k sun3_defconfig
> powerpc mpc834x_mds_defconfig
> sh se7705_defconfig
> xtensa smp_lx200_defconfig
> arm mini2440_defconfig
> sh se7343_defconfig
> sh polaris_defconfig
> sh se7712_defconfig
> xtensa audio_kc705_defconfig
> mips vocore2_defconfig
> arm h5000_defconfig
> arc nsimosci_defconfig
> sh se7722_defconfig
> powerpc asp8347_defconfig
> mips ip32_defconfig
> arm randconfig-c002-20220127
> arm randconfig-c002-20220124
> arm randconfig-c002-20220129
> ia64 allmodconfig
> ia64 defconfig
> ia64 allyesconfig
> m68k allmodconfig
> m68k defconfig
> m68k allyesconfig
> nios2 defconfig
> arc allyesconfig
> nds32 allnoconfig
> nds32 defconfig
> csky defconfig
> alpha defconfig
> alpha allyesconfig
> nios2 allyesconfig
> arc defconfig
> sh allmodconfig
> h8300 allyesconfig
> xtensa allyesconfig
> parisc defconfig
> s390 allyesconfig
> s390 allmodconfig
> parisc allyesconfig
> i386 allyesconfig
> sparc allyesconfig
> sparc defconfig
> i386 defconfig
> i386 debian-10.3-kselftests
> i386 debian-10.3
> mips allyesconfig
> mips allmodconfig
> powerpc allyesconfig
> powerpc allmodconfig
> powerpc allnoconfig
> x86_64 randconfig-a002-20220124
> x86_64 randconfig-a003-20220124
> x86_64 randconfig-a001-20220124
> x86_64 randconfig-a004-20220124
> x86_64 randconfig-a005-20220124
> x86_64 randconfig-a006-20220124
> i386 randconfig-a002-20220124
> i386 randconfig-a005-20220124
> i386 randconfig-a003-20220124
> i386 randconfig-a004-20220124
> i386 randconfig-a001-20220124
> i386 randconfig-a006-20220124
> x86_64 randconfig-a011
> x86_64 randconfig-a013
> x86_64 randconfig-a015
> riscv nommu_k210_defconfig
> riscv allyesconfig
> riscv nommu_virt_defconfig
> riscv allnoconfig
> riscv defconfig
> riscv rv32_defconfig
> riscv allmodconfig
> x86_64 rhel-8.3-kselftests
> um x86_64_defconfig
> um i386_defconfig
> x86_64 defconfig
> x86_64 rhel-8.3
> x86_64 kexec
> x86_64 allyesconfig
> x86_64 rhel-8.3-func
>
> clang tested configs:
> arm randconfig-c002-20220124
> riscv randconfig-c006-20220124
> i386 randconfig-c001-20220124
> powerpc randconfig-c003-20220124
> mips randconfig-c004-20220124
> x86_64 randconfig-c007-20220124
> x86_64 randconfig-c007
> riscv randconfig-c006-20220129
> arm randconfig-c002-20220129
> s390 randconfig-c005-20220129
> powerpc randconfig-c003-20220129
> mips randconfig-c004-20220129
> i386 randconfig-c001
> arm mvebu_v5_defconfig
> x86_64 randconfig-a012
> x86_64 randconfig-a014
> x86_64 randconfig-a016
> x86_64 randconfig-a011-20220124
> x86_64 randconfig-a013-20220124
> x86_64 randconfig-a015-20220124
> x86_64 randconfig-a016-20220124
> x86_64 randconfig-a014-20220124
> x86_64 randconfig-a012-20220124
> i386 randconfig-a011-20220124
> i386 randconfig-a016-20220124
> i386 randconfig-a013-20220124
> i386 randconfig-a014-20220124
> i386 randconfig-a015-20220124
> i386 randconfig-a012-20220124
> riscv randconfig-r042-20220124
> s390 randconfig-r044-20220124
> hexagon randconfig-r045-20220124
> hexagon randconfig-r045-20220127
> hexagon randconfig-r041-20220124
> hexagon randconfig-r041-20220127
>
> ---
> 0-DAY CI Kernel Test Service, Intel Corporation
> https://lists.01.org/hyperkitty/list/[email protected]
--
Ammar Faizi
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [ammarfaizi2-block:testing/io_uring-sendto-recvfrom.v1] BUILD SUCCESS 68d110c39241b887ec388cd3316dbedb85b0cbcf
2022-01-30 9:11 ` [ammarfaizi2-block:testing/io_uring-sendto-recvfrom.v1] BUILD SUCCESS 68d110c39241b887ec388cd3316dbedb85b0cbcf Ammar Faizi
@ 2022-02-07 6:13 ` Chen, Rong A
2022-02-07 11:43 ` [PATCH io_uring-5.17] io_uring: Fix build error potential reading uninitialized value Ammar Faizi
0 siblings, 1 reply; 9+ messages in thread
From: Chen, Rong A @ 2022-02-07 6:13 UTC (permalink / raw)
To: Ammar Faizi, kernel test robot
Cc: GNU/Weeb Mailing List, Tea Inside Mailing List
[-- Attachment #1: Type: text/plain, Size: 11142 bytes --]
Hi,
On 1/30/2022 5:11 PM, Ammar Faizi wrote:
> Hello,
>
> On 1/30/22 3:26 AM, kernel test robot wrote:
>> tree/branch: https://github.com/ammarfaizi2/linux-block
>> testing/io_uring-sendto-recvfrom.v1
>> branch HEAD: 68d110c39241b887ec388cd3316dbedb85b0cbcf io_uring: Add
>> `sendto(2)` and `recvfrom(2)` support
>>
>> possible Warning in current branch (please contact us if interested):
>>
>> fs/io_uring.c:5238 io_recvfrom() error: uninitialized symbol 'flags'.
>>
>> Warning ids grouped by kconfigs:
>>
>> gcc_recent_errors
>> `-- x86_64-randconfig-m001
>> `-- fs-io_uring.c-io_recvfrom()-error:uninitialized-symbol-flags-.
>
>
> Interesting...
> Can I have the reproducer for this warning?
Sorry for the delay, I attached the report for your reference.
Best Regards,
Rong Chen
>
>> elapsed time: 722m
>>
>> configs tested: 137
>> configs skipped: 3
>>
>> The following configs have been built successfully.
>> More configs may be tested in the coming days.
>>
>> gcc tested configs:
>> arm defconfig
>> arm64 allyesconfig
>> arm64 defconfig
>> arm allyesconfig
>> arm allmodconfig
>> i386 randconfig-c001-20220124
>> sh sh03_defconfig
>> arm axm55xx_defconfig
>> powerpc pcm030_defconfig
>> xtensa alldefconfig
>> powerpc ep88xc_defconfig
>> powerpc ep8248e_defconfig
>> m68k stmark2_defconfig
>> powerpc mpc834x_itx_defconfig
>> arm shmobile_defconfig
>> mips decstation_defconfig
>> arm lubbock_defconfig
>> arm iop32x_defconfig
>> arc tb10x_defconfig
>> s390 defconfig
>> sh se7751_defconfig
>> openrisc alldefconfig
>> arc nsimosci_hs_smp_defconfig
>> powerpc sequoia_defconfig
>> m68k sun3_defconfig
>> powerpc mpc834x_mds_defconfig
>> sh se7705_defconfig
>> xtensa smp_lx200_defconfig
>> arm mini2440_defconfig
>> sh se7343_defconfig
>> sh polaris_defconfig
>> sh se7712_defconfig
>> xtensa audio_kc705_defconfig
>> mips vocore2_defconfig
>> arm h5000_defconfig
>> arc nsimosci_defconfig
>> sh se7722_defconfig
>> powerpc asp8347_defconfig
>> mips ip32_defconfig
>> arm randconfig-c002-20220127
>> arm randconfig-c002-20220124
>> arm randconfig-c002-20220129
>> ia64 allmodconfig
>> ia64 defconfig
>> ia64 allyesconfig
>> m68k allmodconfig
>> m68k defconfig
>> m68k allyesconfig
>> nios2 defconfig
>> arc allyesconfig
>> nds32 allnoconfig
>> nds32 defconfig
>> csky defconfig
>> alpha defconfig
>> alpha allyesconfig
>> nios2 allyesconfig
>> arc defconfig
>> sh allmodconfig
>> h8300 allyesconfig
>> xtensa allyesconfig
>> parisc defconfig
>> s390 allyesconfig
>> s390 allmodconfig
>> parisc allyesconfig
>> i386 allyesconfig
>> sparc allyesconfig
>> sparc defconfig
>> i386 defconfig
>> i386 debian-10.3-kselftests
>> i386 debian-10.3
>> mips allyesconfig
>> mips allmodconfig
>> powerpc allyesconfig
>> powerpc allmodconfig
>> powerpc allnoconfig
>> x86_64 randconfig-a002-20220124
>> x86_64 randconfig-a003-20220124
>> x86_64 randconfig-a001-20220124
>> x86_64 randconfig-a004-20220124
>> x86_64 randconfig-a005-20220124
>> x86_64 randconfig-a006-20220124
>> i386 randconfig-a002-20220124
>> i386 randconfig-a005-20220124
>> i386 randconfig-a003-20220124
>> i386 randconfig-a004-20220124
>> i386 randconfig-a001-20220124
>> i386 randconfig-a006-20220124
>> x86_64 randconfig-a011
>> x86_64 randconfig-a013
>> x86_64 randconfig-a015
>> riscv nommu_k210_defconfig
>> riscv allyesconfig
>> riscv nommu_virt_defconfig
>> riscv allnoconfig
>> riscv defconfig
>> riscv rv32_defconfig
>> riscv allmodconfig
>> x86_64 rhel-8.3-kselftests
>> um x86_64_defconfig
>> um i386_defconfig
>> x86_64 defconfig
>> x86_64 rhel-8.3
>> x86_64 kexec
>> x86_64 allyesconfig
>> x86_64 rhel-8.3-func
>>
>> clang tested configs:
>> arm randconfig-c002-20220124
>> riscv randconfig-c006-20220124
>> i386 randconfig-c001-20220124
>> powerpc randconfig-c003-20220124
>> mips randconfig-c004-20220124
>> x86_64 randconfig-c007-20220124
>> x86_64 randconfig-c007
>> riscv randconfig-c006-20220129
>> arm randconfig-c002-20220129
>> s390 randconfig-c005-20220129
>> powerpc randconfig-c003-20220129
>> mips randconfig-c004-20220129
>> i386 randconfig-c001
>> arm mvebu_v5_defconfig
>> x86_64 randconfig-a012
>> x86_64 randconfig-a014
>> x86_64 randconfig-a016
>> x86_64 randconfig-a011-20220124
>> x86_64 randconfig-a013-20220124
>> x86_64 randconfig-a015-20220124
>> x86_64 randconfig-a016-20220124
>> x86_64 randconfig-a014-20220124
>> x86_64 randconfig-a012-20220124
>> i386 randconfig-a011-20220124
>> i386 randconfig-a016-20220124
>> i386 randconfig-a013-20220124
>> i386 randconfig-a014-20220124
>> i386 randconfig-a015-20220124
>> i386 randconfig-a012-20220124
>> riscv randconfig-r042-20220124
>> s390 randconfig-r044-20220124
>> hexagon randconfig-r045-20220124
>> hexagon randconfig-r045-20220127
>> hexagon randconfig-r041-20220124
>> hexagon randconfig-r041-20220127
>>
>> ---
>> 0-DAY CI Kernel Test Service, Intel Corporation
>> https://lists.01.org/hyperkitty/list/[email protected]
>
>
[-- Attachment #2: [kbuild] [ammarfaizi2-block testing_io_uring-sendto-recvfrom.v1 3_5] fs_io_uring.c 5238 io_recvfrom() error uninitialized symbol 'flags'..eml --]
[-- Type: message/rfc822, Size: 21004 bytes --]
From: kernel test robot <[email protected]>
To: [email protected]
Cc: [email protected], Dan Carpenter <[email protected]>
Subject: [kbuild] [ammarfaizi2-block:testing/io_uring-sendto-recvfrom.v1 3/5] fs/io_uring.c:5238 io_recvfrom() error: uninitialized symbol 'flags'.
Date: Sat, 29 Jan 2022 19:46:23 +0800
Message-ID: <[email protected]>
CC: [email protected]
CC: "GNU/Weeb Mailing List" <[email protected]>
CC: [email protected]
TO: Ammar Faizi <[email protected]>
tree: https://github.com/ammarfaizi2/linux-block testing/io_uring-sendto-recvfrom.v1
head: 68d110c39241b887ec388cd3316dbedb85b0cbcf
commit: 8d597f720657a5156c4b3cbced8d1571ba151f62 [3/5] io_uring: Rename `io_{send,recv}` to `io_{sendto,recvfrom}`
:::::: branch date: 6 hours ago
:::::: commit date: 6 hours ago
config: x86_64-randconfig-m001 (https://download.01.org/0day-ci/archive/20220129/[email protected]/config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <[email protected]>
Reported-by: Dan Carpenter <[email protected]>
New smatch warnings:
fs/io_uring.c:5238 io_recvfrom() error: uninitialized symbol 'flags'.
Old smatch warnings:
fs/io_uring.c:8422 io_sqe_files_register() error: we previously assumed 'ctx->file_data' could be null (see line 8394)
vim +/flags +5238 fs/io_uring.c
0fa03c624d8fc9 Jens Axboe 2019-04-19 5189
8d597f720657a5 Ammar Faizi 2022-01-07 5190 static int io_recvfrom(struct io_kiocb *req, unsigned int issue_flags)
fddafacee287b3 Jens Axboe 2020-01-04 5191 {
6b754c8b912a16 Pavel Begunkov 2020-07-16 5192 struct io_buffer *kbuf;
fddafacee287b3 Jens Axboe 2020-01-04 5193 struct io_sr_msg *sr = &req->sr_msg;
fddafacee287b3 Jens Axboe 2020-01-04 5194 struct msghdr msg;
7a7cacba8b4560 Pavel Begunkov 2020-07-16 5195 void __user *buf = sr->buf;
7a7cacba8b4560 Pavel Begunkov 2020-07-16 5196 struct socket *sock;
fddafacee287b3 Jens Axboe 2020-01-04 5197 struct iovec iov;
fddafacee287b3 Jens Axboe 2020-01-04 5198 unsigned flags;
d1fd1c201d7507 Pavel Begunkov 2021-12-05 5199 int ret, min_ret = 0;
45d189c6062922 Pavel Begunkov 2021-02-10 5200 bool force_nonblock = issue_flags & IO_URING_F_NONBLOCK;
7a7cacba8b4560 Pavel Begunkov 2020-07-16 5201
dba4a9256bb4d7 Florent Revest 2020-12-04 5202 sock = sock_from_file(req->file);
7a7cacba8b4560 Pavel Begunkov 2020-07-16 5203 if (unlikely(!sock))
dba4a9256bb4d7 Florent Revest 2020-12-04 5204 return -ENOTSOCK;
fddafacee287b3 Jens Axboe 2020-01-04 5205
bc02ef3325e3ef Pavel Begunkov 2020-07-16 5206 if (req->flags & REQ_F_BUFFER_SELECT) {
51aac424aef980 Pavel Begunkov 2021-10-14 5207 kbuf = io_recv_buffer_select(req, issue_flags);
bcda7baaa3f15c Jens Axboe 2020-02-23 5208 if (IS_ERR(kbuf))
bcda7baaa3f15c Jens Axboe 2020-02-23 5209 return PTR_ERR(kbuf);
bcda7baaa3f15c Jens Axboe 2020-02-23 5210 buf = u64_to_user_ptr(kbuf->addr);
bcda7baaa3f15c Jens Axboe 2020-02-23 5211 }
fddafacee287b3 Jens Axboe 2020-01-04 5212
7a7cacba8b4560 Pavel Begunkov 2020-07-16 5213 ret = import_single_range(READ, buf, sr->len, &iov, &msg.msg_iter);
14c32eee928662 Pavel Begunkov 2020-07-16 5214 if (unlikely(ret))
14c32eee928662 Pavel Begunkov 2020-07-16 5215 goto out_free;
fddafacee287b3 Jens Axboe 2020-01-04 5216
fddafacee287b3 Jens Axboe 2020-01-04 5217 msg.msg_name = NULL;
fddafacee287b3 Jens Axboe 2020-01-04 5218 msg.msg_control = NULL;
fddafacee287b3 Jens Axboe 2020-01-04 5219 msg.msg_controllen = 0;
fddafacee287b3 Jens Axboe 2020-01-04 5220 msg.msg_namelen = 0;
fddafacee287b3 Jens Axboe 2020-01-04 5221 msg.msg_iocb = NULL;
fddafacee287b3 Jens Axboe 2020-01-04 5222 msg.msg_flags = 0;
fddafacee287b3 Jens Axboe 2020-01-04 5223
044118069a23fd Pavel Begunkov 2021-04-01 5224 flags = req->sr_msg.msg_flags;
044118069a23fd Pavel Begunkov 2021-04-01 5225 if (force_nonblock)
fddafacee287b3 Jens Axboe 2020-01-04 5226 flags |= MSG_DONTWAIT;
0031275d119efe Stefan Metzmacher 2021-03-20 5227 if (flags & MSG_WAITALL)
0031275d119efe Stefan Metzmacher 2021-03-20 5228 min_ret = iov_iter_count(&msg.msg_iter);
0031275d119efe Stefan Metzmacher 2021-03-20 5229
0b7b21e42ba2d6 Jens Axboe 2020-01-31 5230 ret = sock_recvmsg(sock, &msg, flags);
7297ce3d59449d Pavel Begunkov 2021-11-23 5231 out_free:
7297ce3d59449d Pavel Begunkov 2021-11-23 5232 if (ret < min_ret) {
7297ce3d59449d Pavel Begunkov 2021-11-23 5233 if (ret == -EAGAIN && force_nonblock)
fddafacee287b3 Jens Axboe 2020-01-04 5234 return -EAGAIN;
fddafacee287b3 Jens Axboe 2020-01-04 5235 if (ret == -ERESTARTSYS)
fddafacee287b3 Jens Axboe 2020-01-04 5236 ret = -EINTR;
93d2bcd2cbfed2 Pavel Begunkov 2021-05-16 5237 req_set_fail(req);
7297ce3d59449d Pavel Begunkov 2021-11-23 @5238 } else if ((flags & MSG_WAITALL) && (msg.msg_flags & (MSG_TRUNC | MSG_CTRUNC))) {
7297ce3d59449d Pavel Begunkov 2021-11-23 5239 req_set_fail(req);
7297ce3d59449d Pavel Begunkov 2021-11-23 5240 }
d1fd1c201d7507 Pavel Begunkov 2021-12-05 5241
d1fd1c201d7507 Pavel Begunkov 2021-12-05 5242 __io_req_complete(req, issue_flags, ret, io_put_kbuf(req));
fddafacee287b3 Jens Axboe 2020-01-04 5243 return 0;
fddafacee287b3 Jens Axboe 2020-01-04 5244 }
fddafacee287b3 Jens Axboe 2020-01-04 5245
:::::: The code at line 5238 was first introduced by commit
:::::: 7297ce3d59449de49d3c9e1f64ae25488750a1fc io_uring: improve send/recv error handling
:::::: TO: Pavel Begunkov <[email protected]>
:::::: CC: Jens Axboe <[email protected]>
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/[email protected]
_______________________________________________
kbuild mailing list -- [email protected]
To unsubscribe send an email to [email protected]
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH io_uring-5.17] io_uring: Fix build error potential reading uninitialized value
2022-02-07 6:13 ` Chen, Rong A
@ 2022-02-07 11:43 ` Ammar Faizi
2022-02-07 13:45 ` Jens Axboe
0 siblings, 1 reply; 9+ messages in thread
From: Ammar Faizi @ 2022-02-07 11:43 UTC (permalink / raw)
To: Jens Axboe
Cc: GNU/Weeb Mailing List, io-uring Mailing list,
Tea Inside Mailing List, Linux Kernel Mailing List,
Alviro Iskandar Setiawan, kernel test robot, Dan Carpenter,
Chen, Rong A, Pavel Begunkov, Ammar Faizi
From: Alviro Iskandar Setiawan <[email protected]>
In io_recv() if import_single_range() fails, the @flags variable is
uninitialized, then it will goto out_free.
After the goto, the compiler doesn't know that (ret < min_ret) is
always true, so it thinks the "if ((flags & MSG_WAITALL) ..." path
could be taken.
The complaint comes from gcc-9 (Debian 9.3.0-22) 9.3.0:
```
fs/io_uring.c:5238 io_recvfrom() error: uninitialized symbol 'flags'
```
Fix this by bypassing the @ret and @flags check when
import_single_range() fails.
Reasons:
1. import_single_range() only returns -EFAULT when it fails.
2. At that point @flags is uninitialized and shouldn't be read.
Reported-by: kernel test robot <[email protected]>
Reported-by: Dan Carpenter <[email protected]>
Reported-by: "Chen, Rong A" <[email protected]>
Link: https://lore.gnuweeb.org/timl/[email protected]/
Cc: Pavel Begunkov <[email protected]>
Suggested-by: Ammar Faizi <[email protected]>
Fixes: 7297ce3d59449de49d3c9e1f64ae25488750a1fc ("io_uring: improve send/recv error handling")
Signed-off-by: Alviro Iskandar Setiawan <[email protected]>
Signed-off-by: Ammar Faizi <[email protected]>
---
fs/io_uring.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/fs/io_uring.c b/fs/io_uring.c
index 2e04f718319d..3445c4da0153 100644
--- a/fs/io_uring.c
+++ b/fs/io_uring.c
@@ -5228,7 +5228,6 @@ static int io_recv(struct io_kiocb *req, unsigned int issue_flags)
min_ret = iov_iter_count(&msg.msg_iter);
ret = sock_recvmsg(sock, &msg, flags);
-out_free:
if (ret < min_ret) {
if (ret == -EAGAIN && force_nonblock)
return -EAGAIN;
@@ -5236,9 +5235,9 @@ static int io_recv(struct io_kiocb *req, unsigned int issue_flags)
ret = -EINTR;
req_set_fail(req);
} else if ((flags & MSG_WAITALL) && (msg.msg_flags & (MSG_TRUNC | MSG_CTRUNC))) {
+out_free:
req_set_fail(req);
}
-
__io_req_complete(req, issue_flags, ret, io_put_kbuf(req));
return 0;
}
base-commit: f6133fbd373811066c8441737e65f384c8f31974
--
2.32.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH io_uring-5.17] io_uring: Fix build error potential reading uninitialized value
2022-02-07 11:43 ` [PATCH io_uring-5.17] io_uring: Fix build error potential reading uninitialized value Ammar Faizi
@ 2022-02-07 13:45 ` Jens Axboe
2022-02-07 14:05 ` [PATCH io_uring-5.17 v2] io_uring: Clean up a false-positive warning from GCC 9.3.0 Ammar Faizi
2022-02-07 14:20 ` [PATCH io_uring-5.17] io_uring: Fix build error potential reading uninitialized value Dan Carpenter
0 siblings, 2 replies; 9+ messages in thread
From: Jens Axboe @ 2022-02-07 13:45 UTC (permalink / raw)
To: Ammar Faizi
Cc: GNU/Weeb Mailing List, io-uring Mailing list,
Tea Inside Mailing List, Linux Kernel Mailing List,
Alviro Iskandar Setiawan, kernel test robot, Dan Carpenter,
Chen, Rong A, Pavel Begunkov
On 2/7/22 4:43 AM, Ammar Faizi wrote:
> From: Alviro Iskandar Setiawan <[email protected]>
>
> In io_recv() if import_single_range() fails, the @flags variable is
> uninitialized, then it will goto out_free.
>
> After the goto, the compiler doesn't know that (ret < min_ret) is
> always true, so it thinks the "if ((flags & MSG_WAITALL) ..." path
> could be taken.
>
> The complaint comes from gcc-9 (Debian 9.3.0-22) 9.3.0:
> ```
> fs/io_uring.c:5238 io_recvfrom() error: uninitialized symbol 'flags'
> ```
> Fix this by bypassing the @ret and @flags check when
> import_single_range() fails.
The compiler should be able to deduce this, and I guess newer compilers
do which is why we haven't seen this warning before. I'm fine with doing
this as a cleanup, but I think the commit title should be modified a
bit. It sounds like there might be an issue reading uninitialized data,
which isn't actually true.
--
Jens Axboe
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH io_uring-5.17 v2] io_uring: Clean up a false-positive warning from GCC 9.3.0
2022-02-07 13:45 ` Jens Axboe
@ 2022-02-07 14:05 ` Ammar Faizi
2022-02-07 15:38 ` Jens Axboe
2022-02-07 14:20 ` [PATCH io_uring-5.17] io_uring: Fix build error potential reading uninitialized value Dan Carpenter
1 sibling, 1 reply; 9+ messages in thread
From: Ammar Faizi @ 2022-02-07 14:05 UTC (permalink / raw)
To: Jens Axboe
Cc: GNU/Weeb Mailing List, io-uring Mailing list,
Tea Inside Mailing List, Linux Kernel Mailing List,
Alviro Iskandar Setiawan, kernel test robot, Dan Carpenter,
Chen, Rong A, Pavel Begunkov, Ammar Faizi
From: Alviro Iskandar Setiawan <[email protected]>
In io_recv(), if import_single_range() fails, the @flags variable is
uninitialized, then it will goto out_free.
After the goto, the compiler doesn't know that (ret < min_ret) is
always true, so it thinks the "if ((flags & MSG_WAITALL) ..." path
could be taken.
The complaint comes from gcc-9 (Debian 9.3.0-22) 9.3.0:
```
fs/io_uring.c:5238 io_recvfrom() error: uninitialized symbol 'flags'
```
Fix this by bypassing the @ret and @flags check when
import_single_range() fails.
Reasons:
1. import_single_range() only returns -EFAULT when it fails.
2. At that point, @flags is uninitialized and shouldn't be read.
Reported-by: kernel test robot <[email protected]>
Reported-by: Dan Carpenter <[email protected]>
Reported-by: "Chen, Rong A" <[email protected]>
Link: https://lore.gnuweeb.org/timl/[email protected]/
Cc: Pavel Begunkov <[email protected]>
Suggested-by: Ammar Faizi <[email protected]>
Fixes: 7297ce3d59449de49d3c9e1f64ae25488750a1fc ("io_uring: improve send/recv error handling")
Signed-off-by: Alviro Iskandar Setiawan <[email protected]>
Signed-off-by: Ammar Faizi <[email protected]>
---
v2:
- Update the subject line
fs/io_uring.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/fs/io_uring.c b/fs/io_uring.c
index 2e04f718319d..3445c4da0153 100644
--- a/fs/io_uring.c
+++ b/fs/io_uring.c
@@ -5228,7 +5228,6 @@ static int io_recv(struct io_kiocb *req, unsigned int issue_flags)
min_ret = iov_iter_count(&msg.msg_iter);
ret = sock_recvmsg(sock, &msg, flags);
-out_free:
if (ret < min_ret) {
if (ret == -EAGAIN && force_nonblock)
return -EAGAIN;
@@ -5236,9 +5235,9 @@ static int io_recv(struct io_kiocb *req, unsigned int issue_flags)
ret = -EINTR;
req_set_fail(req);
} else if ((flags & MSG_WAITALL) && (msg.msg_flags & (MSG_TRUNC | MSG_CTRUNC))) {
+out_free:
req_set_fail(req);
}
-
__io_req_complete(req, issue_flags, ret, io_put_kbuf(req));
return 0;
}
base-commit: f6133fbd373811066c8441737e65f384c8f31974
--
2.32.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH io_uring-5.17] io_uring: Fix build error potential reading uninitialized value
2022-02-07 13:45 ` Jens Axboe
2022-02-07 14:05 ` [PATCH io_uring-5.17 v2] io_uring: Clean up a false-positive warning from GCC 9.3.0 Ammar Faizi
@ 2022-02-07 14:20 ` Dan Carpenter
2022-02-07 14:33 ` Alviro Iskandar Setiawan
1 sibling, 1 reply; 9+ messages in thread
From: Dan Carpenter @ 2022-02-07 14:20 UTC (permalink / raw)
To: Jens Axboe
Cc: Ammar Faizi, GNU/Weeb Mailing List, io-uring Mailing list,
Tea Inside Mailing List, Linux Kernel Mailing List,
Alviro Iskandar Setiawan, kernel test robot, Chen, Rong A,
Pavel Begunkov
On Mon, Feb 07, 2022 at 06:45:57AM -0700, Jens Axboe wrote:
> On 2/7/22 4:43 AM, Ammar Faizi wrote:
> > From: Alviro Iskandar Setiawan <[email protected]>
> >
> > In io_recv() if import_single_range() fails, the @flags variable is
> > uninitialized, then it will goto out_free.
> >
> > After the goto, the compiler doesn't know that (ret < min_ret) is
> > always true, so it thinks the "if ((flags & MSG_WAITALL) ..." path
> > could be taken.
> >
> > The complaint comes from gcc-9 (Debian 9.3.0-22) 9.3.0:
> > ```
> > fs/io_uring.c:5238 io_recvfrom() error: uninitialized symbol 'flags'
> > ```
> > Fix this by bypassing the @ret and @flags check when
> > import_single_range() fails.
>
> The compiler should be able to deduce this, and I guess newer compilers
> do which is why we haven't seen this warning before.
No, we disabled GCC's uninitialized variable checking a couple years
back. Linus got sick of the false positives. You can still see it if
you enable W=2
fs/io_uring.c: In function ‘io_recv’:
fs/io_uring.c:5252:20: warning: ‘flags’ may be used uninitialized in this function [-Wmaybe-uninitialized]
} else if ((flags & MSG_WAITALL) && (msg.msg_flags & (MSG_TRUNC | MSG_CTRUNC))) {
~~~~~~~^~~~~~~~~~~~~~
If you introduce an uninitialized variable bug then likelyhood is the
kbuild-bot will send you a Clang warning or a Smatch warning or both.
I don't think anyone looks at GCC W=2 warnings.
regards,
dan carpenter
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH io_uring-5.17] io_uring: Fix build error potential reading uninitialized value
2022-02-07 14:20 ` [PATCH io_uring-5.17] io_uring: Fix build error potential reading uninitialized value Dan Carpenter
@ 2022-02-07 14:33 ` Alviro Iskandar Setiawan
2022-02-07 15:37 ` Jens Axboe
0 siblings, 1 reply; 9+ messages in thread
From: Alviro Iskandar Setiawan @ 2022-02-07 14:33 UTC (permalink / raw)
To: Dan Carpenter
Cc: Jens Axboe, Ammar Faizi, GNU/Weeb Mailing List,
io-uring Mailing list, Tea Inside Mailing List,
Linux Kernel Mailing List, kernel test robot, Chen, Rong A,
Pavel Begunkov
On Mon, Feb 7, 2022 at 9:21 PM Dan Carpenter <[email protected]> wrote:
> On Mon, Feb 07, 2022 at 06:45:57AM -0700, Jens Axboe wrote:
> > On 2/7/22 4:43 AM, Ammar Faizi wrote:
> > > From: Alviro Iskandar Setiawan <[email protected]>
> > >
> > > In io_recv() if import_single_range() fails, the @flags variable is
> > > uninitialized, then it will goto out_free.
> > >
> > > After the goto, the compiler doesn't know that (ret < min_ret) is
> > > always true, so it thinks the "if ((flags & MSG_WAITALL) ..." path
> > > could be taken.
> > >
> > > The complaint comes from gcc-9 (Debian 9.3.0-22) 9.3.0:
> > > ```
> > > fs/io_uring.c:5238 io_recvfrom() error: uninitialized symbol 'flags'
> > > ```
> > > Fix this by bypassing the @ret and @flags check when
> > > import_single_range() fails.
> >
> > The compiler should be able to deduce this, and I guess newer compilers
> > do which is why we haven't seen this warning before.
The compiler can't deduce this because the import_single_range() is
located in a different translation unit (different C file), so it
can't prove that (ret < min_ret) is always true as it can't see the
function definition (in reality, it is always true because it only
returns either 0 or -EFAULT).
>
> No, we disabled GCC's uninitialized variable checking a couple years
> back. Linus got sick of the false positives. You can still see it if
> you enable W=2
>
> fs/io_uring.c: In function ‘io_recv’:
> fs/io_uring.c:5252:20: warning: ‘flags’ may be used uninitialized in this function [-Wmaybe-uninitialized]
> } else if ((flags & MSG_WAITALL) && (msg.msg_flags & (MSG_TRUNC | MSG_CTRUNC))) {
> ~~~~~~~^~~~~~~~~~~~~~
>
> If you introduce an uninitialized variable bug then likelyhood is the
> kbuild-bot will send you a Clang warning or a Smatch warning or both.
> I don't think anyone looks at GCC W=2 warnings.
>
This warning is valid, and the compiler should really warn that. But
again, in reality, this is still a false-positive warning, because
that "else if" will never be taken from the "goto out_free" path.
-- Viro
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH io_uring-5.17] io_uring: Fix build error potential reading uninitialized value
2022-02-07 14:33 ` Alviro Iskandar Setiawan
@ 2022-02-07 15:37 ` Jens Axboe
0 siblings, 0 replies; 9+ messages in thread
From: Jens Axboe @ 2022-02-07 15:37 UTC (permalink / raw)
To: Alviro Iskandar Setiawan, Dan Carpenter
Cc: Ammar Faizi, GNU/Weeb Mailing List, io-uring Mailing list,
Tea Inside Mailing List, Linux Kernel Mailing List,
kernel test robot, Chen, Rong A, Pavel Begunkov
On 2/7/22 7:33 AM, Alviro Iskandar Setiawan wrote:
> On Mon, Feb 7, 2022 at 9:21 PM Dan Carpenter <[email protected]> wrote:
>> On Mon, Feb 07, 2022 at 06:45:57AM -0700, Jens Axboe wrote:
>>> On 2/7/22 4:43 AM, Ammar Faizi wrote:
>>>> From: Alviro Iskandar Setiawan <[email protected]>
>>>>
>>>> In io_recv() if import_single_range() fails, the @flags variable is
>>>> uninitialized, then it will goto out_free.
>>>>
>>>> After the goto, the compiler doesn't know that (ret < min_ret) is
>>>> always true, so it thinks the "if ((flags & MSG_WAITALL) ..." path
>>>> could be taken.
>>>>
>>>> The complaint comes from gcc-9 (Debian 9.3.0-22) 9.3.0:
>>>> ```
>>>> fs/io_uring.c:5238 io_recvfrom() error: uninitialized symbol 'flags'
>>>> ```
>>>> Fix this by bypassing the @ret and @flags check when
>>>> import_single_range() fails.
>>>
>>> The compiler should be able to deduce this, and I guess newer compilers
>>> do which is why we haven't seen this warning before.
>
> The compiler can't deduce this because the import_single_range() is
> located in a different translation unit (different C file), so it
> can't prove that (ret < min_ret) is always true as it can't see the
> function definition (in reality, it is always true because it only
> returns either 0 or -EFAULT).
Yes you are right, I forgot this is the generic helper, and not our
internal one.
>> No, we disabled GCC's uninitialized variable checking a couple years
>> back. Linus got sick of the false positives. You can still see it if
>> you enable W=2
>>
>> fs/io_uring.c: In function ‘io_recv’:
>> fs/io_uring.c:5252:20: warning: ‘flags’ may be used uninitialized in this function [-Wmaybe-uninitialized]
>> } else if ((flags & MSG_WAITALL) && (msg.msg_flags & (MSG_TRUNC | MSG_CTRUNC))) {
>> ~~~~~~~^~~~~~~~~~~~~~
>>
>> If you introduce an uninitialized variable bug then likelyhood is the
>> kbuild-bot will send you a Clang warning or a Smatch warning or both.
>> I don't think anyone looks at GCC W=2 warnings.
>>
>
> This warning is valid, and the compiler should really warn that. But
> again, in reality, this is still a false-positive warning, because
> that "else if" will never be taken from the "goto out_free" path.
Right, as mentioned in my email, there is no bug there. But I do like
the patch as it cleans it up too, the error-out path should not include
non-cleanup items.
--
Jens Axboe
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH io_uring-5.17 v2] io_uring: Clean up a false-positive warning from GCC 9.3.0
2022-02-07 14:05 ` [PATCH io_uring-5.17 v2] io_uring: Clean up a false-positive warning from GCC 9.3.0 Ammar Faizi
@ 2022-02-07 15:38 ` Jens Axboe
0 siblings, 0 replies; 9+ messages in thread
From: Jens Axboe @ 2022-02-07 15:38 UTC (permalink / raw)
To: Ammar Faizi
Cc: Chen, Rong A, Linux Kernel Mailing List, Alviro Iskandar Setiawan,
io-uring Mailing list, GNU/Weeb Mailing List, Dan Carpenter,
Pavel Begunkov, Tea Inside Mailing List, kernel test robot
On Mon, 7 Feb 2022 21:05:33 +0700, Ammar Faizi wrote:
> From: Alviro Iskandar Setiawan <[email protected]>
>
> In io_recv(), if import_single_range() fails, the @flags variable is
> uninitialized, then it will goto out_free.
>
> After the goto, the compiler doesn't know that (ret < min_ret) is
> always true, so it thinks the "if ((flags & MSG_WAITALL) ..." path
> could be taken.
>
> [...]
Applied, thanks!
[1/1] io_uring: Clean up a false-positive warning from GCC 9.3.0
commit: 0d7c1153d9291197c1dc473cfaade77acb874b4b
Best regards,
--
Jens Axboe
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2022-02-07 15:38 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <61f5a2fa./zZ+BkSwdkmvxv7x%[email protected]>
2022-01-30 9:11 ` [ammarfaizi2-block:testing/io_uring-sendto-recvfrom.v1] BUILD SUCCESS 68d110c39241b887ec388cd3316dbedb85b0cbcf Ammar Faizi
2022-02-07 6:13 ` Chen, Rong A
2022-02-07 11:43 ` [PATCH io_uring-5.17] io_uring: Fix build error potential reading uninitialized value Ammar Faizi
2022-02-07 13:45 ` Jens Axboe
2022-02-07 14:05 ` [PATCH io_uring-5.17 v2] io_uring: Clean up a false-positive warning from GCC 9.3.0 Ammar Faizi
2022-02-07 15:38 ` Jens Axboe
2022-02-07 14:20 ` [PATCH io_uring-5.17] io_uring: Fix build error potential reading uninitialized value Dan Carpenter
2022-02-07 14:33 ` Alviro Iskandar Setiawan
2022-02-07 15:37 ` Jens Axboe
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox