All of lore.kernel.org
 help / color / mirror / Atom feed
* [axboe-block:for-5.17/block 106/108] block/blk-ioc.c:307 set_task_ioprio() error: we previously assumed 'task->io_context' could be null (see line 300)
@ 2021-12-17  7:49 ` Dan Carpenter
  0 siblings, 0 replies; 9+ messages in thread
From: kernel test robot @ 2021-12-17  7:40 UTC (permalink / raw)
  To: kbuild

[-- Attachment #1: Type: text/plain, Size: 4460 bytes --]

CC: kbuild-all(a)lists.01.org
CC: linux-kernel(a)vger.kernel.org
TO: Christoph Hellwig <hch@lst.de>
CC: Jens Axboe <axboe@kernel.dk>
CC: Jan Kara <jack@suse.cz>

tree:   https://git.kernel.org/pub/scm/linux/kernel/git/axboe/linux-block.git for-5.17/block
head:   5ef1630586317e92c9ebd7b4ce48f393b7ff790f
commit: 5fc11eebb4a98df5324a4de369bb5ab7f0007ff7 [106/108] block: open code create_task_io_context in set_task_ioprio
:::::: branch date: 14 hours ago
:::::: commit date: 14 hours ago
config: i386-randconfig-m021-20211216 (https://download.01.org/0day-ci/archive/20211217/202112171520.5hNnOM0q-lkp(a)intel.com/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 <lkp@intel.com>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>

smatch warnings:
block/blk-ioc.c:307 set_task_ioprio() error: we previously assumed 'task->io_context' could be null (see line 300)

vim +307 block/blk-ioc.c

86db1e29772372 Jens Axboe        2008-01-29  270  
a411cd3cfdc5bb Christoph Hellwig 2021-12-09  271  int set_task_ioprio(struct task_struct *task, int ioprio)
a411cd3cfdc5bb Christoph Hellwig 2021-12-09  272  {
a411cd3cfdc5bb Christoph Hellwig 2021-12-09  273  	int err;
a411cd3cfdc5bb Christoph Hellwig 2021-12-09  274  	const struct cred *cred = current_cred(), *tcred;
a411cd3cfdc5bb Christoph Hellwig 2021-12-09  275  
a411cd3cfdc5bb Christoph Hellwig 2021-12-09  276  	rcu_read_lock();
a411cd3cfdc5bb Christoph Hellwig 2021-12-09  277  	tcred = __task_cred(task);
a411cd3cfdc5bb Christoph Hellwig 2021-12-09  278  	if (!uid_eq(tcred->uid, cred->euid) &&
a411cd3cfdc5bb Christoph Hellwig 2021-12-09  279  	    !uid_eq(tcred->uid, cred->uid) && !capable(CAP_SYS_NICE)) {
a411cd3cfdc5bb Christoph Hellwig 2021-12-09  280  		rcu_read_unlock();
a411cd3cfdc5bb Christoph Hellwig 2021-12-09  281  		return -EPERM;
a411cd3cfdc5bb Christoph Hellwig 2021-12-09  282  	}
a411cd3cfdc5bb Christoph Hellwig 2021-12-09  283  	rcu_read_unlock();
a411cd3cfdc5bb Christoph Hellwig 2021-12-09  284  
a411cd3cfdc5bb Christoph Hellwig 2021-12-09  285  	err = security_task_setioprio(task, ioprio);
a411cd3cfdc5bb Christoph Hellwig 2021-12-09  286  	if (err)
a411cd3cfdc5bb Christoph Hellwig 2021-12-09  287  		return err;
a411cd3cfdc5bb Christoph Hellwig 2021-12-09  288  
8472161b77c41d Christoph Hellwig 2021-12-09  289  	task_lock(task);
8472161b77c41d Christoph Hellwig 2021-12-09  290  	if (unlikely(!task->io_context)) {
8472161b77c41d Christoph Hellwig 2021-12-09  291  		struct io_context *ioc;
8472161b77c41d Christoph Hellwig 2021-12-09  292  
8472161b77c41d Christoph Hellwig 2021-12-09  293  		task_unlock(task);
5fc11eebb4a98d Christoph Hellwig 2021-12-09  294  
5fc11eebb4a98d Christoph Hellwig 2021-12-09  295  		ioc = alloc_io_context(GFP_ATOMIC, NUMA_NO_NODE);
5fc11eebb4a98d Christoph Hellwig 2021-12-09  296  		if (!ioc)
5fc11eebb4a98d Christoph Hellwig 2021-12-09  297  			return -ENOMEM;
5fc11eebb4a98d Christoph Hellwig 2021-12-09  298  
5fc11eebb4a98d Christoph Hellwig 2021-12-09  299  		task_lock(task);
5fc11eebb4a98d Christoph Hellwig 2021-12-09 @300  		if (task->io_context || (task->flags & PF_EXITING)) {
5fc11eebb4a98d Christoph Hellwig 2021-12-09  301  			kmem_cache_free(iocontext_cachep, ioc);
5fc11eebb4a98d Christoph Hellwig 2021-12-09  302  			ioc = task->io_context;
5fc11eebb4a98d Christoph Hellwig 2021-12-09  303  		} else {
5fc11eebb4a98d Christoph Hellwig 2021-12-09  304  			task->io_context = ioc;
a411cd3cfdc5bb Christoph Hellwig 2021-12-09  305  		}
8472161b77c41d Christoph Hellwig 2021-12-09  306  	}
8472161b77c41d Christoph Hellwig 2021-12-09 @307  	task->io_context->ioprio = ioprio;
8472161b77c41d Christoph Hellwig 2021-12-09  308  	task_unlock(task);
8472161b77c41d Christoph Hellwig 2021-12-09  309  	return 0;
a411cd3cfdc5bb Christoph Hellwig 2021-12-09  310  }
a411cd3cfdc5bb Christoph Hellwig 2021-12-09  311  EXPORT_SYMBOL_GPL(set_task_ioprio);
a411cd3cfdc5bb Christoph Hellwig 2021-12-09  312  

:::::: The code at line 307 was first introduced by commit
:::::: 8472161b77c41d260c5ba0af6bf940269b297bb6 block: fold get_task_io_context into set_task_ioprio

:::::: TO: Christoph Hellwig <hch@lst.de>
:::::: CC: Jens Axboe <axboe@kernel.dk>

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

^ permalink raw reply	[flat|nested] 9+ messages in thread
* [axboe-block:for-5.17/block 106/108] block/blk-ioc.c:307 set_task_ioprio() error: we previously assumed 'task->io_context' could be null (see line 300)
@ 2021-12-17  7:49 ` Dan Carpenter
  0 siblings, 0 replies; 9+ messages in thread
