MC芯片使用MCAL发现CAN外设的Can_Lld_ReceiveOneFrame函数存在bug

yishui66

一、背景
CAN FD，DTC=15，实际数据只有8字节，此时一发报文给MCU接收，MCU就会进hardfault，如下:
3a1de83d-a2c3-4f47-b369-7ac89790870a-e32a0c85347d61e53e7c277772c6545.png

二、现状
排查为Can_Lld_ReceiveOneFrame函数中未对Payload配置的大小进行检查就直接取数据（存在数组越界风险），原函数如下：

CAN_FUNC LOCAL_INLINE void Can_Lld_ReceiveOneFrame(uint32 * Addr, Can_HwHandleType Hrh, const Can_ChannelConfigType *ChConfigPtr)
{
	Can_HwType MailBoxInfo;
	PduInfoType PduInfo;
#if (CAN_CODE_IDHIT == STD_ON)
	uint8 ChnLogicId = ChConfigPtr->CanChannelId;
	uint16 Idhit = CAN_CONST_WHOLE_OF_DBYTE_ALL_TRUE;
#endif
	Can_CallbackPtrOfReceiveType Callback;
	uint32 DlcCode;
#if (CAN_FD_USAGE == STD_ON)
	PduLengthType MailBoxDatalength = 0U;
    uint8 PduData[CAN_MAXIM_DATA_LENGTH];
#else
    uint8 PduData[8];
#endif
	Can_HwObjRegionType RegionId = Can_ConfigPtr->CanHohCfgPtr[Hrh].CanHwObjRegionId;
	const uint32 *RamPtr = Addr;
	MailBoxInfo.Hoh = Hrh;
	MailBoxInfo.ControllerId = ChConfigPtr->CanChannelId;
	MailBoxInfo.CanId = Can_Lld_GetCanFrameId(RamPtr, RegionId);
	DlcCode = (RamPtr[0] & CAN_MB_HERADER_0_DLC_MASK) >> CAN_MB_HERADER_0_DLC_SHIFT;
	PduInfo.SduLength = (PduLengthType)Can_Table_DlcToDatalength[(uint8)DlcCode];
	for(uint8 Index = 0U; Index < (uint8)PduInfo.SduLength; ++Index)
    {
		PduData[Index] = CAN8_READ((uint32)(&RamPtr[0]) + Can_Table_FrameDataAddr[Index]);
    }
    PduInfo.SduDataPtr = PduData;
#if (CAN_ENHANCE_FIFO_USAGE == STD_ON) /* Handle Enhanced fifo receive */
	if(CAN_RX_FIFO_ENHANCE == RegionId)
	{
#if (STD_ON == CAN_CODE_IDHIT)
		Idhit = (uint16)RamPtr[Can_Table_DlcToIdhitOff[DlcCode]] & CAN_ENHANCE_RXFIFO_IDHIT_MASK;
#endif
	}
    else
#endif

#if (CAN_LEGACY_FIFO_USAGE == STD_ON) /* Handle legacy fifo receive */
	if(CAN_RX_FIFO_LEGACY == RegionId)
	{
#if (STD_ON == CAN_CODE_IDHIT)
		Idhit = (uint16)((RamPtr[0] & CAN_LEGACY_RXFIFO_IDHIT_MASK) >> CAN_LEGACY_RXFIFO_IDHIT_SHIFT);
#endif
	}
	else
#endif

	{
		if((uint32)CAN_CSCODE_RX_OVERRUN == ((RamPtr[0] & CAN_MB_HERADER_0_CODE_MASK) >> CAN_MB_HERADER_0_CODE_SHIFT))
		{	/* CAN_CSCODE_RX_OVERRUN */
			/* Can module shall raise the runtime error CAN_E_DATALOST in case of "overwrite" or
			"overrun" event detection.
			Trace SWS_Can_00395 */
			Can_Bak_CallDatalost(ChConfigPtr->CanChannelId);
			if(NULL_PTR != ChConfigPtr->CanCallbackPtr->OverrunCallback)
			{
				ChConfigPtr->CanCallbackPtr->OverrunCallback(Hrh);
			}
		}
#if (CAN_FD_USAGE == STD_ON)
		if(TRUE == ChConfigPtr->FdUsage)
		{
			MailBoxDatalength = ChConfigPtr->PayloadConfigPtr-> \
						MbRegionConfig[RegionId].PayloadSize;
			if (PduInfo.SduLength > MailBoxDatalength)
			{
				PduInfo.SduLength = MailBoxDatalength;
			}
		}
#endif
		/* Read Timer register to unlock the MB */
		CAN32_READ(((uint32)Addr & CAN_BASE_ADDR_MASK) + CAN_TIMER_OFFSET32);
	}
#if (CAN_CODE_IDHIT == STD_ON)
	if(NULL_PTR != Can_LocFifoIdhitPtr[ChnLogicId])
	{
		*Can_LocFifoIdhitPtr[ChnLogicId] = Idhit;
	}
#endif
	Callback = Can_ConfigPtr->CanReceiveCallback;
    if(NULL_PTR == Callback)
	{
		CanIf_RxIndication(&MailBoxInfo, &PduInfo);
	}
	else
	{
		if(TRUE == Callback(MailBoxInfo.Hoh, MailBoxInfo.CanId, (uint8)(PduInfo.SduLength), PduInfo.SduDataPtr))
		{
			CanIf_RxIndication(&MailBoxInfo, &PduInfo);
		}
	}
#if (CAN_CODE_IDHIT == STD_ON)
	if(NULL_PTR != Can_LocFifoIdhitPtr[ChnLogicId])
	{
		*Can_LocFifoIdhitPtr[ChnLogicId] = CAN_CONST_WHOLE_OF_DBYTE_ALL_TRUE;
	}
#endif
}

