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

Container id buffer struct #299

Merged
merged 1 commit into from
Jun 19, 2024

Conversation

banditopazzo
Copy link
Member

Reviewing #293 I noticed a potential issue with the signature of the function get_container_info.

The above mentioned PR changes are the following:

- static __always_inline int get_container_info(struct task_struct *cur_tsk, char *buf, size_t sz, int *offset)
+ static __always_inline int get_container_info(struct task_struct *cur_tsk,
                                              char *buf, int *offset)

The new version assumes that char *buf is big at least CONTAINER_ID_MAX_BUF using this size in bpf helpers, but this is not true just looking at the signature.

The old version it's not free of issue: it takes the size_t sz from the caller and safely use this size in the helpers. In general this approach is fine, it's the correct way of use it but in this case we need the buffer at least of size 72. Now we rely on the documentation of the function:

/*
...
The array size MUST be greater than 72.
...
*/

but I'm proposing to use a dedicated struct for the job. The struct enforces the size of the buffer and as a plus we can pack int id_offset with the buffer.

@banditopazzo banditopazzo changed the base branch from main to vadorovsky/docker-openrc June 19, 2024 10:49
@banditopazzo banditopazzo force-pushed the container_id_buffer_struct branch from 1a525bd to 896d07f Compare June 19, 2024 10:50
@banditopazzo banditopazzo requested a review from vadorovsky June 19, 2024 10:51
@banditopazzo banditopazzo force-pushed the container_id_buffer_struct branch from 896d07f to b830118 Compare June 19, 2024 10:54
@vadorovsky
Copy link
Member

Nice, definitely cleaner than the approach with passing a pointer to offset integer.

@vadorovsky vadorovsky merged commit 802a07b into vadorovsky/docker-openrc Jun 19, 2024
@vadorovsky vadorovsky deleted the container_id_buffer_struct branch June 19, 2024 12:48
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