kubernetes-client / c

Official C client library for Kubernetes
Apache License 2.0
148 stars 49 forks source link

Potential Handler Leaks #243

Closed 903d09f6-e821-4655-9e8b-7e6068b26f74 closed 4 months ago

903d09f6-e821-4655-9e8b-7e6068b26f74 commented 4 months ago

Hello. I'm using the output of this project with gratitude. I currently found two potential file handler leaks in files in the c/kubernetes/config/... directory.

  1. https://github.com/kubernetes-client/c/blob/dc7d67098361c3579e3dd8153ccc6c4c30ddfedb/kubernetes/config/exec_provider.c
    int kube_exec_and_get_result(ExecCredential_t * exec_credential, const kubeconfig_property_t * exec)
    ...
    #ifndef _WIN32
    fp = popen(command_string, "r");    /* r means read from stdout */
    #else
    fp = _popen(command_string, "r");   /* r means read from stdout */
    #endif
    if (fp) {
        result_string = calloc(1, KUBECONFIG_EXEC_RESULT_BUFFER_SIZE);
        if (!result_string) {
            fprintf(stderr, "%s: Cannot allocate memory for command result.[%s]\n", fname, strerror(errno));
    ///////////////////////// HERE is ADDED /////////////////////////
    #ifndef _WIN32
            pclose(fp);
    #else
            _pclose(fp);
    #endif
    ////////////////////////////////////////////////////////////////////
            return -1;
        }
        int result_string_remaining_size = KUBECONFIG_EXEC_RESULT_BUFFER_SIZE - 1;
        char string_buf[KUBECONFIG_STRING_BUFFER_SIZE];
        memset(string_buf, 0, sizeof(string_buf));
        while (fgets(string_buf, sizeof(string_buf), fp) != NULL) {
            result_string_remaining_size -= strlen(string_buf);
            if (result_string_remaining_size <= 0) {
                fprintf(stderr, "%s: The buffer for exec result is not sufficient.\n", fname);
                rc = -1;
    ///////////////////////// HERE is ADDED /////////////////////////
    #ifndef _WIN32
                pclose(fp);
    #else
                _pclose(fp);
    #endif
    ////////////////////////////////////////////////////////////////////
                goto end;
            }
    ...
  2. https://github.com/kubernetes-client/c/blob/dc7d67098361c3579e3dd8153ccc6c4c30ddfedb/kubernetes/config/kube_config_yaml.c
    ...
    int kubeyaml_save_kubeconfig(const kubeconfig_t * kubeconfig)
    {
    ...
    /* Set a file output. */
    FILE *output = NULL;
    if (kubeconfig->fileName) {
        output = fopen(kubeconfig->fileName, "wb");
    ...
    /* Initialize the emitter object. */
    if (!yaml_emitter_initialize(&emitter)) {
        fprintf(stderr, "%s: Could not initialize the emitter object\n", fname);
    ///////////////////////// HERE is ADDED /////////////////////////
        fclose(output);
    ////////////////////////////////////////////////////////////////////
        return -1;
    }
    ...

I'm not sure if the above files were automatically generated or not, but please consider adding these handler closing operation if you possible.

Thank you.

ityuhui commented 4 months ago

Thanks for reporting these issues. These files are not generated, but written manually. Do you want to sumbit a PR to deliver your report ? If you don't want to, I can do it.

903d09f6-e821-4655-9e8b-7e6068b26f74 commented 4 months ago

Okay. I requested it by PR #244 Check and merge please :-)

903d09f6-e821-4655-9e8b-7e6068b26f74 commented 4 months ago

This issue is fixed by PR #244