* [bcache:nvdimm-meta 11/12] drivers/md/bcache/journal.c:114 journal_read_bucket() error: potentially dereferencing uninitialized 'j'.
@ 2021-08-05 11:18 Dan Carpenter
2021-08-05 16:30 ` Coly Li
0 siblings, 1 reply; 2+ messages in thread
From: Dan Carpenter @ 2021-08-05 11:18 UTC (permalink / raw)
To: kbuild, Coly Li; +Cc: lkp, kbuild-all, linux-kernel
tree: https://git.kernel.org/pub/scm/linux/kernel/git/colyli/linux-bcache.git nvdimm-meta
head: a12f8ec824edd1317f14882c7d0aee5e5c941edd
commit: 5f408d113974d2bb3eb1b237d549724f7509ab23 [11/12] bcache: read jset from NVDIMM pages for journal replay
config: x86_64-randconfig-m001-20210804 (attached as .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:
drivers/md/bcache/journal.c:114 journal_read_bucket() error: potentially dereferencing uninitialized 'j'.
vim +/j +114 drivers/md/bcache/journal.c
cafe563591446c Kent Overstreet 2013-03-23 106
cafe563591446c Kent Overstreet 2013-03-23 107 /* This function could be simpler now since we no longer write
cafe563591446c Kent Overstreet 2013-03-23 108 * journal entries that overlap bucket boundaries; this means
cafe563591446c Kent Overstreet 2013-03-23 109 * the start of a bucket will always have a valid journal entry
cafe563591446c Kent Overstreet 2013-03-23 110 * if it has any journal entries at all.
cafe563591446c Kent Overstreet 2013-03-23 111 */
On my kernel there is a "j = data;" line here, but I guess it got
removed so that's why Smatch is complaining?
cafe563591446c Kent Overstreet 2013-03-23 112 while (len) {
cafe563591446c Kent Overstreet 2013-03-23 113 struct list_head *where;
cafe563591446c Kent Overstreet 2013-03-23 @114 size_t blocks, bytes = set_bytes(j);
^
cafe563591446c Kent Overstreet 2013-03-23 115
b3fa7e77e67e64 Kent Overstreet 2013-08-05 116 if (j->magic != jset_magic(&ca->sb)) {
46f5aa8806e34f Joe Perches 2020-05-27 117 pr_debug("%u: bad magic\n", bucket_index);
cafe563591446c Kent Overstreet 2013-03-23 118 return ret;
b3fa7e77e67e64 Kent Overstreet 2013-08-05 119 }
cafe563591446c Kent Overstreet 2013-03-23 120
b3fa7e77e67e64 Kent Overstreet 2013-08-05 121 if (bytes > left << 9 ||
b3fa7e77e67e64 Kent Overstreet 2013-08-05 122 bytes > PAGE_SIZE << JSET_BITS) {
46f5aa8806e34f Joe Perches 2020-05-27 123 pr_info("%u: too big, %zu bytes, offset %u\n",
b3fa7e77e67e64 Kent Overstreet 2013-08-05 124 bucket_index, bytes, offset);
cafe563591446c Kent Overstreet 2013-03-23 125 return ret;
b3fa7e77e67e64 Kent Overstreet 2013-08-05 126 }
cafe563591446c Kent Overstreet 2013-03-23 127
cafe563591446c Kent Overstreet 2013-03-23 128 if (bytes > len << 9)
cafe563591446c Kent Overstreet 2013-03-23 129 goto reread;
cafe563591446c Kent Overstreet 2013-03-23 130
b3fa7e77e67e64 Kent Overstreet 2013-08-05 131 if (j->csum != csum_set(j)) {
46f5aa8806e34f Joe Perches 2020-05-27 132 pr_info("%u: bad csum, %zu bytes, offset %u\n",
b3fa7e77e67e64 Kent Overstreet 2013-08-05 133 bucket_index, bytes, offset);
cafe563591446c Kent Overstreet 2013-03-23 134 return ret;
b3fa7e77e67e64 Kent Overstreet 2013-08-05 135 }
cafe563591446c Kent Overstreet 2013-03-23 136
4e1ebae3ee4e0c Coly Li 2020-10-01 137 blocks = set_blocks(j, block_bytes(ca));
cafe563591446c Kent Overstreet 2013-03-23 138
2464b693148e5d Coly Li 2019-06-28 139 /*
2464b693148e5d Coly Li 2019-06-28 140 * Nodes in 'list' are in linear increasing order of
2464b693148e5d Coly Li 2019-06-28 141 * i->j.seq, the node on head has the smallest (oldest)
2464b693148e5d Coly Li 2019-06-28 142 * journal seq, the node on tail has the biggest
2464b693148e5d Coly Li 2019-06-28 143 * (latest) journal seq.
2464b693148e5d Coly Li 2019-06-28 144 */
2464b693148e5d Coly Li 2019-06-28 145
2464b693148e5d Coly Li 2019-06-28 146 /*
2464b693148e5d Coly Li 2019-06-28 147 * Check from the oldest jset for last_seq. If
2464b693148e5d Coly Li 2019-06-28 148 * i->j.seq < j->last_seq, it means the oldest jset
2464b693148e5d Coly Li 2019-06-28 149 * in list is expired and useless, remove it from
9c9b81c45619e7 Bhaskar Chowdhury 2021-04-11 150 * this list. Otherwise, j is a candidate jset for
2464b693148e5d Coly Li 2019-06-28 151 * further following checks.
2464b693148e5d Coly Li 2019-06-28 152 */
cafe563591446c Kent Overstreet 2013-03-23 153 while (!list_empty(list)) {
cafe563591446c Kent Overstreet 2013-03-23 154 i = list_first_entry(list,
cafe563591446c Kent Overstreet 2013-03-23 155 struct journal_replay, list);
cafe563591446c Kent Overstreet 2013-03-23 156 if (i->j.seq >= j->last_seq)
cafe563591446c Kent Overstreet 2013-03-23 157 break;
cafe563591446c Kent Overstreet 2013-03-23 158 list_del(&i->list);
cafe563591446c Kent Overstreet 2013-03-23 159 kfree(i);
cafe563591446c Kent Overstreet 2013-03-23 160 }
cafe563591446c Kent Overstreet 2013-03-23 161
2464b693148e5d Coly Li 2019-06-28 162 /* iterate list in reverse order (from latest jset) */
cafe563591446c Kent Overstreet 2013-03-23 163 list_for_each_entry_reverse(i, list, list) {
cafe563591446c Kent Overstreet 2013-03-23 164 if (j->seq == i->j.seq)
cafe563591446c Kent Overstreet 2013-03-23 165 goto next_set;
cafe563591446c Kent Overstreet 2013-03-23 166
2464b693148e5d Coly Li 2019-06-28 167 /*
2464b693148e5d Coly Li 2019-06-28 168 * if j->seq is less than any i->j.last_seq
2464b693148e5d Coly Li 2019-06-28 169 * in list, j is an expired and useless jset.
2464b693148e5d Coly Li 2019-06-28 170 */
cafe563591446c Kent Overstreet 2013-03-23 171 if (j->seq < i->j.last_seq)
cafe563591446c Kent Overstreet 2013-03-23 172 goto next_set;
cafe563591446c Kent Overstreet 2013-03-23 173
2464b693148e5d Coly Li 2019-06-28 174 /*
2464b693148e5d Coly Li 2019-06-28 175 * 'where' points to first jset in list which
2464b693148e5d Coly Li 2019-06-28 176 * is elder then j.
2464b693148e5d Coly Li 2019-06-28 177 */
cafe563591446c Kent Overstreet 2013-03-23 178 if (j->seq > i->j.seq) {
cafe563591446c Kent Overstreet 2013-03-23 179 where = &i->list;
cafe563591446c Kent Overstreet 2013-03-23 180 goto add;
cafe563591446c Kent Overstreet 2013-03-23 181 }
cafe563591446c Kent Overstreet 2013-03-23 182 }
cafe563591446c Kent Overstreet 2013-03-23 183
cafe563591446c Kent Overstreet 2013-03-23 184 where = list;
cafe563591446c Kent Overstreet 2013-03-23 185 add:
cafe563591446c Kent Overstreet 2013-03-23 186 i = kmalloc(offsetof(struct journal_replay, j) +
cafe563591446c Kent Overstreet 2013-03-23 187 bytes, GFP_KERNEL);
cafe563591446c Kent Overstreet 2013-03-23 188 if (!i)
cafe563591446c Kent Overstreet 2013-03-23 189 return -ENOMEM;
cafe563591446c Kent Overstreet 2013-03-23 190 memcpy(&i->j, j, bytes);
2464b693148e5d Coly Li 2019-06-28 191 /* Add to the location after 'where' points to */
cafe563591446c Kent Overstreet 2013-03-23 192 list_add(&i->list, where);
cafe563591446c Kent Overstreet 2013-03-23 193 ret = 1;
cafe563591446c Kent Overstreet 2013-03-23 194
a231f07a5fe30a Coly Li 2019-06-28 195 if (j->seq > ja->seq[bucket_index])
cafe563591446c Kent Overstreet 2013-03-23 196 ja->seq[bucket_index] = j->seq;
cafe563591446c Kent Overstreet 2013-03-23 197 next_set:
cafe563591446c Kent Overstreet 2013-03-23 198 offset += blocks * ca->sb.block_size;
cafe563591446c Kent Overstreet 2013-03-23 199 len -= blocks * ca->sb.block_size;
cafe563591446c Kent Overstreet 2013-03-23 200 j = ((void *) j) + blocks * block_bytes(ca);
cafe563591446c Kent Overstreet 2013-03-23 201 }
cafe563591446c Kent Overstreet 2013-03-23 202 }
cafe563591446c Kent Overstreet 2013-03-23 203
cafe563591446c Kent Overstreet 2013-03-23 204 return ret;
cafe563591446c Kent Overstreet 2013-03-23 205 }
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: [bcache:nvdimm-meta 11/12] drivers/md/bcache/journal.c:114 journal_read_bucket() error: potentially dereferencing uninitialized 'j'.
2021-08-05 11:18 [bcache:nvdimm-meta 11/12] drivers/md/bcache/journal.c:114 journal_read_bucket() error: potentially dereferencing uninitialized 'j' Dan Carpenter
@ 2021-08-05 16:30 ` Coly Li
0 siblings, 0 replies; 2+ messages in thread
From: Coly Li @ 2021-08-05 16:30 UTC (permalink / raw)
To: Dan Carpenter; +Cc: lkp, kbuild-all, linux-kernel, kbuild
On 8/5/21 7:18 PM, Dan Carpenter wrote:
> tree: https://git.kernel.org/pub/scm/linux/kernel/git/colyli/linux-bcache.git nvdimm-meta
> head: a12f8ec824edd1317f14882c7d0aee5e5c941edd
> commit: 5f408d113974d2bb3eb1b237d549724f7509ab23 [11/12] bcache: read jset from NVDIMM pages for journal replay
> config: x86_64-randconfig-m001-20210804 (attached as .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:
> drivers/md/bcache/journal.c:114 journal_read_bucket() error: potentially dereferencing uninitialized 'j'.
>
> vim +/j +114 drivers/md/bcache/journal.c
>
> cafe563591446c Kent Overstreet 2013-03-23 106
> cafe563591446c Kent Overstreet 2013-03-23 107 /* This function could be simpler now since we no longer write
> cafe563591446c Kent Overstreet 2013-03-23 108 * journal entries that overlap bucket boundaries; this means
> cafe563591446c Kent Overstreet 2013-03-23 109 * the start of a bucket will always have a valid journal entry
> cafe563591446c Kent Overstreet 2013-03-23 110 * if it has any journal entries at all.
> cafe563591446c Kent Overstreet 2013-03-23 111 */
>
> On my kernel there is a "j = data;" line here, but I guess it got
> removed so that's why Smatch is complaining?
Removing "j = data" is on purpose, the jset *j is initialized by the
previous code block which I list here,
96 while (offset < ca->sb.bucket_size) {
97 reread: left = ca->sb.bucket_size - offset;
98 len = min_t(unsigned int, left, PAGE_SECTORS <<
JSET_BITS);
99
100 if (!bch_has_feature_nvdimm_meta(&ca->sb))
101 j = __jnl_rd_bkt(ca, bucket_index, len,
offset, &cl);
102 #if defined(CONFIG_BCACHE_NVM_PAGES)
103 else
104 j = __jnl_rd_nvm_bkt(ca, bucket_index, len,
offset);
105 #endif
106
107 /* This function could be simpler now since we no
longer write
108 * journal entries that overlap bucket boundaries;
this means
109 * the start of a bucket will always have a valid
journal entry
110 * if it has any journal entries at all.
111 */
112 while (len) {
jset *j is initialized at line 101 for non CONFIG_BCACHE_NVM_PAGES
condition, and at line 104 for configed CONFIG_BCACHE_NVM_PAGES condition.
The warning might be from a missing condition check for "else if
(bch_has_feature_nvdimm_meta(&ca->sb))" in code block line 100 to line
105. The static checking tool may think for such condition branch, jset
*j is undefined and referenced by the following code. But if
CONFIG_BCACHE_NVM_PAGES is not configured, such condition branch won't
happen, the supported feature set checking will make sure if the cache
device is formatted to use nvdimm but the kernel does not support yet,
the kernel will report unsupported feature and fail the registration.
Your remind is informative and helpful, before reconstruct the source
code files to handle the config condition more clean, I will add code
comments at line 102 to explain how the undefined jset *j won't happen.
Thanks.
Coly Li
> ---
> 0-DAY CI Kernel Test Service, Intel Corporation
> https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
>
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2021-08-05 16:31 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-05 11:18 [bcache:nvdimm-meta 11/12] drivers/md/bcache/journal.c:114 journal_read_bucket() error: potentially dereferencing uninitialized 'j' Dan Carpenter
2021-08-05 16:30 ` Coly Li
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).