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.

Sign up for GitHub

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

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
3d0675b
utils: iio_rwdev: Fix out-of-scope reference to stack variable
pcercuei Dec 20, 2023
92a0928
buffer: Free attributes list when destroying buffer
pcercuei Dec 20, 2023
28efc96
network: Fix memory leak in network_create_context()
pcercuei Dec 20, 2023
baa4fbd
context: Fix non-working concatenation of context descriptions
pcercuei Dec 20, 2023
b7c796c
iiod-responder: Fix potential use-after-free caused by race
pcercuei Dec 20, 2023
a310cef
iio-responder: Remove useless local variable
pcercuei Dec 20, 2023
2f009a9
local: Fix use-after-free
pcercuei Dec 21, 2023
18f27cd
local: Fix double-free of context pdata
pcercuei Jan 12, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions buffer.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -195,6 +195,7 @@ void iio_buffer_destroy(struct iio_buffer *buf)
iio_task_destroy(buf->worker);
iio_mutex_destroy(buf->lock);
iio_channels_mask_destroy(buf->mask);
free(buf->attrlist.attrs);
free(buf);
}

Expand Down
7 changes: 3 additions & 4 deletions context.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -546,7 +546,7 @@ iio_create_context_from_xml(const struct iio_context_params *params,
const char **ctx_values, unsigned int nb_ctx_attrs)
{
struct iio_context *ctx;
char *new_description;
char *new_description = NULL;
unsigned int i;
ssize_t len;
int ret;
Expand Down Expand Up @@ -596,11 +596,10 @@ iio_create_context_from_xml(const struct iio_context_params *params,
prm_err(params, "Unable to alloc memory\n");
goto err_context_destroy;
}

free(ctx->description);
ctx->description = new_description;
}

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

@pamolloy pamolloy Jun 28, 2024 •
edited
Loading

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.

Copy link

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 •
edited
Loading

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.

ctx->params = *params;

return ctx;
Expand Down
2 changes: 2 additions & 0 deletions network.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -754,6 +754,8 @@ static struct iio_context * network_create_context(const struct iio_context_para
if (ret)
goto err_destroy_iiod_client;

free(description);

iio_context_set_pdata(ctx, pdata);

/* pdata->io_ctx.params points to the 'params' function argument,
Expand Down
2 changes: 1 addition & 1 deletion utils/iio_rwdev.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ static void * sig_handler_thd(void *data)

static void setup_sig_handler(void)
{
sigset_t mask, oldmask;
static sigset_t mask, oldmask;
pthread_t thd;
int ret;

Expand Down