83fc5ce4-e252-48a5-a1e7-c300996ad577-8257456443cc897bf700834de1f78a2.png

②现修改原函数如下：

AN_FUNC LOCAL_INLINE void Can_Lld_ReceiveOneFrame(uint32 * Addr, Can_HwHandleType Hrh, const Can_ChannelConfigType *ChConfigPtr)
{
	Can_HwType MailBoxInfo;
	PduInfoType PduInfo;
#if (CAN_CODE_IDHIT == STD_ON)
	uint8 ChnLogicId = ChConfigPtr->CanChannelId;
	uint16 Idhit = CAN_CONST_WHOLE_OF_DBYTE_ALL_TRUE;
#endif
	Can_CallbackPtrOfReceiveType Callback;
	uint32 DlcCode;
#if (CAN_FD_USAGE == STD_ON)
	PduLengthType MailBoxDatalength = 0U;
    uint8 PduData[CAN_MAXIM_DATA_LENGTH];
#else
    uint8 PduData[8];
#endif
	Can_HwObjRegionType RegionId = Can_ConfigPtr->CanHohCfgPtr[Hrh].CanHwObjRegionId;
	const uint32 *RamPtr = Addr;
	MailBoxInfo.Hoh = Hrh;
	MailBoxInfo.ControllerId = ChConfigPtr->CanChannelId;
	MailBoxInfo.CanId = Can_Lld_GetCanFrameId(RamPtr, RegionId);
	DlcCode = (RamPtr[0] & CAN_MB_HERADER_0_DLC_MASK) >> CAN_MB_HERADER_0_DLC_SHIFT;
	PduInfo.SduLength = (PduLengthType)Can_Table_DlcToDatalength[(uint8)DlcCode];
#if (CAN_FD_USAGE == STD_ON)
	if(TRUE == ChConfigPtr->FdUsage)
	{
			MailBoxDatalength = ChConfigPtr->PayloadConfigPtr->MbRegionConfig[RegionId].PayloadSize;

			// 限制读取长度为Payload和PduData数组长度的最小值
			if (PduInfo.SduLength > MailBoxDatalength)
			{
					PduInfo.SduLength = MailBoxDatalength;
			}
			if (PduInfo.SduLength > sizeof(PduData))
			{
					PduInfo.SduLength = sizeof(PduData);  // 防止数组越界
			}
	}
#endif
	for(uint8 Index = 0U; Index < (uint8)PduInfo.SduLength; ++Index)
    {
		PduData[Index] = CAN8_READ((uint32)(&RamPtr[0]) + Can_Table_FrameDataAddr[Index]);
    }
    PduInfo.SduDataPtr = PduData;
#if (CAN_ENHANCE_FIFO_USAGE == STD_ON) /* Handle Enhanced fifo receive */
	if(CAN_RX_FIFO_ENHANCE == RegionId)
	{
#if (STD_ON == CAN_CODE_IDHIT)
		Idhit = (uint16)RamPtr[Can_Table_DlcToIdhitOff[DlcCode]] & CAN_ENHANCE_RXFIFO_IDHIT_MASK;
#endif
	}
    else
#endif

#if (CAN_LEGACY_FIFO_USAGE == STD_ON) /* Handle legacy fifo receive */
	if(CAN_RX_FIFO_LEGACY == RegionId)
	{
#if (STD_ON == CAN_CODE_IDHIT)
		Idhit = (uint16)((RamPtr[0] & CAN_LEGACY_RXFIFO_IDHIT_MASK) >> CAN_LEGACY_RXFIFO_IDHIT_SHIFT);
#endif
	}
	else
#endif

	{
		if((uint32)CAN_CSCODE_RX_OVERRUN == ((RamPtr[0] & CAN_MB_HERADER_0_CODE_MASK) >> CAN_MB_HERADER_0_CODE_SHIFT))
		{	/* CAN_CSCODE_RX_OVERRUN */
			/* Can module shall raise the runtime error CAN_E_DATALOST in case of "overwrite" or
			"overrun" event detection.
			Trace SWS_Can_00395 */
			Can_Bak_CallDatalost(ChConfigPtr->CanChannelId);
			if(NULL_PTR != ChConfigPtr->CanCallbackPtr->OverrunCallback)
			{
				ChConfigPtr->CanCallbackPtr->OverrunCallback(Hrh);
			}
		}
		/* Read Timer register to unlock the MB */
		CAN32_READ(((uint32)Addr & CAN_BASE_ADDR_MASK) + CAN_TIMER_OFFSET32);
	}
#if (CAN_CODE_IDHIT == STD_ON)
	if(NULL_PTR != Can_LocFifoIdhitPtr[ChnLogicId])
	{
		*Can_LocFifoIdhitPtr[ChnLogicId] = Idhit;
	}
#endif
	Callback = Can_ConfigPtr->CanReceiveCallback;
    if(NULL_PTR == Callback)
	{
		CanIf_RxIndication(&MailBoxInfo, &PduInfo);
	}
	else
	{
		if(TRUE == Callback(MailBoxInfo.Hoh, MailBoxInfo.CanId, (uint8)(PduInfo.SduLength), PduInfo.SduDataPtr))
		{
			CanIf_RxIndication(&MailBoxInfo, &PduInfo);
		}
	}
#if (CAN_CODE_IDHIT == STD_ON)
	if(NULL_PTR != Can_LocFifoIdhitPtr[ChnLogicId])
	{
		*Can_LocFifoIdhitPtr[ChnLogicId] = CAN_CONST_WHOLE_OF_DBYTE_ALL_TRUE;
	}
#endif
}

