You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
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:
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?
The text was updated successfully, but these errors were encountered:
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.
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 functionstatic int LinuxSockBind()
in the fileiso14229.c
is declared but its memory is not cleared before using it. After that, the return value of the call toioctl()
is not checked for errors like in the calls tosocket()
,setsockopt()
, andbind()
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: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 tobind()
after it.I see that within the same function you use either
perror
orprintf()
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?
The text was updated successfully, but these errors were encountered: