linux-can / can-utils

Linux-CAN / SocketCAN user space applications
2.4k stars 712 forks source link

When ISO-TP Socket is not bound the call to write sets errno EADDRNOTAVAIL #349

Closed derek-will closed 2 years ago

derek-will commented 2 years ago

When a CAN_ISOTP socket is not bound, then a call to write sets errno EADDRNOTAVAIL.

I would expect EADDRNOTAVAIL to be returned when passing in nonsense address information into bind which according to the source code it does.

Is there a rationale for this errno on write when the socket is not bound?

derek-will commented 2 years ago

After doing some thinking and experimenting, for UDP sockets one would get EDESTADDRREQ if a write is attempted without a connect. However, CAN_ISOTP has no support for connect and so therefore EADDRNOTAVAIL is the next best thing. The whole "Cannot assign requested address" comment just threw me as unexpected and it is typically an errno associated with bind. Something like EBADF would adhere to the manpage for write and "File descriptor in bad state" would sort of make sense.

On the other end, when trying to do a read on a UDP socket without calling bind first then the call will block indefinitely. CAN_ISOTP exhibits the same behavior. It is sort of undesirable. Now this is likely outside the scope of SocketCAN, but it would be nice if the sockets API would return an error and set an appropriate errno in such a state, but it appears that this is not the case. I am always a big fan of "fail early" and it seems like the sockets API does not do this in this case.

hartkopp commented 2 years ago

On the other end, when trying to do a read on a UDP socket without calling bind first then the call will block indefinitely. CAN_ISOTP exhibits the same behavior. It is sort of undesirable. Now this is likely outside the scope of SocketCAN, but it would be nice if the sockets API would return an error and set an appropriate errno in such a state, but it appears that this is not the case. I am always a big fan of "fail early" and it seems like the sockets API does not do this in this case.

Oh, that's an interesting idea. If I understand you correctly you suggest to return EADDRNOTAVAIL when reading from an unbound isotp socket?

derek-will commented 2 years ago

Oh, that's an interesting idea. If I understand you correctly you suggest to return EADDRNOTAVAIL when reading from an unbound isotp socket?

Thanks for filtering through those thoughts. :-)

Yes, I think it would be best if when someone attempts to read from an unbound ISO-TP socket that they get rejected and the appropriate errno is set. I do not think there is a case or there ever would be one where an application developer would need to read from an unbound ISO-TP socket.

For example, with the J2534 API a call to PassThruReadMsgs will return ERR_NO_FLOW_CONTROL on a ISO15765 channel without a filter set.

If reads on unbound sockets will never succeed, then rejecting the call to read, recv, etc. and setting an appropriate errno will assist the application developer in tracing errors in their code. I think EADDRNOTAVAIL is a sensible choice.

hartkopp commented 2 years ago

I think EADDRNOTAVAIL is a sensible choice.

I would like to add a Suggested-by: tag, when creating a patch for it, if you are fine to send me an email address that would show up in the Linux kernel then. Unfortunately a GitHub account name does not make it through the checkpatch script ;-)

derek-will commented 2 years ago

Hi Oliver, I emailed you my email address. What a strange statement. :-)

hartkopp commented 2 years ago

Hi Derek,

your suggestion is now in the upstream process and will show up in Linux 5.18 (and the stable kernels): https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net-next.git/commit/?id=30ffd5332e06

Many thanks, Oliver