callbackRequest calls epicsInterruptContextMessage with string stored on stack

[Ambroz Bizjak]

This is a theoretical report found incidentally when looking at the code.

The RTEMS implementation of epicsInterruptContextMessage requires the sting remains available (src/libCom/osi/os/RTEMS/osdInterrupt.c):

/*
 * Pass a message from an interrupt context.
 * Note that this passes a pointer to the message, not the message itself.
 * This implies that the message must remain valid after the
 * interrupt context is no longer active.
 */
void
epicsInterruptContextMessage (const char *message)

But callbackRequest calls it with a string constructed on the stack (src/db/callback.c):

    if (!pushOK) {
        char msg[48] = "callbackRequest: ";

        strcat(msg, threadName[priority]);
        strcat(msg, " ring buffer full\n");
        epicsInterruptContextMessage(msg);
        ringOverflow[priority] = TRUE;
    }

This implies that the message printed may be garbage and theoretically a crash may happen depending on what happens to be on that thread's stack when the message is read.

[Andrew Johnson (anj)]

This aspect of the epicsInterruptContextMessage() API is undocumented, the AppDevGuide says nothing about the message string. I put that message creation code into callbackRequest(); all other calls to epicsInterruptContextMessage() in Base pass in a static string.

The osi/os/vxWorks implementation calls logMsg() which queues the whole string; the osi/os/default implementation calls errlogPrintf() which behaves similarly but that's only legal because its version of epicsInterruptIsInterruptContext() can never return true.

Heinz's new osi/os/RTEMS-posix implementation (for RTEMS 4.12, which should be merged into EPICS 7) just calls printk() which is presumably safe, but the original that Ambroz posted above is still present in osi/os/RTEMS for the older RTEMS versions.

The only solution I can see which retains the nicer message from callbackRequest() would be to have 3 static error strings and pick one based on the priority. Suggested patch attached (based on 3.14 branch), but I haven't tested it. We should also document the static string requirement properly.

[Ben Franksen (bfrk)]

A perfectly valid and appropriate fix IMO.