Pulse-Eight / libcec

USB CEC Adapter communication Library http://libcec.pulse-eight.com/
Other
705 stars 282 forks source link

RPI: "stack smashing detected" crash with libcec_transmit when number of parameters >= 33 #602

Open ssalonen opened 2 years ago

ssalonen commented 2 years ago

I can crash the app when number of parameters exceed 33 or more.

Minimal code to reproduce the issue.

Crash with #define PARAMSIZE 33 but not with #define PARAMSIZE 32

//
// main.c
//
#include <stdio.h>
#include <stdint.h>
#include <cecc.h>

#ifndef LIBCEC_OSD_NAME_SIZE
#define LIBCEC_OSD_NAME_SIZE 16
#endif

// PARAMSIZE 33: *** stack smashing detected ***: ./main terminated
// PARAMSIZE 32  seems ok
#define PARAMSIZE 33

void logger(void *cbparam, const cec_log_message *message)
{
    printf("LOG: %s\n", message->message);
}

int main()
{
    libcec_configuration cec_config;
    int openStatus;
    cec_datapacket params;
    cec_command command;
    char device_name[LIBCEC_OSD_NAME_SIZE] = "RPITest";

    ICECCallbacks callbacks;
    callbacks.logMessage = NULL;
    callbacks.keyPress = NULL;
    callbacks.commandReceived = NULL;
    callbacks.configurationChanged = NULL;
    callbacks.alert = NULL;
    callbacks.menuStateChanged = NULL;
    callbacks.sourceActivated = NULL;
    callbacks.logMessage = &logger;

    libcec_clear_configuration(&cec_config);
    cec_config.clientVersion = LIBCEC_VERSION_CURRENT;
    cec_config.bActivateSource = 0;
    cec_config.callbacks = &callbacks;

    memcpy(cec_config.strDeviceName, device_name, LIBCEC_OSD_NAME_SIZE);
    cec_config.deviceTypes.types[0] = CEC_DEVICE_TYPE_AUDIO_SYSTEM;

    printf("Initializing\n");
    libcec_connection_t connection = libcec_initialise(&cec_config);
    if (connection == NULL)
    {
        printf("Failed to initialize\n");
        return 1;
    }
    printf("Initialized: %p\n", connection);

    printf("opening connection\n");
    if ((openStatus = libcec_open(connection, "RPI", 1000)) == 0)
    {
        printf("Failed to open\n");
        return 2;
    }
    printf("opened connection: %d\n", openStatus);

    uint8_t paramdata[PARAMSIZE] = {0};
    paramdata[0] = 5;
    memcpy(params.data, paramdata, PARAMSIZE);
    params.size = PARAMSIZE;

    command.ack = 1;
    command.destination = CEC_DEVICE_TYPE_TV;
    command.initiator = CECDEVICE_AUDIOSYSTEM;
    command.eom = 1;
    command.parameters = params;
    command.opcode_set = 1;
    command.opcode = CEC_OPCODE_REPORT_AUDIO_STATUS;
    command.transmit_timeout = 2000;

    printf("transmitting command\n");
    libcec_transmit(connection, &command); 
    printf("transmitted command\n");

    printf("closing connection\n");
    libcec_close(connection);
    printf("closed connection\n");
    return 0;
}

Build with

gcc -I/usr/include/libcec -lcec -lp8-platform -no-pie -ggdb3 -o main main.c

Running

