linux-kernel - re: f2fs: factor out discard_cmd usage from general rb

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-next>] [day] [month] [year] [list]

Message-ID: <e50ebe1a-73a0-5800-71e3-0ddd366727ac@gmail.com>
Date:   Fri, 24 Mar 2023 11:21:11 +0000
From:   "Colin King (gmail)" <colin.i.king@...il.com>
To:     Jaegeuk Kim <jaegeuk@...nel.org>, Chao Yu <chao@...nel.org>,
        linux-f2fs-devel@...ts.sourceforge.net
Cc:     "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: re: f2fs: factor out discard_cmd usage from general rb_tree use

Hi,

static analysis with clang scan build has detected a potential issue 
introduced by the following commit:

commit 7e9775a516ff6c1e73ee2b42ec563cafee38f42f
Author: Jaegeuk Kim <jaegeuk@...nel.org>
Date:   Fri Mar 10 11:12:35 2023 -0800

f2fs: factor out discard_cmd usage from general rb_tree use


The warning is as follows:

fs/f2fs/segment.c:1425:4: warning: Value stored to 'tdc' is never read 
[deadcode.DeadStores]

The while loop in function __update_discard_tree_range is as follows (+ 
my annotations):


         while (1) {
                 struct rb_node *node;
                 struct discard_cmd *tdc = NULL;

### tdc is set to NULL

                 if (prev_dc) {
                         di.lstart = prev_dc->di.lstart + prev_dc->di.len;
                         if (di.lstart < lstart)
                                 di.lstart = lstart;
                         if (di.lstart >= end)
                                 break;

                         if (!next_dc || next_dc->di.lstart > end)
                                 di.len = end - di.lstart;
                         else
                                 di.len = next_dc->di.lstart - di.lstart;
                         di.start = start + di.lstart - lstart;
                 }

                 if (!di.len)
                         goto next;

                 if (prev_dc && prev_dc->state == D_PREP &&
                         prev_dc->bdev == bdev &&
                         __is_discard_back_mergeable(&di, &prev_dc->di,
 
max_discard_blocks)) {
                         prev_dc->di.len += di.len;
                         dcc->undiscard_blks += di.len;
                         __relocate_discard_cmd(dcc, prev_dc);
                         di = prev_dc->di;
                         tdc = prev_dc;

### tdc is set to prev_dc, however, it is not not read any more with th 
introduction of the "goto next"" statement introduced in the commit 
mentioned earlier

                         goto next;
                 }

                 if (next_dc && next_dc->state == D_PREP &&
                         next_dc->bdev == bdev &&
                         __is_discard_front_mergeable(&di, &next_dc->di,
 
max_discard_blocks)) {
                         next_dc->di.lstart = di.lstart;
                         next_dc->di.len += di.len;
                         next_dc->di.start = di.start;
                         dcc->undiscard_blks += di.len;
                         __relocate_discard_cmd(dcc, next_dc);

### tdc is always NULL, there is no path to this code where tdc is ever 
set to a non-NULL value.

                         if (tdc)
                                 __remove_discard_cmd(sbi, tdc);
                         goto next;
                 }

                 __insert_discard_cmd(sbi, bdev, di.lstart, di.start, 
di.len);
  next:
                 prev_dc = next_dc;
                 if (!prev_dc)
                         break;

                 node = rb_next(&prev_dc->rb_node);
                 next_dc = rb_entry_safe(node, struct discard_cmd, rb_node);
         }