Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

dlt_config_file_parser.c:Fix a pointer release bug in the file。 #376

Merged
merged 1 commit into from
May 11, 2022

Conversation

Leslie-bcy
Copy link
Contributor

@Leslie-bcy Leslie-bcy commented May 4, 2022

In the dlt_config_file_set_section function of dlt_config_file_parser.c, the following code exists:

s-name is not set to null after free.

It will be freed again in the dlt_config_file_release function.

@thanhbnq
Copy link
Collaborator

thanhbnq commented May 9, 2022

Thanks @Safe-BCY ,
I strongly believe that the commit message should be updated instead of only "bb".
Please kindly update (please refer to other commits for format)
Thanh

@Leslie-bcy
Copy link
Contributor Author

Thank you, I will resubmit according to the format.

@Leslie-bcy Leslie-bcy changed the title bb dlt_config_file_parser.c:Fix a pointer release bug in the file。 May 10, 2022
@Leslie-bcy
Copy link
Contributor Author

Thanks @Safe-BCY , I strongly believe that the commit message should be updated instead of only "bb". Please kindly update (please refer to other commits for format) Thanh

The format has been modified as required, please check。

@Leslie-bcy
Copy link
Contributor Author

further information:The Same pointer(s->name)is freed again in the dlt_config_file _release。

void dlt_config_file_release(DltConfigFile *file)
{
int i = 0;

if (file != NULL) {
    int max = file->num_sections;

    for (i = 0; i < max; i++) {
        DltConfigFileSection *s = &file->sections[i];
        DltConfigKeyData *node = file->sections[i].list;
        free(s->name);    //free again。

        if (s->keys != NULL)
            free(s->keys);

        while (node) {
            DltConfigKeyData *tmp = node;
            node = node->next;
            free(tmp->key);
            free(tmp->data);
            free(tmp);
        }
    }

    free(file->sections);
    free(file);
}

}

@thanhbnq
Copy link
Collaborator

@Safe-BCY
Well, it looks like you just update information in your comment, not about the commit message itself.
We still see same as before. You can check via tab "Commits" in this PR.
Thank you,
Thanh

@Leslie-bcy
Copy link
Contributor Author

@Safe-BCY Well, it looks like you just update information in your comment, not about the commit message itself. We still see same as before. You can check via tab "Commits" in this PR. Thank you, Thanh

@Leslie-bcy Leslie-bcy closed this May 10, 2022
@Leslie-bcy Leslie-bcy reopened this May 10, 2022
@Leslie-bcy Leslie-bcy closed this May 10, 2022
@Leslie-bcy
Copy link
Contributor Author

Sorry, this is the first time I submit PR.Please see if what I submit now meets the requirements.

@Leslie-bcy Leslie-bcy deleted the aaa branch May 10, 2022 06:55
@Leslie-bcy Leslie-bcy restored the aaa branch May 10, 2022 06:56
@Leslie-bcy Leslie-bcy reopened this May 10, 2022
@thanhbnq
Copy link
Collaborator

thanhbnq commented May 10, 2022

@Safe-BCY ,

  1. In case you want to update your commit, please update via "amend". Then you will see one commit instead of two now in your develop branch.
  2. In your commit message, I think there are several references for you:
    https://gist.github.com/robertpainsi/b632364184e70900af4ab688decf6f53
  3. The information in your current commit message could be shared via the conversation of PR. Then you still can keep the commit message clean, short and informative.
  4. Please add sign-off at the end of git commit message.

Thank you,
Thanh

In the dlt_config_file_set_section function of dlt_config_file_parser.c:
  s-name is not set to null after free.
  It will be freed again in the dlt_config_file_release function.

Signed-off-by: Zhongyang.Bao <Zhongyang.Bao@zeekrlife.com>
@Leslie-bcy
Copy link
Contributor Author

Thank you. I have revised it again. Please check it, please.

@thanhbnq thanhbnq merged commit 6a3bd90 into COVESA:master May 11, 2022
@thanhbnq
Copy link
Collaborator

@Safe-BCY
It is good now. Thank you very much!

@Leslie-bcy
Copy link
Contributor Author

@Safe-BCY It is good now. Thank you very much!

May I ask if you can help apply for CVE number?

@Leslie-bcy
Copy link
Contributor Author

@thanhbnq May I ask if you can help apply for CVE number?

@carnil
Copy link

carnil commented Jun 16, 2022

CVE-2022-31291 seems to have been assigned for this issue.

@Leslie-bcy
Copy link
Contributor Author

CVE-2022-31291 seems to have been assigned for this issue.

Thank you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants