public inbox for [email protected]
 help / color / mirror / Atom feed
* 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