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

Misc. fixes #1121

Merged
merged 8 commits into from
Jan 12, 2024
Merged

Misc. fixes #1121

merged 8 commits into from
Jan 12, 2024

Conversation

pcercuei
Copy link
Contributor

Fix some out-of-scope references, memory leaks, use-after-free, among other things.

setup_sig_handler() was passing as argument to the signal thread a stack
variable.

Signed-off-by: Paul Cercueil <[email protected]>
Free the attributes list, so that we don't leak memory.

Signed-off-by: Paul Cercueil <[email protected]>
The "description" string was not freed before returning on success.

Signed-off-by: Paul Cercueil <[email protected]>
When using a backend that creates a iio_context using the XML backend
internally, the code supposed to concatenate the context descriptions
did not work properly, as it did not re-set ctx->description.

Signed-off-by: Paul Cercueil <[email protected]>
There was a race between iiod_io_unref() and the read thread.

Signed-off-by: Paul Cercueil <[email protected]>
When both the DMABUF and the MMAP interfaces were enabled, and running
on a kernel with the DMABUF interface, the code would free a block's
pdata in local_free_dmabuf(), and deference it afterwards.

Signed-off-by: Paul Cercueil <[email protected]>
The context pdata structure is already freed in iio_context_destroy().

Signed-off-by: Paul Cercueil <[email protected]>
@pcercuei pcercuei merged commit 7b973b5 into main Jan 12, 2024
24 checks passed
@pcercuei pcercuei deleted the pcercuei/misc-fixes branch January 12, 2024 12:44
}

free(ctx->description);
ctx->description = new_description;
Copy link

@pamolloy pamolloy Jun 28, 2024

Choose a reason for hiding this comment

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

If description == null and ctx->description is a valid pointer this will free the valid pointer and then assign new_description (which is null) to ctx-description.

I'll try to send a patch this weekend. 🤞

Copy link
Contributor

Choose a reason for hiding this comment

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

correct... good catch!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well, I disagree. If description is NULL it makes sense that ctx->description is set to NULL, no?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nevermind, I think you are right. It would make sense if the description was replaced, but the code tries to append the old description to the new one.
So please send a PR.

Choose a reason for hiding this comment

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

Thanks for the quick replies! Just out of curiosity, how is description here normally populated? In my case I'm running iio_info -u "serial:..." and IIOD on an MCU.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Here it is populated by the XML backend from the XML string sent by IIOD, which itself will get the description from the libiio backend's iio_backend struct.

Copy link

@pamolloy pamolloy Jun 28, 2024

Choose a reason for hiding this comment

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

Sorry that wasn't phrased clearly. 😅 I understand how it works in my case.

I was curious in what cases the description that is passed in as an argument to iio_create_context_from_xml() will not be null, which I assume is the "normal" use case.

Or said otherwise, why are there potentially two descriptions that need to be concatenated?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It happens e.g. with the network backend, which prepends the IP address to the description found in the XML string.

pamolloy added a commit to pamolloy/libiio that referenced this pull request Jul 1, 2024
Add check when attempting to concatenate description strings.

When description was NULL and ctx->description was a valid string then
ctx->description was freed and replaced by NULL. This caused iio_info to
not print the backend description string.

Additionally, do not free(NULL) when ctx->description is NULL.

Fixes analogdevicesinc#1121

Signed-off-by: Philip Molloy <[email protected]>
pamolloy added a commit to pamolloy/libiio that referenced this pull request Jul 3, 2024
Add check when attempting to concatenate description strings.

When description was NULL and ctx->description was a valid string then
ctx->description was freed and replaced by NULL. This caused iio_info to
not print the backend description string.

Additionally, do not free(NULL) when ctx->description is NULL.

Fixes analogdevicesinc#1121

Signed-off-by: Philip Molloy <[email protected]>
pamolloy added a commit to pamolloy/libiio that referenced this pull request Jul 18, 2024
Add check when attempting to concatenate description strings.

When description was NULL and ctx->description was a valid string then
ctx->description was freed and replaced by NULL. This caused iio_info to
not print the backend description string.

Additionally, do not free(NULL) when ctx->description is NULL.

Fixes analogdevicesinc#1121

Signed-off-by: Philip Molloy <[email protected]>
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.

4 participants