* [dlech-linux:bone-counter 3/12] drivers/counter/counter-chrdev.c:189 counter_set_watch() warn: potential spectre issue 'counter->signals' (local cap)
@ 2020-10-14 13:30 kernel test robot
0 siblings, 0 replies; 4+ messages in thread
From: kernel test robot @ 2020-10-14 13:30 UTC (permalink / raw)
To: kbuild
[-- Attachment #1: Type: text/plain, Size: 9256 bytes --]
CC: kbuild-all(a)lists.01.org
TO: William Breathitt Gray <vilhelm.gray@gmail.com>
CC: David Lechner <david@lechnology.com>
tree: https://github.com/dlech/linux bone-counter
head: f8f0ad10f30b5c1bea1a22f5de5dc9f2c6b7db1a
commit: c4408f814b1a257c3854ec272e8348ef8fe55f48 [3/12] counter: Add character device interface
:::::: branch date: 12 hours ago
:::::: commit date: 2 days ago
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@intel.com>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
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 159
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)
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 }
c4408f814b1a257 William Breathitt Gray 2020-09-26 253
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 28963 bytes --]
^ permalink raw reply [flat|nested] 4+ messages in thread
* [dlech-linux:bone-counter 3/12] drivers/counter/counter-chrdev.c:189 counter_set_watch() warn: potential spectre issue 'counter->signals' (local cap)
@ 2020-10-14 14:22 ` Dan Carpenter
0 siblings, 0 replies; 4+ messages in thread
From: Dan Carpenter @ 2020-10-14 14:22 UTC (permalink / raw)
To: kbuild
[-- Attachment #1: Type: text/plain, Size: 9735 bytes --]
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@intel.com>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
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@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
[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 28963 bytes --]
^ permalink raw reply [flat|nested] 4+ messages in thread
* [dlech-linux:bone-counter 3/12] drivers/counter/counter-chrdev.c:189 counter_set_watch() warn: potential spectre issue 'counter->signals' (local cap)
@ 2020-10-14 14:22 ` Dan Carpenter
0 siblings, 0 replies; 4+ messages in thread
From: Dan Carpenter @ 2020-10-14 14:22 UTC (permalink / raw)
To: kbuild-all
[-- Attachment #1: Type: text/plain, Size: 9735 bytes --]
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@intel.com>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
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@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
[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 28963 bytes --]
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [dlech-linux:bone-counter 3/12] drivers/counter/counter-chrdev.c:189 counter_set_watch() warn: potential spectre issue 'counter->signals' (local cap)
2020-10-14 14:22 ` Dan Carpenter
(?)
@ 2020-10-18 17:06 ` William Breathitt Gray
-1 siblings, 0 replies; 4+ messages in thread
From: William Breathitt Gray @ 2020-10-18 17:06 UTC (permalink / raw)
To: kbuild-all
[-- Attachment #1: Type: text/plain, Size: 10222 bytes --]
On Wed, Oct 14, 2020 at 05:22:34PM +0300, Dan Carpenter wrote:
> 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@intel.com>
> Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Thank you for pointing out these issues. I'll work to resolve them and
CC you in the next revision.
William Breathitt Gray
>
> 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(a)lists.01.org
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2020-10-18 17:06 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-10-14 14:22 [dlech-linux:bone-counter 3/12] drivers/counter/counter-chrdev.c:189 counter_set_watch() warn: potential spectre issue 'counter->signals' (local cap) Dan Carpenter
2020-10-14 14:22 ` Dan Carpenter
2020-10-18 17:06 ` William Breathitt Gray
-- strict thread matches above, loose matches on Subject: below --
2020-10-14 13:30 kernel test robot
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.