[kbuild-all] Re: [dlech-linux:bone-counter 3/12] drivers/counter/counter-chrdev.c:189 counter_set_watch() warn: potential spectre issue 'counter->signals' (local cap)

tree: https://github.com/dlech/linux bone-counter head: f8f0ad10f30b5c1bea1a22f5de5dc9f2c6b7db1a commit: c4408f814b1a257c3854ec272e8348ef8fe55f48 [3/12] counter: Add character device interface config: x86_64-randconfig-m001-20201014 (attached as .config) compiler: gcc-9 (Debian 9.3.0-15) 9.3.0 If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot <lkp(a)intel.com&gt; Reported-by: Dan Carpenter <dan.carpenter(a)oracle.com&gt;

smatch warnings: drivers/counter/counter-chrdev.c:189 counter_set_watch() warn: potential spectre issue 'counter->signals' [r] (local cap) drivers/counter/counter-chrdev.c:198 counter_set_watch() warn: potential spectre issue 'counter->counts' [r] (local cap) drivers/counter/counter-chrdev.c:242 counter_set_watch() warn: potential spectre issue 'ext' [r] (local cap) drivers/counter/counter-chrdev.c:247 counter_set_watch() warn: possible spectre second half. 'comp_node.comp' vim +189 drivers/counter/counter-chrdev.c c4408f814b1a257 William Breathitt Gray 2020-09-26 160 static int counter_set_watch(struct counter_device *const counter, c4408f814b1a257 William Breathitt Gray 2020-09-26 161 const unsigned long arg) c4408f814b1a257 William Breathitt Gray 2020-09-26 162 { c4408f814b1a257 William Breathitt Gray 2020-09-26 163 void __user *const uwatch = (void __user *)arg; c4408f814b1a257 William Breathitt Gray 2020-09-26 164 struct counter_watch watch; c4408f814b1a257 William Breathitt Gray 2020-09-26 165 struct counter_comp_node comp_node; c4408f814b1a257 William Breathitt Gray 2020-09-26 166 size_t parent, id; c4408f814b1a257 William Breathitt Gray 2020-09-26 167 struct counter_comp *ext; c4408f814b1a257 William Breathitt Gray 2020-09-26 168 size_t num_ext; c4408f814b1a257 William Breathitt Gray 2020-09-26 169 c4408f814b1a257 William Breathitt Gray 2020-09-26 170 if (copy_from_user(&watch, uwatch, sizeof(watch))) c4408f814b1a257 William Breathitt Gray 2020-09-26 171 return -EFAULT; c4408f814b1a257 William Breathitt Gray 2020-09-26 172 parent = watch.component.parent; c4408f814b1a257 William Breathitt Gray 2020-09-26 173 id = watch.component.id; c4408f814b1a257 William Breathitt Gray 2020-09-26 174 c4408f814b1a257 William Breathitt Gray 2020-09-26 175 /* Configure parent component info for comp node */ c4408f814b1a257 William Breathitt Gray 2020-09-26 176 switch (watch.component.scope) { c4408f814b1a257 William Breathitt Gray 2020-09-26 177 case COUNTER_SCOPE_DEVICE: c4408f814b1a257 William Breathitt Gray 2020-09-26 178 comp_node.parent = NULL; c4408f814b1a257 William Breathitt Gray 2020-09-26 179 c4408f814b1a257 William Breathitt Gray 2020-09-26 180 ext = counter->ext; c4408f814b1a257 William Breathitt Gray 2020-09-26 181 num_ext = counter->num_ext; c4408f814b1a257 William Breathitt Gray 2020-09-26 182 break; c4408f814b1a257 William Breathitt Gray 2020-09-26 183 case COUNTER_SCOPE_SIGNAL: c4408f814b1a257 William Breathitt Gray 2020-09-26 184 if (counter->num_signals < parent + 1) Smatch looks at these and complains about Spectre issues. I'm not an expert at Spectre but that's probably a valid complaint and this code should be using array_index_nospec(). This code has some other bugs as well. 1) "watch.component.parent" is a u64 but "parent" is a size_t so that's a potential problem on 32 bit systems. 2) If "parent is ULONG_MAX then "parent + 1" will have an integer overflow to zero leading to an out of bounds read. The normal way to write these conditions is with the varable on the left and the limit on the right. Use >= to avoid the integer overflow. So to solve all three bugs do: u64 parent, id; ... if (parent >= counter->num_signals) return -EINVAL; parent = array_index_nospec(parent, counter->num_signals); c4408f814b1a257 William Breathitt Gray 2020-09-26 185 return -EINVAL; c4408f814b1a257 William Breathitt Gray 2020-09-26 186 c4408f814b1a257 William Breathitt Gray 2020-09-26 187 comp_node.parent = counter->signals + parent; c4408f814b1a257 William Breathitt Gray 2020-09-26 188 c4408f814b1a257 William Breathitt Gray 2020-09-26 @189 ext = counter->signals[parent].ext; c4408f814b1a257 William Breathitt Gray 2020-09-26 190 num_ext = counter->signals[parent].num_ext; c4408f814b1a257 William Breathitt Gray 2020-09-26 191 break; c4408f814b1a257 William Breathitt Gray 2020-09-26 192 case COUNTER_SCOPE_COUNT: c4408f814b1a257 William Breathitt Gray 2020-09-26 193 if (counter->num_counts < parent + 1) c4408f814b1a257 William Breathitt Gray 2020-09-26 194 return -EINVAL; c4408f814b1a257 William Breathitt Gray 2020-09-26 195 c4408f814b1a257 William Breathitt Gray 2020-09-26 196 comp_node.parent = counter->counts + parent; c4408f814b1a257 William Breathitt Gray 2020-09-26 197 c4408f814b1a257 William Breathitt Gray 2020-09-26 @198 ext = counter->counts[parent].ext; c4408f814b1a257 William Breathitt Gray 2020-09-26 199 num_ext = counter->counts[parent].num_ext; c4408f814b1a257 William Breathitt Gray 2020-09-26 200 break; c4408f814b1a257 William Breathitt Gray 2020-09-26 201 default: c4408f814b1a257 William Breathitt Gray 2020-09-26 202 return -EINVAL; c4408f814b1a257 William Breathitt Gray 2020-09-26 203 } c4408f814b1a257 William Breathitt Gray 2020-09-26 204 c4408f814b1a257 William Breathitt Gray 2020-09-26 205 /* Configure component info for comp node */ c4408f814b1a257 William Breathitt Gray 2020-09-26 206 switch (watch.component.type) { c4408f814b1a257 William Breathitt Gray 2020-09-26 207 case COUNTER_COMPONENT_SIGNAL: c4408f814b1a257 William Breathitt Gray 2020-09-26 208 if (watch.component.scope != COUNTER_SCOPE_SIGNAL) c4408f814b1a257 William Breathitt Gray 2020-09-26 209 return -EINVAL; c4408f814b1a257 William Breathitt Gray 2020-09-26 210 c4408f814b1a257 William Breathitt Gray 2020-09-26 211 comp_node.comp.type = COUNTER_COMP_SIGNAL_LEVEL; c4408f814b1a257 William Breathitt Gray 2020-09-26 212 comp_node.comp.signal_u8_read = counter->ops->signal_read; c4408f814b1a257 William Breathitt Gray 2020-09-26 213 break; c4408f814b1a257 William Breathitt Gray 2020-09-26 214 case COUNTER_COMPONENT_COUNT: c4408f814b1a257 William Breathitt Gray 2020-09-26 215 if (watch.component.scope != COUNTER_SCOPE_COUNT) c4408f814b1a257 William Breathitt Gray 2020-09-26 216 return -EINVAL; c4408f814b1a257 William Breathitt Gray 2020-09-26 217 c4408f814b1a257 William Breathitt Gray 2020-09-26 218 comp_node.comp.type = COUNTER_COMP_U64; c4408f814b1a257 William Breathitt Gray 2020-09-26 219 comp_node.comp.count_u64_read = counter->ops->count_read; c4408f814b1a257 William Breathitt Gray 2020-09-26 220 break; c4408f814b1a257 William Breathitt Gray 2020-09-26 221 case COUNTER_COMPONENT_FUNCTION: c4408f814b1a257 William Breathitt Gray 2020-09-26 222 if (watch.component.scope != COUNTER_SCOPE_COUNT) c4408f814b1a257 William Breathitt Gray 2020-09-26 223 return -EINVAL; c4408f814b1a257 William Breathitt Gray 2020-09-26 224 c4408f814b1a257 William Breathitt Gray 2020-09-26 225 comp_node.comp.type = COUNTER_COMP_FUNCTION; c4408f814b1a257 William Breathitt Gray 2020-09-26 226 comp_node.comp.count_u8_read = counter->ops->function_read; c4408f814b1a257 William Breathitt Gray 2020-09-26 227 break; c4408f814b1a257 William Breathitt Gray 2020-09-26 228 case COUNTER_COMPONENT_SYNAPSE_ACTION: c4408f814b1a257 William Breathitt Gray 2020-09-26 229 if (watch.component.scope != COUNTER_SCOPE_COUNT) c4408f814b1a257 William Breathitt Gray 2020-09-26 230 return -EINVAL; c4408f814b1a257 William Breathitt Gray 2020-09-26 231 if (counter->counts[parent].num_synapses < id + 1) c4408f814b1a257 William Breathitt Gray 2020-09-26 232 return -EINVAL; c4408f814b1a257 William Breathitt Gray 2020-09-26 233 c4408f814b1a257 William Breathitt Gray 2020-09-26 234 comp_node.comp.type = COUNTER_COMP_SYNAPSE_ACTION; c4408f814b1a257 William Breathitt Gray 2020-09-26 235 comp_node.comp.action_read = counter->ops->action_read; c4408f814b1a257 William Breathitt Gray 2020-09-26 236 comp_node.comp.priv = counter->counts[parent].synapses + id; c4408f814b1a257 William Breathitt Gray 2020-09-26 237 break; c4408f814b1a257 William Breathitt Gray 2020-09-26 238 case COUNTER_COMPONENT_EXTENSION: c4408f814b1a257 William Breathitt Gray 2020-09-26 239 if (num_ext < id + 1) c4408f814b1a257 William Breathitt Gray 2020-09-26 240 return -EINVAL; c4408f814b1a257 William Breathitt Gray 2020-09-26 241 c4408f814b1a257 William Breathitt Gray 2020-09-26 @242 comp_node.comp = ext[id]; c4408f814b1a257 William Breathitt Gray 2020-09-26 243 break; c4408f814b1a257 William Breathitt Gray 2020-09-26 244 default: c4408f814b1a257 William Breathitt Gray 2020-09-26 245 return -EINVAL; c4408f814b1a257 William Breathitt Gray 2020-09-26 246 } c4408f814b1a257 William Breathitt Gray 2020-09-26 @247 if (!comp_node.comp.count_u8_read) c4408f814b1a257 William Breathitt Gray 2020-09-26 248 return -EFAULT; c4408f814b1a257 William Breathitt Gray 2020-09-26 249 comp_node.component = watch.component; c4408f814b1a257 William Breathitt Gray 2020-09-26 250 c4408f814b1a257 William Breathitt Gray 2020-09-26 251 return counter_set_event_node(counter, &watch, &comp_node); c4408f814b1a257 William Breathitt Gray 2020-09-26 252 } --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org