On Thu, 29 Jul 2021 08:08:37 +0300
"Tzvetomir Stoyanov (VMware)" <tz.stoyanov@xxxxxxxxx> wrote:
> Added a safety check to ensure requested buffer index is valid.
>
> Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@xxxxxxxxx>
> ---
> lib/trace-cmd/trace-input.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/lib/trace-cmd/trace-input.c b/lib/trace-cmd/trace-input.c
> index af11cbc6..787d6825 100644
> --- a/lib/trace-cmd/trace-input.c
> +++ b/lib/trace-cmd/trace-input.c
> @@ -3946,13 +3946,14 @@ struct tracecmd_input *
> tracecmd_buffer_instance_handle(struct tracecmd_input *handle, int indx)
> {
> struct tracecmd_input *new_handle;
> - struct input_buffer_instance *buffer = &handle->buffers[indx];
> + struct input_buffer_instance *buffer;
> size_t offset;
> ssize_t ret;
>
> if (indx >= handle->nr_buffers)
> return NULL;
>
> + buffer = &handle->buffers[indx];
This part is unneeded.
You could have indx = 10000000000000, and it wont bug.
Try it!
$ echo '
#include <stdio.h>
struct my_buffer {
int buf;
};
struct my_handle {
struct my_buffer *buffers;
};
int main() {
int indx = 10000000;
struct my_buffer buf;
struct my_handle hand = { .buffers = &buf };
struct my_handle *phand = &hand;
struct my_buffer *pbuf = &phand->buffers[indx];
printf("pbuf = %p\n", pbuf);
return 0;
}' > /tmp/blah
$ gcc -o /tmp/blah /tmp/blah.c -g -Wall
$ /tmp/blah
pbuf = 0x7ffe867b9b74
The reason is because we are getting the address of the indexed
location, and we are not dereferencing it. Thus, it is perfectly safe
to keep the code as is. There was no safety check added. Please remove
this hunk.
/me is reminded of the first X-Men movie, where Rogue warned Wolverine
about the guy that was about to stab him. Afterward, she said, "I saved
your life", and Wolverine replied "No you didn't.". As Rogue didn't
know that Wolverine had super healing powers where the knife would not
kill him. ;-)
-- Steve
|