Menu

#193 2nd batch of untested patches for cppcheck complaints, mostly bounds checks

v1.0 (example)
open
nobody
None
7
2019-05-28
2019-05-28
No

This is a second set of a untested fixes for some other cppcheck complaints:

  1. XDmaPs_IsActive() in FreeRTOS/Demo/CORTEX_A9_Zynq_ZC702/RTOSDemo_bsp/ps7_cortexa9_0/libsrc/dmaps_v2_3/src/xdmaps.c could access invalid memory if (incorrectly?) called with parameter Channel = XDMAPS_CHANNELS_PER_DEV.

  2. Printf format typographical error: "100%" should be "100%%" in S25FL1D_EraseChip() in FreeRTOS/Demo/CORTEX_M7_SAMV71_Xplained_IAR_Keil/libboard_samv7-ek/source/s25fl1.c.

  3. In FreeRTOS/Demo/CORTEX_M7_SAMV71_Xplained_IAR_Keil/libboard_samv7-ek/source/s25fl1.c, the functions Xil_SetMPURegion and Xil_SetMPURegionByRegNum each take a 64-bit size parameter, and act on invalid data if the specified size was greater than 4G. Cortex M7 has a vritual address space size of 4GB, so I think my additions would server only to catch bad parameters.

  4. generate_rtos_stats() in FreeRTOS/Demo/ColdFire_MCF52233_Eclipse/RTOSDemo/webserver/httpd-cgi.c wrote past the end of the 32 character array cCountBuf[], 45 characters past by my figuring. At first, I just increased the size of the array, but then I realized the array was unnecessary, because its contents was only written to another location in memory, which sprintf could just as easily target directly, so my patch eliminates cCountBuf.

  5. In FreeRTOS/Demo/Common/ethernet/FreeTCPIP/uip.c in uip_reass(), there appears to be a potential buffer overflow because an overflow guard failed to account for the first UIP_IPH_LEN (whatever that means) bytes of uip_reassbuf[] already being used. So, I adjusted the guard accordinly. Also, it was previous checking offset < N and (offset + len) < N, but len and offset are both unsigned 16 bit values, so promoting them both to 32-bit and comparing only offset + len should suffice, as len cannot be negative, and adding two 16 bit unsigned integers will never overflow a 32-bit integer. So, that is what I put in the patch.

I would also like to add the following note to Richard Barry: Thanks so much for processing my previous set of suggested fixes to cppcheck complaints, but I am embarassed to write that I made a serious typographical error in my previous submission of the wolfssl change. I accidentally deleted an fwrite call. I put notes in the closed ticket ( https://sourceforge.net/p/freertos/bugs/192/ ) and the discussion thread about this, but I wanted to mention it in this new ticket, to make sure you get notified. Also, as long as I am correcting my previous patch, I should also mention that, I broke an apparent convention against using tabs in that WolfSSL file with my patch. If it is easier for you if I post a new version of that patch, please let me know and I'll do it.

I am sorry I have not yet figured out how to build FreeRTOS to check that these changes at least compile, but I thought I should send changes #4 and #5 from the above list promptly, and that I might as well send along the other changes too while they are fresh in my mind.

Anyhow, I hope this is helpful. Thanks for the prompt processing of my previous patch submission and thanks in advance for considering this one.

1 Attachments

Discussion


Log in to post a comment.

MongoDB Logo MongoDB