search

RT-Thread / rt-thread

RT-Thread is an open source IoT Real-Time Operating System (RTOS).
https://www.rt-thread.io
Apache License 2.0
10.51k stars 5.03k forks source link

[kernel]临界区保护不全可能引发的一些非常“寸”的bug #4753

Closed mysterywolf closed 3 years ago

mysterywolf commented 3 years ago

image 红箭头所指的关调度保护的函数是我后加的

以此截图为例，在rt_thread_idle_entry函数中，调用回调函数时先判断回调函数是否为空（if (idle_hook_list[i] != RT_NULL)），然后执行这个回调函数。但是如果非常的赶巧，在判断之后，用户在其他线程中把回调撤销掉了（即改成了NULL），此时，如果继续执行这个回调函数会引发灾难性后果，因为直接访问了一个非法的地址NULL。 虽然这个问题，非常非常的凑巧，800年遇不到一会，但是一旦遇到就是直接死机。尤其是RT-Thread设备框架上面用了大量的这种回调函数（ops）。

因此在回调函数里边，这个问题需要引起重视，该罩保护的必须扣上临界区保护。所以对于上图，建议采用关掉度保护还是关中断保护？

Guozhanxin commented 3 years ago

我感觉关调度就可以了。idle里本来就不可以做挂起自己的操作，和关调度之后的限制差不多，并不会引入更多的限制：

https://gitee.com/rtthread/docs-online/pulls/107/files

mysterywolf commented 3 years ago

嗯 我觉得也是 用关中断保护 可能会导致关中断时间不可控，限制的太死了

majianjia commented 3 years ago

这个会影响现有的一些例子，比如那个cpuusage类型的应用。导致无法统计CPU 占用时间。 有些钩子是可以被打断的，这样直接套上critical 不太合适

还需要考虑像Power management 组件之类的应用，需要操作中断的。

Guozhanxin commented 3 years ago

这个会影响现有的一些例子，比如那个cpuusage类型的应用。导致无法统计CPU 占用时间。 有些钩子是可以被打断的，这样直接套上critical 不太合适

是的，那应该怎么保护这段临界区呢。。。

majianjia commented 3 years ago

类似这种的操作基本上很多地方都会有。

if(run != NULL)
    run();

所有带callback 带接口的框架都逃不开这样的判断。解决方法只有从应用方面去避开。 比如，单次初始化后不再修改钩子函数。 或者提供框架本身的deinit接口，用比较规范的顺序去删除这个接口，删除时保证不会运行到相关判断的代码。

mysterywolf commented 3 years ago

callback里边不能调用阻塞函数 如果不调用阻塞函数的话 是不会有问题的。 计算cpu使用率调用idel hook干啥了啊 按照道理来讲 就是计数啊，一顿加

majianjia commented 3 years ago

这样毕竟是限制了idle hook的一些要求，之前是可以做一些非常耗时的应用的，比如原来就算是把图形给idle刷也是可以的。 那个cpu的例子是一开始一顿加，记一个不被打断情况下的最大值，然后正常跑的时候允许被打断所以会小于最大值。

mysterywolf commented 3 years ago

那个cpu的例子是一开始一顿加，记一个不被打断情况下的最大值，然后正常跑的时候允许被打断所以会小于最大值。

这个计算CPU使用率的思想和ucos是一样的，关调度不影响

之前是可以做一些非常耗时的应用的，比如原来就算是把图形给idle刷也是可以的

我认为这个是违规的，hook本身就只能执行一些快速短暂的操作，不适合执行冗长的程序，无论是uCOS还是RTT都是这样要求的。这个应该考虑放在软件定时器任务或者单独创建一个线程中

mysterywolf commented 3 years ago

要么就是采用mutex，用mutex保护idle hook有点太杀鸡用牛刀了 不对 不能用mutex，idle线程不允许被主动挂起，只能被别的线程抢占出去，保护要么关调度，要么关中断，只能用这两种方法的其中一种。 所以关调度保护没有问题，加关调度正好防止hook内用户调用有可能被调用主动挂起的函数，比如delay之类的。

mysterywolf commented 3 years ago

我发现uCOS代码也没有任何保护，不过人家不保护是有原因的，他的回调函数是在程序里定死的，不可以动态更改。所以不会遇到我说的这种我问题。 https://github.com/weston-embedded/uC-OS3/blob/f024163107842d37a919bade8d0249c4acf64b4c/Ports/Renesas/V850E2M/CubeSuite%2B/os_cpu_c.c#L74-80

majianjia commented 3 years ago

问题在这类用法到处都是，人为不去改应该是约定俗成的。 因为你这个接口是用API 来初始化的，不应该由用户直接去操作结构体/数组来删掉，而应该用相关的API接口来删除。 我看了一下，这个数组也是用static修饰的，用户只能用接口操作。

如果要避免这里的野指针问题，应该从相关的API接口考虑。 比如应该考虑怎么去优化这个函数避免这个问题https://github.com/RT-Thread/rt-thread/blob/aa13a78b3ffa30b1bdcfcc734a2be6874b2e53c6/src/idle.c#L104 这个删除接口，现在还没有考虑这个问题。确实是有可能跑飞的可能。可以加一个flag，看看idle本身是不是正在验证钩子。验证的话晚点删或者等退出后删。

像cpuusage这种情况，它的采样时间内，必须要允许调度不然没办法采样。而且采样时间需要足够长才能有效。默认好像是10ms，10ms其实效果并不好还应该更大。 对于钩子应该尽量少耗时，这种要求在其他地方是对的，但是这个idle线程毕竟比较特殊，就像你说不能用mutex，它并不是一个正常的线程。 还有一种情况，是在以前RTT没有PM组件的情况时，或者现在用户自定义PM休眠时，休眠钩子都是放idle hook里面的。如果中断关了，WFI之类的可能会醒不来。

RTT的软件timer是不应该做耗时，因为两点，一是优先级特别高，默认是4。二是timer是一个线程遍历所有的回调，回调之间会相互影响。

mysterywolf commented 3 years ago

好家伙，FIFO问题还没解决，我又引出了另一个问题。😂

mysterywolf commented 3 years ago

idle线程不能被调度出去，idle线程只能被其他线程所抢占。idle是兜底线程，如果连idle都被调度出去了，那么不排除在某一时刻，cpu将没有线程可执行。

image

道理是一样的

mysterywolf commented 3 years ago

image RT-Thread 手册中也明确提到了