* 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 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
* 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
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