pi@REDACTED:~$ ./main 
Initializing
Initialized: 0x1890318
opening connection
LOG: Broadcast (F): osd name set to 'Broadcast'
LOG: Open - vc_cec initialised
LOG: logical address changed to Free use (e)
LOG: connection opened
LOG: << Broadcast (F) -> TV (0): POLL
LOG: initiator 'Broadcast' is not supported by the CEC adapter. using 'Free use' instead
LOG: << e0
LOG: processor thread started
LOG: >> POLL sent
LOG: TV (0): device status changed into 'present'
LOG: << requesting vendor ID of 'TV' (0)
LOG: << e0:8c
LOG: >> 0f:87:00:00:f0
LOG: TV (0): vendor = Samsung (0000f0)
LOG: expected response received (87: device vendor id)
LOG: replacing the command handler for device 'TV' (0)
LOG: registering new CEC client - v4.0.4
LOG: detecting logical address for type 'audiosystem'
LOG: trying logical address 'Audio'
LOG: << Audio (5) -> Audio (5): POLL
LOG: << 55
LOG: >> TV (0) -> Broadcast (F): device vendor id (87)
LOG: << 55
LOG: >> POLL not sent
LOG: using logical address 'Audio'
LOG: Audio (5): device status changed into 'handled by libCEC'
LOG: Audio (5): power status changed from 'unknown' to 'on'
LOG: Audio (5): vendor = Pulse Eight (001582)
LOG: Audio (5): CEC version 1.4
LOG: AllocateLogicalAddresses - device '0', type 'audio system', LA '5'
LOG: logical address changed to Audio (5)
LOG: Audio (5): osd name set to 'RPITest'
LOG: Audio (5): menu language set to 'eng'
LOG: GetPhysicalAddress - physical address = 4000
LOG: AutodetectPhysicalAddress - autodetected physical address '4000'
LOG: Audio (5): physical address changed from ffff to 4000
LOG: << Audio (5) -> broadcast (F): physical address 4000
LOG: << 5f:84:40:00:05
LOG: CEC client registered: libCEC version = 4.0.4, client version = 4.0.4, firmware version = 1, logical address(es) = Audio (5) , physical address: 4.0.0.0, git revision: libcec-4.0.4, compiled on Fri Feb  1 01:48:47 UTC 2019 by root@hostname: Name or service not known on Linux 4.15.0-44-generic (armv7l), features: P8_USB, DRM, P8_detect, randr, RPi
LOG: << Audio (5) -> TV (0): OSD name 'RPITest'
LOG: << 50:47:52:50:49:54:65:73:74
LOG: << requesting power status of 'TV' (0)
LOG: << 50:8f
LOG: >> 05:90:01
LOG: TV (0): power status changed from 'unknown' to 'standby'
opened connection: 1
transmitting command
LOG: >> TV (0) -> Audio (5): report power status (90)
LOG: expected response received (90: report power status)
LOG: << 50:7a:05:00:00:00:00:00:00:00:00:00:00:00:00:00:00:00:00:00:00:00:00:00:00:00:00:00:00:00:00:00:00:00:00
LOG: sending command 'report audio status' failed (-1)
*** stack smashing detected ***: ./main terminated
Aborted (core dumped)

System info

pi@REDACTED:~$ dpkg -l libcec4* libp8*
Desired=Unknown/Install/Remove/Purge/Hold
| Status=Not/Inst/Conf-files/Unpacked/halF-conf/Half-inst/trig-aWait/Trig-pend
|/ Err?=(none)/Reinst-required (Status,Err: uppercase=bad)
||/ Name              Version       Architecture  Description
+++-=================-=============-=============-========================================
ii  libcec4:armhf     4.0.4.1~stret armhf         libCEC communication Library (shared lib
ii  libcec4-dbgsym:ar 4.0.4.1~stret armhf         debug symbols for libcec4
un  libp8-platform    <none>        <none>        (no description available)
ii  libp8-platform-de 2.1.0.2~stret armhf         Pulse-Eight platform support library -- 
ii  libp8-platform2:a 2.1.0.2~stret armhf         Pulse-Eight platform support library

pi@REDACTED:~$ uname -a
Linux hifiberry 4.19.66-v7+ #1253 SMP Thu Aug 15 11:49:46 BST 2019 armv7l GNU/Linux

pi@REDACTED:~$ lsb_release -a
No LSB modules are available.
Distributor ID: Raspbian
Description:    Raspbian GNU/Linux 9.13 (stretch)
Release:    9.13
Codename:   stretch

Possible root cause analysis

I am not well-versed debugging issues like this. However, reading the code, RPiCECAdapterMessageQueue.cpp@libcec-4.0.4, I see some issues:

cec_adapter_message_state CRPiCECAdapterMessageQueue::Write(const cec_command &command, bool &bRetry, uint32_t iLineTimeout, bool bIsReply, VC_CEC_ERROR_T &vcReply)
<SNIP>

#if defined(RPI_USE_SEND_MESSAGE2)
<SNIP>
#else
<SNIP>
 uint8_t payload[32];
<SNIP>
    for (uint8_t iPtr = 0; iPtr < command.parameters.size; iPtr++)
      payload[iPtr + 1] = command.parameters.At(iPtr);
  }
<SNIP>

#fi

Since command.parameters can be up to 64 uint8_t (ref: cectypes.h@libcec-4.0.4), we can easily go out-of-bounds with payload local variable which is only 32 uint8_t.

There are some similarities to this issue, although that was report for Pulse8 adapter, not RPI adapter.