possible bug in devstat_selectdevs()
Andriy Gapon
avg at FreeBSD.org
Wed Nov 13 14:30:25 UTC 2019
I wonder if anyone remembers devstat code enough to help me or, at least, to
sanity check my line of thinking.
I am looking at a crash that happened in devstat_selectdevs(num_selections=27,
numdevs=25). At the time of the crash there was some reconfiguration of logical
volumes on a RAID controller, so "disks" were coming and going.
The first relevant block of code in the function is:
/*
* In this case, we have selected devices before, but the device
* list has changed since we last selected devices, so we need to
* either enlarge or reduce the size of the device selection list.
*/
} else if (*num_selections != numdevs) {
*dev_select = (struct device_selection *)reallocf(*dev_select,
numdevs * sizeof(struct device_selection));
*select_generation = current_generation;
init_selections = 1;
So, dev_select array is realloc-ed to have space for numdevs elements.
Then we have this:
if (((init_selected_var != 0) || (init_selections != 0)
|| (perf_select != 0)) && (changed == 0)){
old_dev_select = (struct device_selection *)malloc(
*num_selections * sizeof(struct device_selection));
if (old_dev_select == NULL) {
snprintf(devstat_errbuf, sizeof(devstat_errbuf),
"%s: Cannot allocate memory for selection list
backup",
__func__);
return(-1);
}
old_num_selections = *num_selections;
==> bcopy(*dev_select, old_dev_select,
sizeof(struct device_selection) * *num_selections);
}
The crash happened in the bcopy() call.
So, we are trying to copy num_selections (I omit pointer dereferencing) elements
from the dev_select array. But in the previous block we resized the array to
numdevs and in this case numdevs is less than num_selections.
The code is quite unfamiliar to me. My first instinct is to just clamp the copy
size, but I am not sure if that would be the right thing.
Maybe realloc of dev_select should be done after bcopy-ing out of the array?
Or maybe it's okay to realloc only if the size is going up?
Any help is appreciated.
Thank you very much in advance!
--
Andriy Gapon
More information about the freebsd-current
mailing list