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

clean the memory of struct ifreq ifr and check the return value of ioctl for errors #19

Closed
muehlke opened this issue Oct 12, 2023 · 1 comment

Comments

@muehlke
Copy link

muehlke commented Oct 12, 2023

Hello @driftregion ,

I tested creating some UDS clients with different interface names like "can2" or something like "can1234" to be surprised that the client could still be created and that it could send over the only available interface "can0". Then I realized that the variable struct ifreq ifr in the function static int LinuxSockBind() in the file iso14229.c is declared but its memory is not cleared before using it. After that, the return value of the call to ioctl() is not checked for errors like in the calls to socket(), setsockopt(), and bind() within the same function.

I think it did work because before I did these tests I had chosen the right interface name "can0" so the memory of the struct got the right index for interface but in another call with a wrong interface name, the call to ioctl was not successful but since the uncleared memory still uncleared the valid index for "can0" the client could still be bound to the right interface. I changed the code to:

    struct ifreq ifr;
    memset(&ifr, 0, sizeof(ifr));
    strncpy(ifr.ifr_name, if_name, sizeof(ifr.ifr_name) - 1);
    if (ioctl(fd, SIOCGIFINDEX, &ifr) < 0) {
        printf("ioctl: %s %s\n", strerror(errno), if_name);
        return -1;
    }

and this prevents the problem and is a cleaner way of dealing with the struct and the call to ioctl. The warning message pattern was taken from the call to bind() after it.

I see that within the same function you use either perror or printf() to report errors, is there a reason for that? Wouldn't it be better to keep it consistent?

What do you think about this change?

driftregion added a commit that referenced this issue Oct 21, 2023
@driftregion
Copy link
Owner

Hi @muehlke , thank you. failing to initialize struct ifreq is definitely a bug, as is not checking the return code of ioctl. Please check whether #20 fixes it.

I see that within the same function you use either perror or printf() to report errors, is there a reason for that? Wouldn't it be better to keep it consistent?

You're correct. I added a commit to #20 that reports errors on stderr. Please let me know if this works for you.

driftregion added a commit that referenced this issue Nov 1, 2023
initialize struct ifreq ifr #19
@muehlke muehlke closed this as completed Nov 3, 2023
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

No branches or pull requests

2 participants