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)

All of lore.kernel.org
 help / color / mirror / Atom feed

From: Eric Dumazet <eric.dumazet@gmail.com>
To: kbuild-all@lists.01.org
Subject: 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)
Date: Mon, 20 Dec 2021 18:57:35 -0800	[thread overview]
Message-ID: <9ebee41f-74e6-df29-35aa-d662cdca5c17@gmail.com> (raw)
In-Reply-To: <202112171520.5hNnOM0q-lkp@intel.com>

[-- 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);

WARNING: multiple messages have this Message-ID (diff)

From: Eric Dumazet <eric.dumazet@gmail.com>
To: Dan Carpenter <dan.carpenter@oracle.com>,
	kbuild@lists.01.org, Christoph Hellwig <hch@lst.de>
Cc: lkp@intel.com, kbuild-all@lists.01.org,
	linux-kernel@vger.kernel.org, Jens Axboe <axboe@kernel.dk>,
	Jan Kara <jack@suse.cz>
Subject: 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)
Date: Mon, 20 Dec 2021 18:57:35 -0800	[thread overview]
Message-ID: <9ebee41f-74e6-df29-35aa-d662cdca5c17@gmail.com> (raw)
In-Reply-To: <202112171520.5hNnOM0q-lkp@intel.com>


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);

next prev parent reply	other threads:[~2021-12-21  2:57 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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 [this message]
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

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=9ebee41f-74e6-df29-35aa-d662cdca5c17@gmail.com \
    --to=eric.dumazet@gmail.com \
    --cc=kbuild-all@lists.01.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Be sure your reply has a Subject: header at the top and a blank line before the message body.

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.