最近在公司项目中遇到一个问题,觉得挺有意思的,但涉及的知识点又十分基础,记录一下。
首先问题是出现了一个kernel panic,(可以看出是个64位的系统)
Unable to handle kernel paging request at virtual address ffffffd854850118
代码在运行期间非法访问了一个奇怪的地址,一般而言内核Panic出现非法访问时,大概率会是一个NULL地址或者NULL地址加上一个偏移量,出现这种特别大的,一般怀疑是“踩内存”了。
那么根据Call Trace我们定位到了出错的代码段(由于公司安全问题,代码做了简化)
bool detect(
u32 x,
u32 y
)
{
...
float R = patchColor[y][x].R * Gains[0]) / 1024.f;
...
}
非法访问发生在 float R = patchColor[y][x].R * Gains[0]) / 1024.f; 这一行。
经过分析,不难发现可能是x或者y的值异常导致了数组越界。
进一步去确认x,y的来源以及patchColor数组的定义
patch_struct patchColor[64][64];
u32 Start1;
u32 End1;
u32 Start2;
u32 End2;
...
for (u32 col = Start1; col <= End1; col++)
{
...
for (u32 row = Start2; row >= End2; row--)
{
...
bool isdetect = detect(col, row);
...
}
}
可以看到patchColor[64][64]是一个64x64的二维数组;x,y来源于上一层的for循环,那么机智的读者肯定不会和我一样看了半天才发现端倪。
如果第二层循环的End2值为0,那么问题就发生了,计数器row为unsigned int类型,那么当它自减到0,再减1,它的值为0xffffffff(正数),那么循环仍然进行,0xffffffff传入了detect函数,非法访问发生了,patchColor[x][0xffffffff]。
问题找到了,灵机一动的我(并不是)立马改出了第一版patch:把row改成signed int类型,这样自减的时候就不会变成正数0xffffffff,而是-1,为了验证可靠性,“聪明”的作者还把End2写成0,验证了这个循环确实到0就会停止。但是!!这个Panic问题仍然存在,请问是为什么呢?
u32 Start1;
u32 End1;
u32 Start2;
u32 End2;
...
for (i32 col = Start1; col <= End1; col++)
{
...
for (i32 row = Start2; row >= End2; row--)
{
...
bool isdetect = detect(col, row);
...
}
}
虽然我把row改成了signed int后,不再会出现0xffffffff的正值,但是由于End2是unsigned int型,在for循环的判断语句中row (i32)>= End2(u32)时会有类似隐式转换的行为,将signed int类型的row变量转换成unsigned int再与End2进行比较,因此Patch1无法按预期规避问题。
这里坑人的是,前一步我代入0验证Patch(row >= 0)是不会发生隐式转换的。
至于最终的Patch,有小伙伴建议说把第二个for循环也改成从小到大的循环,这样就能规避到0时的尴尬境地。但是,原代码应该是考虑到从大到小检索能够更快的检索到想要的数才这么选择的。
因此最后采用了如下的修改。
u32 Start1;
u32 End1;
u32 Start2;
u32 End2;
...
for (u32 col = Start1; col <= End1; col++)
{
...
for (u32 row = Start2; row >= End2; row--)
{
...
bool isdetect = detect(col, row);
...
if(col == 0)
break;
}
}
最后还是感叹,不论写多复杂的代码,代码规范性和扎实的基础永远都是不可忽略的。