spi.c Error message builder is buggy

[anthonygclark]

Hey all,

I've seen some overflows and other corruption on the SPI error message builder. I've edited it to be the following with much more success. Though, I will admit the initial sizeof(errmsg) at 96 bytes seemed concerning but doing the math between the exact separators and extra strings, 96 bytes is spot on. I didn't track down the corruption I was seeing but replacing the function with the one below works well. I didn't want to do a merge request as I'm sure this edit is opinionated.

static int _spi_error(struct spi_handle *spi, int code, int c_errno, const char *fmt, ...) {
    va_list ap;

    char sep[] = ": ";

    // split buffer access arbitrarily, why not in half?
    char temp1[sizeof(spi->error.errmsg) / 2] = {0};
    char temp2[sizeof(spi->error.errmsg) / 2 - sizeof(sep) - 1] = {0}; // -1 for null terminator

    spi->error.c_errno = c_errno;

    va_start(ap, fmt);
    vsnprintf(temp1, sizeof(temp1), fmt, ap);
    va_end(ap);

    /* Tack on strerror() and errno */
    if (c_errno) {
        // - The most we can do is sizeof(temp2) to not guarantee truncation
        // - The tool `errno` only reports a max used string of 49 bytes:
        //     errno -ls | xargs -L1  | cut -d' ' -f3- | awk '{print length}' | sort -n | tail -1
        // - snprintf will truncate anyway
        // - Put the [errno %d] hint first as any truncation will not remove valuable information
        // - 10 bytes for the extra information, this will be fragile if someone changes the extra
        //     strings, but the compiler warnings will let you know.
        char temp3[sizeof(temp2) - 10] = {0};
        snprintf(temp2, sizeof(temp2), "[errno %d] %s", c_errno, strerror_r(c_errno, temp3, sizeof(temp3)));
    }

    snprintf(spi->error.errmsg, sizeof(spi->error.errmsg),
             "%s%s%s", temp1, sep, temp2);

    return code;
}

[vsergeev]

Do you have a reproducible example of this? e.g. setting errno then formatting the message