具体变更如下：

②或者直接这样修改:
35188dec-1db3-4696-aaf5-76515f8d1fdd-b5581c930b9ef10da7e6318c56e6877.png
三、述求
希望云途大佬能够帮忙评估一下这样修改有无其他影响？
这两种修改方式哪种会更好一些？是否均可？
谢谢支持

jiankang_wang

感谢您的反馈！
这个Bug在解决这个帖子的Issue时被修复掉了，但是最新的驱动还需要一点时间才能release，所以目前建议您修改如下：

这样的修改后将先判断数据的有效位数，然后再获取数据，这样就不存在数组越界风险了。

修改有无其他影响？

原本驱动也考虑到了数据截断的问题，但没有考虑到可能会存在越界到不可访问的区域。这样的修改于原代码相比较，只是调整了接收数据长度的判断位置，所以不会有其他的影响。

此外，原本的判断代码是包裹在一个 if~elseif~else 代码块中，用于区分接收到的消息是Legacy FIFO, Enhanced FIFO, 以及 MB，包裹在这个代码块中，是为了避免接收FIFO消息时不必要的判断，所以可以直接将这部分代码一移出代码块。

这两种修改方式哪种会更好一些？是否均可？

两种方式中，第一种好一点。因为这样只是发生了数据截断，而第二种方式中，会导致 callback 函数中的报文数据是随机的值，debug起来更难理解。

YunTu Forum

MC芯片使用MCAL发现CAN外设的Can_Lld_ReceiveOneFrame函数存在bug

Online Users