linux-kernel - Re: [PATCH 17/17] block, bfq: remove unnecessary local variable __bfqq in bfq_setup

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 23 Feb 2023 14:34:33 +0800
From:   Kemeng Shi <shikemeng@...weicloud.com>
To:     Dan Carpenter <error27@...il.com>, oe-kbuild@...ts.linux.dev,
        paolo.valente@...aro.org, axboe@...nel.dk, jack@...e.cz
Cc:     lkp@...el.com, oe-kbuild-all@...ts.linux.dev,
        linux-block@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 17/17] block, bfq: remove unnecessary local variable
 __bfqq in bfq_setup_merge



on 2/23/2023 1:48 PM, Dan Carpenter wrote:
> Hi Kemeng,
> 
> https://git-scm.com/docs/git-format-patch#_base_tree_information]
> 
> url:    https://github.com/intel-lab-lkp/linux/commits/Kemeng-Shi/block-bfq-properly-mark-bfqq-remained-idle/20230219-104312
> base:   https://git.kernel.org/pub/scm/linux/kernel/git/axboe/linux-block.git for-next
> patch link:    https://lore.kernel.org/r/20230219104309.1511562-18-shikemeng%40huaweicloud.com
> patch subject: [PATCH 17/17] block, bfq: remove unnecessary local variable __bfqq in bfq_setup_merge
> config: openrisc-randconfig-m041-20230219 (https://download.01.org/0day-ci/archive/20230220/202302200841.9zinyY7i-lkp@intel.com/config)
> compiler: or1k-linux-gcc (GCC) 12.1.0
> 
> If you fix the issue, kindly add following tag where applicable
> | Reported-by: kernel test robot <lkp@...el.com>
> | Reported-by: Dan Carpenter <error27@...il.com>
> | Link: https://lore.kernel.org/r/202302200841.9zinyY7i-lkp@intel.com/
> 
> New smatch warnings:
> block/bfq-iosched.c:2785 bfq_setup_merge() error: we previously assumed 'new_bfqq' could be null (see line 2766)
> 
> Old smatch warnings:
> block/bfq-iosched.c:6195 __bfq_insert_request() warn: variable dereferenced before check 'bfqq' (see line 6191)
> 
> vim +/new_bfqq +2785 block/bfq-iosched.c
> 
> 36eca894832351 Arianna Avanzini 2017-04-12  2751  static struct bfq_queue *
> 36eca894832351 Arianna Avanzini 2017-04-12  2752  bfq_setup_merge(struct bfq_queue *bfqq, struct bfq_queue *new_bfqq)
> 36eca894832351 Arianna Avanzini 2017-04-12  2753  {
> 36eca894832351 Arianna Avanzini 2017-04-12  2754  	int process_refs, new_process_refs;
> 36eca894832351 Arianna Avanzini 2017-04-12  2755  
> 36eca894832351 Arianna Avanzini 2017-04-12  2756  	/*
> 36eca894832351 Arianna Avanzini 2017-04-12  2757  	 * If there are no process references on the new_bfqq, then it is
> 36eca894832351 Arianna Avanzini 2017-04-12  2758  	 * unsafe to follow the ->new_bfqq chain as other bfqq's in the chain
> 36eca894832351 Arianna Avanzini 2017-04-12  2759  	 * may have dropped their last reference (not just their last process
> 36eca894832351 Arianna Avanzini 2017-04-12  2760  	 * reference).
> 36eca894832351 Arianna Avanzini 2017-04-12  2761  	 */
> 36eca894832351 Arianna Avanzini 2017-04-12  2762  	if (!bfqq_process_refs(new_bfqq))
> 36eca894832351 Arianna Avanzini 2017-04-12  2763  		return NULL;
> 36eca894832351 Arianna Avanzini 2017-04-12  2764  
> 36eca894832351 Arianna Avanzini 2017-04-12  2765  	/* Avoid a circular list and skip interim queue merges. */
> 114533e1e26a36 Kemeng Shi       2023-02-19 @2766  	while ((new_bfqq = new_bfqq->new_bfqq)) {
> 114533e1e26a36 Kemeng Shi       2023-02-19  2767  		if (new_bfqq == bfqq)
> 36eca894832351 Arianna Avanzini 2017-04-12  2768  			return NULL;
> 36eca894832351 Arianna Avanzini 2017-04-12  2769  	}
> 
> This now loops until new_bfqq is NULL.
> 
> 36eca894832351 Arianna Avanzini 2017-04-12  2770  
> 36eca894832351 Arianna Avanzini 2017-04-12  2771  	process_refs = bfqq_process_refs(bfqq);
> 36eca894832351 Arianna Avanzini 2017-04-12  2772  	new_process_refs = bfqq_process_refs(new_bfqq);
> 
> What?Sorry, I didn't expect that there is a behavior change. I will drop
this one in next version. Thanks.

-- 
Best wishes
Kemeng Shi