Re: [PATCH net v11] can: isotp: fix timer drain order, wakeup handling and tx_gen ordering

On 25.07.26 14:31, [email protected] wrote:

>> +
>> +		/* drain to avoid stale error for a later poll()/SO_ERROR */
>> +		sock_error(sk);
>> +
>> +		return err ? err : size;
> 
> [Severity: High]
> Does discarding the return value of sock_error() here silently drop
> asynchronous RX socket errors?
> 
> If a concurrent RX timeout or malformed PDU sets sk->sk_err to ETIMEDOUT
> or EBADMSG, calling sock_error(sk) will unconditionally clear it to 0. Since
> the returned value is ignored and the function prioritizes returning the TX
> result or size, it seems the application will never be notified of these RX
> failures via sendmsg(), poll(), or getsockopt(SO_ERROR).
> 
> Should we preserve or return this error if no TX error occurred?

sk_err is a single per-socket slot shared by the RX and TX paths, so it 
cannot cleanly separate a TX result from a concurrent RX error.

Note that a plain unreliable datagram protocol like UDP/IP gives no 
feedback for bad or missing received data at all - it just silently 
drops it. ISO-TP does a bit more and surfaces RX problems 
(EBADMSG/EILSEQ/ETIMEDOUT) via sk_err on a best-effort basis, which is 
fine: for an unreliable protocol these are ultimately caught by timeouts 
at a higher ISO layer anyway.

What this patch fixes to 100% is the send path's POSIX contract: 
sendmsg() now returns the TX-specific result from tx_result[] (>0 bytes 
on success, <0 on a real TX error), instead of returning whatever 
happened to sit in sk_err - which could misreport an unrelated 
concurrent RX error as the TX outcome. Draining sk_err afterwards is 
required in CAN_ISOTP_WAIT_TX_DONE mode, since the result is delivered 
synchronously via the return value.

So the send path is now correct and race-free; the shared-sk_err RX 
notification stays best-effort by design. Truly separating the two would 
need distinct RX/TX error channels (e.g. MSG_ERRQUEUE), which is a 
feature, not a fix, and out of scope here.

Best regards,
Oliver