pcStatusString size

casper0 wrote on Thursday, May 17, 2012:

Maybe I am the only one who likes to use long task names (>22 chars), or I am doing something wrong, but when I tried printing status for tasks with long names I ended writing beyond the end of available string space and corrupting other data.

It appears that giving room for 30 characters for the variables ought to be enough and then add space for the longest possible task name.

Current>>> PRIVILEGED_DATA static char pcStatusString; // Not enough space for long task names.
Proposed>>> PRIVILEGED_DATA static char pcStatusString;

I am also thinking of changing the “sprintf” to a “snprintf”, and limiting the string size to the available memory.

Noticed this in FreeRTOS 7.1.0 and 7.1.1

xz8987f wrote on Friday, May 18, 2012:

Yes, any usage of sprintf() in the FreeRTOS kernel is a source of buffer overflow. That’s why in my port/version for Processor Expert and CodeWarrior (http://www.steinerberg.com/EmbeddedComponents/FreeRTOS) all calls have been replaced with save string routines which do not write beyond the buffers.
I think a similar thing needs to be done for the Kernel base sources.


rtel wrote on Friday, May 18, 2012:

Ideally there would be no limit on the length of a task’s name, and the TCB would just contain a pointer to the string.  However, that is dangerous because it is assumed that the string is persistent in memory, and not in a temporary buffer or on the stack.  Therefore the name is instead copied into an array using the following code:

strncpy( ( char * ) pxTCB->pcTaskName, ( const char * ) pcName, ( unsigned short ) configMAX_TASK_NAME_LEN );

That should in theory prevent the array from overflowing, presuming strncpy() is implemented correctly.  The line of code that follows then ensures the string is terminated correctly, in case the entire array was filled.

However, many C libraries that come with compilers require a large ROM footprint and huge stacks, so many FreeRTOS demos come with a cut down version of sprintf() that has limited functionality, but requires very little ROM or RAM.  To ensure projects still link, the file that contains the sprintf() function also contains an snprintf() function, but it is not implemented correctly and just blindly calls sprintf().  If you project contains the file (third party) file named printf-stdarg.c, then this is most likely the cause of your problem.

This has been discussed a few times here, and I think, although it is a third party file, I need to do something about it for the next release.


rtel wrote on Friday, May 18, 2012:

Sorry guys, for the second time this morning I have provided an incorrect answer.  Scrub my last reply above.  It is not strncpy() that is not implemented, it is snprintf() that is not implemented in printf-stdarg.c.  strncpy() is not even mentioned in printf-stdarg.c.  I mis-read your original post.

See if I can do better this time….

The reason snprintf() is not used in prvListTaskWithinSingleList() is for the reason I mentioned in my previous post, so maybe there was some accidental relevance to that post after all.

I take your point that the arbitrary 50 in the code is a very bad thing.  In fact, I’m surprised it is there as it breaks the coding standard for it not to use a define.  I have just changed it to:

PRIVILEGED_DATA static char pcStatusString[ configMAX_TASK_NAME_LEN + 30 ];

Although that leaves a magic number, it is more than adequately sized.