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

fix some FDs leaks #334

Merged
merged 6 commits into from
Feb 6, 2024
Merged

Conversation

giuseppe
Copy link
Collaborator

@giuseppe giuseppe commented Feb 6, 2024

more information in each commit message.

Found by static analysis.

Signed-off-by: Giuseppe Scrivano <[email protected]>
it is freed with "free" in different places, so allocate using the
stdlib function.

Signed-off-by: Giuseppe Scrivano <[email protected]>
it is allocated using g_malloc0 and passed to a list, that is later
freed with g_free.

Signed-off-by: Giuseppe Scrivano <[email protected]>
Signed-off-by: Giuseppe Scrivano <[email protected]>
Signed-off-by: Giuseppe Scrivano <[email protected]>
Signed-off-by: Giuseppe Scrivano <[email protected]>
@giuseppe
Copy link
Collaborator Author

giuseppe commented Feb 6, 2024

@AkihiroSuda PTAL

@AkihiroSuda AkihiroSuda added this to the v1.2.3 milestone Feb 6, 2024
@@ -57,7 +60,7 @@ struct api_ctx {

struct api_ctx *api_ctx_alloc(struct slirp4netns_config *cfg)
{
struct api_ctx *ctx = (struct api_ctx *)g_malloc0(sizeof(*ctx));
struct api_ctx *ctx = (struct api_ctx *)calloc(1, sizeof(*ctx));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

g_malloc0 using malloc internally is just an implementation detail. Since we free ctx with free, we should either change g_malloc() -> malloc(), or free() with g_free().

Copy link
Member

@AkihiroSuda AkihiroSuda left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks

@AkihiroSuda AkihiroSuda merged commit 51ef0a9 into rootless-containers:master Feb 6, 2024
8 checks passed
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.

2 participants