c++memory-leakslibssh

memory leak while using ssh_channel_open_session in an array


I have this function which executes a command through a ssh channel:

std::string ssh::exec_ssh_command(const char* command)
{
    std::string receive = "";
    std::string err;
    int rc, nbytes;
    char buffer[2000];
    MyException errMsg;
    try {
        ssh_channel channel = ssh_channel_new(my_ssh_session);
        if (channel == NULL)
        {
            receive = "Channel allocation failed.";
            throw MyException(receive);
        }
        rc = ssh_channel_open_session(channel);
        if (rc != SSH_OK)
        {
            receive = "Opening session channel failed : ";
            receive += ssh_get_error(my_ssh_session);

            throw MyException(receive);
        }

        rc = ssh_channel_request_exec(channel, command);
        if (rc != SSH_OK) {
            receive = "Channel's request executing failed.";
            throw MyException(receive);
        }
        nbytes = ssh_channel_read(channel, buffer, sizeof(buffer), 0);
        receive = buffer;
        if (nbytes > 0)
        {
            receive.erase(nbytes - 1, 2000);
        }
        else
        {
            receive = "Error in command: not found or wrong syntax";
            throw MyException(receive);
        }

        if (nbytes < 0)
        {
            receive = "Error in reading data from channel ";
            throw MyException(receive);
        }

        ssh_channel_free(channel);
    }
    catch (MyException& err)
    {
        ssh_channel_free(channel);

        throw err;

    }
    return receive;
}

I used this function to run commands before and get a single output. now I want to use this function in a loop, like below:

Service monitoring::osFields()
{
    std::string num;
    int serviceNumbers, active, faild, loaded, not_found;
    serviceNumbers = getServiceNumbers();
    Service *services = new Service[serviceNumbers];

    std::string service_name_command;
    std::string service_load_state_commands;
    std::string service_active_state_commands;
    std::string service_sub_state_commands;
    try
    {
        num = sshObj->exec_ssh_command("systemctl list-units --state active | grep service | wc -l");
        active = std::stoi(num);
        for (int i = 0; i < active; i++)
        {
            service_name_command = "systemctl list-units --state active | grep service | sed -s -n " + std::to_string(i+1) + "p | awk '{print $1}'";
            services[i].name = sshObj->exec_ssh_command(service_name_command);

            service_load_state_commands = "systemctl list-units --state active | grep service | sed -s -n " + std::to_string(i+1) + "p | awk '{print $2}'";
            services[i].load = sshObj->exec_ssh_command(service_load_state_commands);

            service_active_state_commands = "systemctl list-units --state active | grep service | sed -s -n " + std::to_string(i + 1) + "p | awk '{print $3}'";
            services[i].active = sshObj->exec_ssh_command(service_active_state_commands);

            service_sub_state_commands = "systemctl list-units --state active | grep service | sed -s -n " + std::to_string(i + 1) + "p | awk '{print $4}'";
            services[i].sub = sshObj->exec_ssh_command(service_sub_state_commands);

        }
        
    }
    catch (MyException& err)
    {
        throw MyException("in function smc_getNICs ::" + string(err.what()));
    }
    return *services;
}

getServiceNumbers(); is a function to count service numbers.
and this is Service structure:

struct Service {
    const char* name;
    const char* load;
    const char* active;
    const char* sub;
};

I have a connectSession function which is called in constructor of ssh class and makes a ssh session. whenever I run my code, it crashes in i=43. it is about memory leak and mostly stops in ssh_channel_open_session function, but sometime on ssh_channel_request_exec.
EDIT:
exec_ssh_comman overload:


const char * ssh::exec_ssh_command(std::string command)
{
    const char* _retVal = NULL;

    _retVal = stringToConstChar(exec_ssh_command(command.c_str()));

    return _retVal;
}

stringToCharfunction:

const char* stringToConstChar(string str)
{
    const char* command = new char[str.size()];
    strcpy((char*)command, str.c_str());
    const char* cm = command;
    return cm;
}

Solution

  • const char* command = new char[str.size()];
    strcpy((char*)command, str.c_str());
    const char* cm = command;
    return cm;
    

    There's your memory leak. This newed char buffer does not get deleted anywhere.

    Additionally, the char buffer is too small, and this results in memory corruption as well. Unless fixed, this program will start randomly crashing, in random, unrelated places as you continue to develop and/or run this code repeatedly.

    I don't see anything that's actually accomplished here by doing all of these complicated and elaborate steps of copying the contents of a std::string into a separate char buffer (that's too small), and then leaking it. Looks like the only thing this const char * is used for is constructing a completely separate and independent std::string, at which point the memory gets leaked.

    The simplest fix is to just get rid of all of this, completely. This should just return the original std::string. In general, modern C++ code rarely needs to new or delete anything, and this is rarely seen in modern, clean C++ code which uses objects like std::string and various containers to correctly handle all memory allocations and deallocations. The easiest way to avoid bugs related to memory allocations is to simply not do it yourself, and let the C++ library manage memory for you.