Re: [PATCH net 3/7] net/handshake: Pass negative errno through handshake_complete()

On 5/18/26 20:24, Chuck Lever wrote:
> From: Chuck Lever <[email protected]>
> 
> handshake_complete() declares status as unsigned int and
> tls_handshake_done() negates that value (-status) before handing
> it to the TLS consumer. Consumers match on negative errno
> constants -- xs_tls_handshake_done() has
> 
> 	switch (status) {
> 	case 0:
> 	case -EACCES:
> 	case -ETIMEDOUT:
> 		lower_transport->xprt_err = status;
> 		break;
> 	default:
> 		lower_transport->xprt_err = -EACCES;
> 	}
> 
> so the API as designed expects callers to pass positive errno
> values that the tlshd shim then negates.
> 
> Three internal callers in handshake_nl_accept_doit(), the
> net-exit drain, and a kunit test follow kernel convention and
> pass negative errnos -- -EIO, -ETIMEDOUT, -ETIMEDOUT. The
> implicit conversion to unsigned int turns -ETIMEDOUT into
> 0xFFFFFF92; the subsequent -status in tls_handshake_done()
> wraps back to 110, the consumer's switch falls through, and
> the xprt reports -EACCES on what should be -ETIMEDOUT or -EIO.
> 
> Fix the API rather than the call sites. The natural kernel
> convention is negative errno in, negative errno out. Change
> handshake_complete() and hp_done to take int status, drop the
> negation in tls_handshake_done(), and negate once in
> handshake_nl_done_doit() where status arrives from the wire
> as an unsigned netlink attribute. The three internal callers
> were already correct under that convention and need no change.
> 
> At the same wire boundary, clamp values above MAX_ERRNO to the
> default -EIO. A well-behaved tlshd never sends such a value,
> but negating an unconstrained u32 invokes signed-integer
> overflow when the high bit is set, which UBSAN would report.
> 
> Fixes: 3b3009ea8abb ("net/handshake: Create a NETLINK service for handling handshake requests")
> Signed-off-by: Chuck Lever <[email protected]>
> ---
>   net/handshake/handshake-test.c | 2 +-
>   net/handshake/handshake.h      | 4 ++--
>   net/handshake/netlink.c        | 9 +++++++--
>   net/handshake/request.c        | 2 +-
>   net/handshake/tlshd.c          | 4 ++--
>   5 files changed, 13 insertions(+), 8 deletions(-)
> 
Reviewed-by: Hannes Reinecke <[email protected]>

Cheers,

Hannex
-- 
Dr. Hannes Reinecke                  Kernel Storage Architect
[email protected]                                +49 911 74053 688
SUSE Software Solutions GmbH, Frankenstr. 146, 90461 Nürnberg
HRB 36809 (AG Nürnberg), GF: I. Totev, A. McDonald, W. Knoblich