From: Dan Carpenter @ 2021-12-17  7:49 UTC (permalink / raw)
  To: kbuild, Christoph Hellwig
  Cc: lkp, kbuild-all, linux-kernel, Jens Axboe, Jan Kara

tree:   https://git.kernel.org/pub/scm/linux/kernel/git/axboe/linux-block.git for-5.17/block
head:   5ef1630586317e92c9ebd7b4ce48f393b7ff790f
commit: 5fc11eebb4a98df5324a4de369bb5ab7f0007ff7 [106/108] block: open code create_task_io_context in set_task_ioprio
config: i386-randconfig-m021-20211216 (https://download.01.org/0day-ci/archive/20211217/202112171520.5hNnOM0q-lkp@intel.com/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 <lkp@intel.com>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>

smatch warnings:
block/blk-ioc.c:307 set_task_ioprio() error: we previously assumed 'task->io_context' could be null (see line 300)

vim +307 block/blk-ioc.c

a411cd3cfdc5bb Christoph Hellwig 2021-12-09  271  int set_task_ioprio(struct task_struct *task, int ioprio)
a411cd3cfdc5bb Christoph Hellwig 2021-12-09  272  {
a411cd3cfdc5bb Christoph Hellwig 2021-12-09  273  	int err;
a411cd3cfdc5bb Christoph Hellwig 2021-12-09  274  	const struct cred *cred = current_cred(), *tcred;
a411cd3cfdc5bb Christoph Hellwig 2021-12-09  275  
a411cd3cfdc5bb Christoph Hellwig 2021-12-09  276  	rcu_read_lock();
a411cd3cfdc5bb Christoph Hellwig 2021-12-09  277  	tcred = __task_cred(task);
a411cd3cfdc5bb Christoph Hellwig 2021-12-09  278  	if (!uid_eq(tcred->uid, cred->euid) &&
a411cd3cfdc5bb Christoph Hellwig 2021-12-09  279  	    !uid_eq(tcred->uid, cred->uid) && !capable(CAP_SYS_NICE)) {
a411cd3cfdc5bb Christoph Hellwig 2021-12-09  280  		rcu_read_unlock();
a411cd3cfdc5bb Christoph Hellwig 2021-12-09  281  		return -EPERM;
a411cd3cfdc5bb Christoph Hellwig 2021-12-09  282  	}
a411cd3cfdc5bb Christoph Hellwig 2021-12-09  283  	rcu_read_unlock();
a411cd3cfdc5bb Christoph Hellwig 2021-12-09  284  
a411cd3cfdc5bb Christoph Hellwig 2021-12-09  285  	err = security_task_setioprio(task, ioprio);
a411cd3cfdc5bb Christoph Hellwig 2021-12-09  286  	if (err)
a411cd3cfdc5bb Christoph Hellwig 2021-12-09  287  		return err;
a411cd3cfdc5bb Christoph Hellwig 2021-12-09  288  
8472161b77c41d Christoph Hellwig 2021-12-09  289  	task_lock(task);
8472161b77c41d Christoph Hellwig 2021-12-09  290  	if (unlikely(!task->io_context)) {
8472161b77c41d Christoph Hellwig 2021-12-09  291  		struct io_context *ioc;
8472161b77c41d Christoph Hellwig 2021-12-09  292  
8472161b77c41d Christoph Hellwig 2021-12-09  293  		task_unlock(task);
5fc11eebb4a98d Christoph Hellwig 2021-12-09  294  
5fc11eebb4a98d Christoph Hellwig 2021-12-09  295  		ioc = alloc_io_context(GFP_ATOMIC, NUMA_NO_NODE);
5fc11eebb4a98d Christoph Hellwig 2021-12-09  296  		if (!ioc)
5fc11eebb4a98d Christoph Hellwig 2021-12-09  297  			return -ENOMEM;
5fc11eebb4a98d Christoph Hellwig 2021-12-09  298  
5fc11eebb4a98d Christoph Hellwig 2021-12-09  299  		task_lock(task);
5fc11eebb4a98d Christoph Hellwig 2021-12-09 @300  		if (task->io_context || (task->flags & PF_EXITING)) {
                                                                                         ^^^^^^^^^^^^^^^^^^^^^^^^
Assume "task->io_context" is NULL but PF_EXITING is set.

5fc11eebb4a98d Christoph Hellwig 2021-12-09  301  			kmem_cache_free(iocontext_cachep, ioc);
5fc11eebb4a98d Christoph Hellwig 2021-12-09  302  			ioc = task->io_context;
5fc11eebb4a98d Christoph Hellwig 2021-12-09  303  		} else {
5fc11eebb4a98d Christoph Hellwig 2021-12-09  304  			task->io_context = ioc;
a411cd3cfdc5bb Christoph Hellwig 2021-12-09  305  		}
8472161b77c41d Christoph Hellwig 2021-12-09  306  	}
8472161b77c41d Christoph Hellwig 2021-12-09 @307  	task->io_context->ioprio = ioprio;
                                                        ^^^^^^^^^^^^^^^^^^
Unchecked dereference.

8472161b77c41d Christoph Hellwig 2021-12-09  308  	task_unlock(task);
8472161b77c41d Christoph Hellwig 2021-12-09  309  	return 0;
a411cd3cfdc5bb Christoph Hellwig 2021-12-09  310  }

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org


^ permalink raw reply	[flat|nested] 9+ messages in thread
* [axboe-block:for-5.17/block 106/108] block/blk-ioc.c:307 set_task_ioprio() error: we previously assumed 'task->io_context' could be null (see line 300)
@ 2021-12-17  7:49 ` Dan Carpenter
  0 siblings, 0 replies; 9+ messages in thread
From: Dan Carpenter @ 2021-12-17  7:49 UTC (permalink / raw)
  To: kbuild-all

[-- Attachment #1: Type: text/plain, Size: 4062 bytes --]

tree:   https://git.kernel.org/pub/scm/linux/kernel/git/axboe/linux-block.git for-5.17/block
head:   5ef1630586317e92c9ebd7b4ce48f393b7ff790f
commit: 5fc11eebb4a98df5324a4de369bb5ab7f0007ff7 [106/108] block: open code create_task_io_context in set_task_ioprio
config: i386-randconfig-m021-20211216 (https://download.01.org/0day-ci/archive/20211217/202112171520.5hNnOM0q-lkp(a)intel.com/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 <lkp@intel.com>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>

smatch warnings:
block/blk-ioc.c:307 set_task_ioprio() error: we previously assumed 'task->io_context' could be null (see line 300)

vim +307 block/blk-ioc.c

a411cd3cfdc5bb Christoph Hellwig 2021-12-09  271  int set_task_ioprio(struct task_struct *task, int ioprio)
a411cd3cfdc5bb Christoph Hellwig 2021-12-09  272  {
a411cd3cfdc5bb Christoph Hellwig 2021-12-09  273  	int err;
a411cd3cfdc5bb Christoph Hellwig 2021-12-09  274  	const struct cred *cred = current_cred(), *tcred;
a411cd3cfdc5bb Christoph Hellwig 2021-12-09  275  
a411cd3cfdc5bb Christoph Hellwig 2021-12-09  276  	rcu_read_lock();
a411cd3cfdc5bb Christoph Hellwig 2021-12-09  277  	tcred = __task_cred(task);
a411cd3cfdc5bb Christoph Hellwig 2021-12-09  278  	if (!uid_eq(tcred->uid, cred->euid) &&
a411cd3cfdc5bb Christoph Hellwig 2021-12-09  279  	    !uid_eq(tcred->uid, cred->uid) && !capable(CAP_SYS_NICE)) {
a411cd3cfdc5bb Christoph Hellwig 2021-12-09  280  		rcu_read_unlock();
a411cd3cfdc5bb Christoph Hellwig 2021-12-09  281  		return -EPERM;
a411cd3cfdc5bb Christoph Hellwig 2021-12-09  282  	}
a411cd3cfdc5bb Christoph Hellwig 2021-12-09  283  	rcu_read_unlock();
a411cd3cfdc5bb Christoph Hellwig 2021-12-09  284  
a411cd3cfdc5bb Christoph Hellwig 2021-12-09  285  	err = security_task_setioprio(task, ioprio);
a411cd3cfdc5bb Christoph Hellwig 2021-12-09  286  	if (err)
a411cd3cfdc5bb Christoph Hellwig 2021-12-09  287  		return err;
a411cd3cfdc5bb Christoph Hellwig 2021-12-09  288  
8472161b77c41d Christoph Hellwig 2021-12-09  289  	task_lock(task);
8472161b77c41d Christoph Hellwig 2021-12-09  290  	if (unlikely(!task->io_context)) {
8472161b77c41d Christoph Hellwig 2021-12-09  291  		struct io_context *ioc;
8472161b77c41d Christoph Hellwig 2021-12-09  292  
8472161b77c41d Christoph Hellwig 2021-12-09  293  		task_unlock(task);
5fc11eebb4a98d Christoph Hellwig 2021-12-09  294  
5fc11eebb4a98d Christoph Hellwig 2021-12-09  295  		ioc = alloc_io_context(GFP_ATOMIC, NUMA_NO_NODE);
5fc11eebb4a98d Christoph Hellwig 2021-12-09  296  		if (!ioc)
5fc11eebb4a98d Christoph Hellwig 2021-12-09  297  			return -ENOMEM;
5fc11eebb4a98d Christoph Hellwig 2021-12-09  298  
5fc11eebb4a98d Christoph Hellwig 2021-12-09  299  		task_lock(task);
5fc11eebb4a98d Christoph Hellwig 2021-12-09 @300  		if (task->io_context || (task->flags & PF_EXITING)) {
                                                                                         ^^^^^^^^^^^^^^^^^^^^^^^^
Assume "task->io_context" is NULL but PF_EXITING is set.

5fc11eebb4a98d Christoph Hellwig 2021-12-09  301  			kmem_cache_free(iocontext_cachep, ioc);
5fc11eebb4a98d Christoph Hellwig 2021-12-09  302  			ioc = task->io_context;
5fc11eebb4a98d Christoph Hellwig 2021-12-09  303  		} else {
5fc11eebb4a98d Christoph Hellwig 2021-12-09  304  			task->io_context = ioc;
a411cd3cfdc5bb Christoph Hellwig 2021-12-09  305  		}
8472161b77c41d Christoph Hellwig 2021-12-09  306  	}
8472161b77c41d Christoph Hellwig 2021-12-09 @307  	task->io_context->ioprio = ioprio;
                                                        ^^^^^^^^^^^^^^^^^^
Unchecked dereference.

8472161b77c41d Christoph Hellwig 2021-12-09  308  	task_unlock(task);
8472161b77c41d Christoph Hellwig 2021-12-09  309  	return 0;
a411cd3cfdc5bb Christoph Hellwig 2021-12-09  310  }

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

^ permalink raw reply	[flat|nested] 9+ messages in thread
* Re: [axboe-block:for-5.17/block 106/108] block/blk-ioc.c:307 set_task_ioprio() error: we previously assumed 'task->io_context' could be null (see line 300)
  2021-12-17  7:49 ` Dan Carpenter
@ 2021-12-21  2:57   ` Eric Dumazet
  -1 siblings, 0 replies; 9+ messages in thread
From: Eric Dumazet @ 2021-12-21  2:57 UTC (permalink / raw)
  To: Dan Carpenter, kbuild, Christoph Hellwig
  Cc: lkp, kbuild-all, linux-kernel, Jens Axboe, Jan Kara


On 12/16/21 11:49 PM, Dan Carpenter wrote:
> tree:   https://git.kernel.org/pub/scm/linux/kernel/git/axboe/linux-block.git for-5.17/block
> head:   5ef1630586317e92c9ebd7b4ce48f393b7ff790f
> commit: 5fc11eebb4a98df5324a4de369bb5ab7f0007ff7 [106/108] block: open code create_task_io_context in set_task_ioprio
> config: i386-randconfig-m021-20211216 (https://download.01.org/0day-ci/archive/20211217/202112171520.5hNnOM0q-lkp@intel.com/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 <lkp@intel.com>
> Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
>
> smatch warnings:
> block/blk-ioc.c:307 set_task_ioprio() error: we previously assumed 'task->io_context' could be null (see line 300)
>
> vim +307 block/blk-ioc.c
>
> a411cd3cfdc5bb Christoph Hellwig 2021-12-09  271  int set_task_ioprio(struct task_struct *task, int ioprio)
> a411cd3cfdc5bb Christoph Hellwig 2021-12-09  272  {
> a411cd3cfdc5bb Christoph Hellwig 2021-12-09  273  	int err;
> a411cd3cfdc5bb Christoph Hellwig 2021-12-09  274  	const struct cred *cred = current_cred(), *tcred;
> a411cd3cfdc5bb Christoph Hellwig 2021-12-09  275
> a411cd3cfdc5bb Christoph Hellwig 2021-12-09  276  	rcu_read_lock();
> a411cd3cfdc5bb Christoph Hellwig 2021-12-09  277  	tcred = __task_cred(task);
> a411cd3cfdc5bb Christoph Hellwig 2021-12-09  278  	if (!uid_eq(tcred->uid, cred->euid) &&
> a411cd3cfdc5bb Christoph Hellwig 2021-12-09  279  	    !uid_eq(tcred->uid, cred->uid) && !capable(CAP_SYS_NICE)) {
> a411cd3cfdc5bb Christoph Hellwig 2021-12-09  280  		rcu_read_unlock();
> a411cd3cfdc5bb Christoph Hellwig 2021-12-09  281  		return -EPERM;
> a411cd3cfdc5bb Christoph Hellwig 2021-12-09  282  	}
> a411cd3cfdc5bb Christoph Hellwig 2021-12-09  283  	rcu_read_unlock();
> a411cd3cfdc5bb Christoph Hellwig 2021-12-09  284
> a411cd3cfdc5bb Christoph Hellwig 2021-12-09  285  	err = security_task_setioprio(task, ioprio);
> a411cd3cfdc5bb Christoph Hellwig 2021-12-09  286  	if (err)
> a411cd3cfdc5bb Christoph Hellwig 2021-12-09  287  		return err;
> a411cd3cfdc5bb Christoph Hellwig 2021-12-09  288
> 8472161b77c41d Christoph Hellwig 2021-12-09  289  	task_lock(task);
> 8472161b77c41d Christoph Hellwig 2021-12-09  290  	if (unlikely(!task->io_context)) {
> 8472161b77c41d Christoph Hellwig 2021-12-09  291  		struct io_context *ioc;
> 8472161b77c41d Christoph Hellwig 2021-12-09  292
> 8472161b77c41d Christoph Hellwig 2021-12-09  293  		task_unlock(task);
> 5fc11eebb4a98d Christoph Hellwig 2021-12-09  294
> 5fc11eebb4a98d Christoph Hellwig 2021-12-09  295  		ioc = alloc_io_context(GFP_ATOMIC, NUMA_NO_NODE);
> 5fc11eebb4a98d Christoph Hellwig 2021-12-09  296  		if (!ioc)
> 5fc11eebb4a98d Christoph Hellwig 2021-12-09  297  			return -ENOMEM;
> 5fc11eebb4a98d Christoph Hellwig 2021-12-09  298
> 5fc11eebb4a98d Christoph Hellwig 2021-12-09  299  		task_lock(task);
> 5fc11eebb4a98d Christoph Hellwig 2021-12-09 @300  		if (task->io_context || (task->flags & PF_EXITING)) {
>                                                                                           ^^^^^^^^^^^^^^^^^^^^^^^^
> Assume "task->io_context" is NULL but PF_EXITING is set.
>
> 5fc11eebb4a98d Christoph Hellwig 2021-12-09  301  			kmem_cache_free(iocontext_cachep, ioc);
> 5fc11eebb4a98d Christoph Hellwig 2021-12-09  302  			ioc = task->io_context;
> 5fc11eebb4a98d Christoph Hellwig 2021-12-09  303  		} else {
> 5fc11eebb4a98d Christoph Hellwig 2021-12-09  304  			task->io_context = ioc;
> a411cd3cfdc5bb Christoph Hellwig 2021-12-09  305  		}
> 8472161b77c41d Christoph Hellwig 2021-12-09  306  	}
> 8472161b77c41d Christoph Hellwig 2021-12-09 @307  	task->io_context->ioprio = ioprio;
>                                                          ^^^^^^^^^^^^^^^^^^
> Unchecked dereference.


I will release a syzbot report with a repro like this:

   syscall(__NR_clone, 0x3a3dd4008400af01ul, 0ul, 0x9999999999999999ul, 0ul,
           -1ul);
   syscall(__NR_ioprio_set, 3ul, 0, 0ul);





^ permalink raw reply	[flat|nested] 9+ messages in thread
* Re: [axboe-block:for-5.17/block 106/108] block/blk-ioc.c:307 set_task_ioprio() error: we previously assumed 'task->io_context' could be null (see line 300)
@ 2021-12-21  2:57   ` Eric Dumazet
  0 siblings, 0 replies; 9+ messages in thread
From: Eric Dumazet @ 2021-12-21  2:57 UTC (permalink / raw)
  To: kbuild-all

[-- Attachment #1: Type: text/plain, Size: 4118 bytes --]


On 12/16/21 11:49 PM, Dan Carpenter wrote:
> tree:   https://git.kernel.org/pub/scm/linux/kernel/git/axboe/linux-block.git for-5.17/block
> head:   5ef1630586317e92c9ebd7b4ce48f393b7ff790f
> commit: 5fc11eebb4a98df5324a4de369bb5ab7f0007ff7 [106/108] block: open code create_task_io_context in set_task_ioprio
> config: i386-randconfig-m021-20211216 (https://download.01.org/0day-ci/archive/20211217/202112171520.5hNnOM0q-lkp(a)intel.com/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 <lkp@intel.com>
> Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
>
> smatch warnings:
> block/blk-ioc.c:307 set_task_ioprio() error: we previously assumed 'task->io_context' could be null (see line 300)
>
> vim +307 block/blk-ioc.c
>
> a411cd3cfdc5bb Christoph Hellwig 2021-12-09  271  int set_task_ioprio(struct task_struct *task, int ioprio)
> a411cd3cfdc5bb Christoph Hellwig 2021-12-09  272  {
> a411cd3cfdc5bb Christoph Hellwig 2021-12-09  273  	int err;
> a411cd3cfdc5bb Christoph Hellwig 2021-12-09  274  	const struct cred *cred = current_cred(), *tcred;
> a411cd3cfdc5bb Christoph Hellwig 2021-12-09  275
> a411cd3cfdc5bb Christoph Hellwig 2021-12-09  276  	rcu_read_lock();
> a411cd3cfdc5bb Christoph Hellwig 2021-12-09  277  	tcred = __task_cred(task);
> a411cd3cfdc5bb Christoph Hellwig 2021-12-09  278  	if (!uid_eq(tcred->uid, cred->euid) &&
> a411cd3cfdc5bb Christoph Hellwig 2021-12-09  279  	    !uid_eq(tcred->uid, cred->uid) && !capable(CAP_SYS_NICE)) {
> a411cd3cfdc5bb Christoph Hellwig 2021-12-09  280  		rcu_read_unlock();
> a411cd3cfdc5bb Christoph Hellwig 2021-12-09  281  		return -EPERM;
> a411cd3cfdc5bb Christoph Hellwig 2021-12-09  282  	}
> a411cd3cfdc5bb Christoph Hellwig 2021-12-09  283  	rcu_read_unlock();
> a411cd3cfdc5bb Christoph Hellwig 2021-12-09  284
> a411cd3cfdc5bb Christoph Hellwig 2021-12-09  285  	err = security_task_setioprio(task, ioprio);
> a411cd3cfdc5bb Christoph Hellwig 2021-12-09  286  	if (err)
> a411cd3cfdc5bb Christoph Hellwig 2021-12-09  287  		return err;
> a411cd3cfdc5bb Christoph Hellwig 2021-12-09  288
> 8472161b77c41d Christoph Hellwig 2021-12-09  289  	task_lock(task);
> 8472161b77c41d Christoph Hellwig 2021-12-09  290  	if (unlikely(!task->io_context)) {
> 8472161b77c41d Christoph Hellwig 2021-12-09  291  		struct io_context *ioc;
> 8472161b77c41d Christoph Hellwig 2021-12-09  292
> 8472161b77c41d Christoph Hellwig 2021-12-09  293  		task_unlock(task);
> 5fc11eebb4a98d Christoph Hellwig 2021-12-09  294
> 5fc11eebb4a98d Christoph Hellwig 2021-12-09  295  		ioc = alloc_io_context(GFP_ATOMIC, NUMA_NO_NODE);
> 5fc11eebb4a98d Christoph Hellwig 2021-12-09  296  		if (!ioc)
> 5fc11eebb4a98d Christoph Hellwig 2021-12-09  297  			return -ENOMEM;
> 5fc11eebb4a98d Christoph Hellwig 2021-12-09  298
> 5fc11eebb4a98d Christoph Hellwig 2021-12-09  299  		task_lock(task);
> 5fc11eebb4a98d Christoph Hellwig 2021-12-09 @300  		if (task->io_context || (task->flags & PF_EXITING)) {
>                                                                                           ^^^^^^^^^^^^^^^^^^^^^^^^
> Assume "task->io_context" is NULL but PF_EXITING is set.
>
> 5fc11eebb4a98d Christoph Hellwig 2021-12-09  301  			kmem_cache_free(iocontext_cachep, ioc);
> 5fc11eebb4a98d Christoph Hellwig 2021-12-09  302  			ioc = task->io_context;
> 5fc11eebb4a98d Christoph Hellwig 2021-12-09  303  		} else {
> 5fc11eebb4a98d Christoph Hellwig 2021-12-09  304  			task->io_context = ioc;
> a411cd3cfdc5bb Christoph Hellwig 2021-12-09  305  		}
> 8472161b77c41d Christoph Hellwig 2021-12-09  306  	}
> 8472161b77c41d Christoph Hellwig 2021-12-09 @307  	task->io_context->ioprio = ioprio;
>                                                          ^^^^^^^^^^^^^^^^^^
> Unchecked dereference.


I will release a syzbot report with a repro like this:

   syscall(__NR_clone, 0x3a3dd4008400af01ul, 0ul, 0x9999999999999999ul, 0ul,
           -1ul);
   syscall(__NR_ioprio_set, 3ul, 0, 0ul);




^ permalink raw reply	[flat|nested] 9+ messages in thread
* Re: [axboe-block:for-5.17/block 106/108] block/blk-ioc.c:307 set_task_ioprio() error: we previously assumed 'task->io_context' could be null (see line 300)
  2021-12-17  7:49 ` Dan Carpenter
@ 2021-12-21  3:31   ` Jens Axboe
  -1 siblings, 0 replies; 9+ messages in thread
From: Jens Axboe @ 2021-12-21  3:31 UTC (permalink / raw)
  To: Dan Carpenter, kbuild, Christoph Hellwig
  Cc: lkp, kbuild-all, linux-kernel, Jan Kara

On 12/17/21 12:49 AM, Dan Carpenter wrote:
> tree:   https://git.kernel.org/pub/scm/linux/kernel/git/axboe/linux-block.git for-5.17/block
> head:   5ef1630586317e92c9ebd7b4ce48f393b7ff790f
> commit: 5fc11eebb4a98df5324a4de369bb5ab7f0007ff7 [106/108] block: open code create_task_io_context in set_task_ioprio
> config: i386-randconfig-m021-20211216 (https://download.01.org/0day-ci/archive/20211217/202112171520.5hNnOM0q-lkp@intel.com/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 <lkp@intel.com>
> Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
> 
> smatch warnings:
> block/blk-ioc.c:307 set_task_ioprio() error: we previously assumed 'task->io_context' could be null (see line 300)

This should fix it:


diff --git a/block/blk-ioc.c b/block/blk-ioc.c
index 87bdc9ca8295..71c3a933cf16 100644
--- a/block/blk-ioc.c
+++ b/block/blk-ioc.c
@@ -279,7 +279,12 @@ int set_task_ioprio(struct task_struct *task, int ioprio)
 			return -ENOMEM;
 
 		task_lock(task);
-		if (task->io_context || (task->flags & PF_EXITING)) {
+		if (task->flags & PF_EXITING) {
+			err = -ESRCH;
+			kmem_cache_free(iocontext_cachep, ioc);
+			goto out;
+		}
+		if (task->io_context) {
 			kmem_cache_free(iocontext_cachep, ioc);
 			ioc = task->io_context;
 		} else {
@@ -287,8 +292,9 @@ int set_task_ioprio(struct task_struct *task, int ioprio)
 		}
 	}
 	task->io_context->ioprio = ioprio;
+out:
 	task_unlock(task);
-	return 0;
+	return err;
 }
 EXPORT_SYMBOL_GPL(set_task_ioprio);
 

-- 
Jens Axboe


^ permalink raw reply related	[flat|nested] 9+ messages in thread
* Re: [axboe-block:for-5.17/block 106/108] block/blk-ioc.c:307 set_task_ioprio() error: we previously assumed 'task->io_context' could be null (see line 300)
@ 2021-12-21  3:31   ` Jens Axboe
  0 siblings, 0 replies; 9+ messages in thread
From: Jens Axboe @ 2021-12-21  3:31 UTC (permalink / raw)
  To: kbuild-all

[-- Attachment #1: Type: text/plain, Size: 1641 bytes --]

On 12/17/21 12:49 AM, Dan Carpenter wrote:
> tree:   https://git.kernel.org/pub/scm/linux/kernel/git/axboe/linux-block.git for-5.17/block
> head:   5ef1630586317e92c9ebd7b4ce48f393b7ff790f
> commit: 5fc11eebb4a98df5324a4de369bb5ab7f0007ff7 [106/108] block: open code create_task_io_context in set_task_ioprio
> config: i386-randconfig-m021-20211216 (https://download.01.org/0day-ci/archive/20211217/202112171520.5hNnOM0q-lkp(a)intel.com/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 <lkp@intel.com>
> Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
> 
> smatch warnings:
> block/blk-ioc.c:307 set_task_ioprio() error: we previously assumed 'task->io_context' could be null (see line 300)

This should fix it:


diff --git a/block/blk-ioc.c b/block/blk-ioc.c
index 87bdc9ca8295..71c3a933cf16 100644
--- a/block/blk-ioc.c
+++ b/block/blk-ioc.c
@@ -279,7 +279,12 @@ int set_task_ioprio(struct task_struct *task, int ioprio)
 			return -ENOMEM;
 
 		task_lock(task);
-		if (task->io_context || (task->flags & PF_EXITING)) {
+		if (task->flags & PF_EXITING) {
+			err = -ESRCH;
+			kmem_cache_free(iocontext_cachep, ioc);
+			goto out;
+		}
+		if (task->io_context) {
 			kmem_cache_free(iocontext_cachep, ioc);
 			ioc = task->io_context;
 		} else {
@@ -287,8 +292,9 @@ int set_task_ioprio(struct task_struct *task, int ioprio)
 		}
 	}
 	task->io_context->ioprio = ioprio;
+out:
 	task_unlock(task);
-	return 0;
+	return err;
 }
 EXPORT_SYMBOL_GPL(set_task_ioprio);
 

-- 
Jens Axboe

^ permalink raw reply related	[flat|nested] 9+ messages in thread
* Re: [axboe-block:for-5.17/block 106/108] block/blk-ioc.c:307 set_task_ioprio() error: we previously assumed 'task->io_context' could be null (see line 300)
  2021-12-21  3:31   ` Jens Axboe
@ 2021-12-21  8:28     ` Christoph Hellwig
  -1 siblings, 0 replies; 9+ messages in thread
From: Christoph Hellwig @ 2021-12-21  8:28 UTC (permalink / raw)
  To: Jens Axboe
  Cc: Dan Carpenter, kbuild, Christoph Hellwig, lkp, kbuild-all,
	linux-kernel, Jan Kara

Looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>

^ permalink raw reply	[flat|nested] 9+ messages in thread
* Re: [axboe-block:for-5.17/block 106/108] block/blk-ioc.c:307 set_task_ioprio() error: we previously assumed 'task->io_context' could be null (see line 300)
@ 2021-12-21  8:28     ` Christoph Hellwig
  0 siblings, 0 replies; 9+ messages in thread
From: Christoph Hellwig @ 2021-12-21  8:28 UTC (permalink / raw)
  To: kbuild-all

[-- Attachment #1: Type: text/plain, Size: 58 bytes --]

Looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>

^ permalink raw reply	[flat|nested] 9+ messages in thread
end of thread, other threads:[~2021-12-21  8:28 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-12-17  7:40 [axboe-block:for-5.17/block 106/108] block/blk-ioc.c:307 set_task_ioprio() error: we previously assumed 'task->io_context' could be null (see line 300) kernel test robot
2021-12-17  7:49 ` Dan Carpenter
2021-12-17  7:49 ` Dan Carpenter
2021-12-21  2:57 ` Eric Dumazet
2021-12-21  2:57   ` Eric Dumazet
2021-12-21  3:31 ` Jens Axboe
2021-12-21  3:31   ` Jens Axboe
2021-12-21  8:28   ` Christoph Hellwig
2021-12-21  8:28     ` Christoph Hellwig

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.