Netdev List
 help / color / mirror / Atom feed
* [PATCH net-next 1/2] netlink: specs: handshake: type the remaining key serials s32
@ 2026-09-04 19:04 Jakub Kicinski
  2026-09-04 19:04 ` [PATCH net-next 2/2] netlink: specs: handshake: do not accept the handler-class sentinel Jakub Kicinski
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Jakub Kicinski @ 2026-09-04 19:04 UTC (permalink / raw)
  To: davem
  Cc: netdev, edumazet, pabeni, andrew+netdev, horms, Jakub Kicinski,
	cel, donald.hunter, kernel-tls-handshake

include/linux/key.h has "typedef int32_t key_serial_t" and commit
160f404495aa ("handshake: Fix sign of key_serial_t fields") converted
x509.cert and x509.privkey to s32, but accept.peer-identity,
accept.keyring and done.remote-auth were left as u32 - the same
quantity typed both ways inside one family, and a generated user space
struct with __s32 cert next to __u32 keyring. All three come from
key_serial_t storage in net/handshake/tlshd.c (treq->th_peerid[],
treq->th_keyring), and special keyrings are legitimately negative
(KEY_SPEC_PROCESS_KEYRING is -2).

NLA_U32 and NLA_S32 have the same length and no range check here, so
the only wire effect is how the value is printed.

Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
CC: cel@kernel.org
CC: donald.hunter@gmail.com
CC: kernel-tls-handshake@lists.linux.dev
---
 Documentation/netlink/specs/handshake.yaml | 6 +++---
 net/handshake/genl.c                       | 2 +-
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/Documentation/netlink/specs/handshake.yaml b/Documentation/netlink/specs/handshake.yaml
index ffec12b46759..9ab46da04c49 100644
--- a/Documentation/netlink/specs/handshake.yaml
+++ b/Documentation/netlink/specs/handshake.yaml
@@ -67,7 +67,7 @@ doc: Netlink protocol to request a transport layer security handshake.
         enum: auth
       -
         name: peer-identity
-        type: u32
+        type: s32
         multi-attr: true
       -
         name: certificate
@@ -79,7 +79,7 @@ doc: Netlink protocol to request a transport layer security handshake.
         type: string
       -
         name: keyring
-        type: u32
+        type: s32
   -
     name: done
     attributes:
@@ -93,7 +93,7 @@ doc: Netlink protocol to request a transport layer security handshake.
         type: s32
       -
         name: remote-auth
-        type: u32
+        type: s32
         multi-attr: true
 
 operations:
diff --git a/net/handshake/genl.c b/net/handshake/genl.c
index feac1ad063ee..58606c2a4600 100644
--- a/net/handshake/genl.c
+++ b/net/handshake/genl.c
@@ -21,7 +21,7 @@ static const struct nla_policy handshake_accept_nl_policy[HANDSHAKE_A_ACCEPT_HAN
 static const struct nla_policy handshake_done_nl_policy[HANDSHAKE_A_DONE_REMOTE_AUTH + 1] = {
 	[HANDSHAKE_A_DONE_STATUS] = NLA_POLICY_MAX(NLA_U32, MAX_ERRNO),
 	[HANDSHAKE_A_DONE_SOCKFD] = { .type = NLA_S32, },
-	[HANDSHAKE_A_DONE_REMOTE_AUTH] = { .type = NLA_U32, },
+	[HANDSHAKE_A_DONE_REMOTE_AUTH] = { .type = NLA_S32, },
 };
 
 /* Ops table for handshake */
-- 
2.55.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH net-next 2/2] netlink: specs: handshake: do not accept the handler-class sentinel
  2026-09-04 19:04 [PATCH net-next 1/2] netlink: specs: handshake: type the remaining key serials s32 Jakub Kicinski
@ 2026-09-04 19:04 ` Jakub Kicinski
  2026-09-06 20:19   ` Asbjørn Sloth Tønnesen
  2026-09-06 20:23 ` [PATCH net-next 1/2] netlink: specs: handshake: type the remaining key serials s32 Asbjørn Sloth Tønnesen
  2026-09-09 21:00 ` patchwork-bot+netdevbpf
  2 siblings, 1 reply; 6+ messages in thread
From: Jakub Kicinski @ 2026-09-04 19:04 UTC (permalink / raw)
  To: davem
  Cc: netdev, edumazet, pabeni, andrew+netdev, horms, Jakub Kicinski,
	cel, donald.hunter, ast, matttbe, kernel-tls-handshake

"max" is the exclusive upper bound of enum handshake_handler_class, not
a class a handler can ask for, but the spec lists it as a plain entry.
_init_checks() derives the policy limit from the highest entry, so the
generated policy came out as NLA_POLICY_MAX(NLA_U32, 2) and
handshake_nl_accept_doit() takes class 2 all the way into
handshake_req_next(), which walks hn_requests under hn_lock before
returning -EAGAIN instead of the -EINVAL a bad class deserves.

render-max is how YNL spells this, and unlike a hand written limit it
stays correct when a second handler class is added.

It does change what HANDSHAKE_HANDLER_CLASS_MAX means - the highest
valid class rather than one past it, as in every other Netlink/YNL
family - with the count left as __HANDSHAKE_HANDLER_CLASS_MAX.
The constant is uAPI, but I could not find any user space user,
this constant seems to have been added for kernel's benefit.

I think the risk of changing this is worth taking, having MAX
with different semantics than the rest of Netlink is very confusing.

Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
CC: cel@kernel.org
CC: donald.hunter@gmail.com
CC: ast@fiberby.net
CC: matttbe@kernel.org
CC: kernel-tls-handshake@lists.linux.dev
---
 Documentation/netlink/specs/handshake.yaml | 3 ++-
 include/uapi/linux/handshake.h             | 5 ++++-
 net/handshake/genl.c                       | 2 +-
 net/handshake/handshake-test.c             | 2 +-
 net/handshake/request.c                    | 2 +-
 5 files changed, 9 insertions(+), 5 deletions(-)

diff --git a/Documentation/netlink/specs/handshake.yaml b/Documentation/netlink/specs/handshake.yaml
index 9ab46da04c49..0498ada2dd97 100644
--- a/Documentation/netlink/specs/handshake.yaml
+++ b/Documentation/netlink/specs/handshake.yaml
@@ -22,7 +22,8 @@ doc: Netlink protocol to request a transport layer security handshake.
     type: enum
     name: handler-class
     value-start: 0
-    entries: [none, tlshd, max]
+    entries: [none, tlshd]
+    render-max: true
   -
     type: enum
     name: msg-type
diff --git a/include/uapi/linux/handshake.h b/include/uapi/linux/handshake.h
index d7e40f594888..f907aa082520 100644
--- a/include/uapi/linux/handshake.h
+++ b/include/uapi/linux/handshake.h
@@ -13,7 +13,10 @@
 enum handshake_handler_class {
 	HANDSHAKE_HANDLER_CLASS_NONE,
 	HANDSHAKE_HANDLER_CLASS_TLSHD,
-	HANDSHAKE_HANDLER_CLASS_MAX,
+
+	/* private: */
+	__HANDSHAKE_HANDLER_CLASS_MAX,
+	HANDSHAKE_HANDLER_CLASS_MAX = (__HANDSHAKE_HANDLER_CLASS_MAX - 1)
 };
 
 enum handshake_msg_type {
diff --git a/net/handshake/genl.c b/net/handshake/genl.c
index 58606c2a4600..26e3efec64ca 100644
--- a/net/handshake/genl.c
+++ b/net/handshake/genl.c
@@ -14,7 +14,7 @@
 
 /* HANDSHAKE_CMD_ACCEPT - do */
 static const struct nla_policy handshake_accept_nl_policy[HANDSHAKE_A_ACCEPT_HANDLER_CLASS + 1] = {
-	[HANDSHAKE_A_ACCEPT_HANDLER_CLASS] = NLA_POLICY_MAX(NLA_U32, 2),
+	[HANDSHAKE_A_ACCEPT_HANDLER_CLASS] = NLA_POLICY_MAX(NLA_U32, 1),
 };
 
 /* HANDSHAKE_CMD_DONE - do */
diff --git a/net/handshake/handshake-test.c b/net/handshake/handshake-test.c
index 3dd507470d5f..4b99acd9f9c7 100644
--- a/net/handshake/handshake-test.c
+++ b/net/handshake/handshake-test.c
@@ -42,7 +42,7 @@ static struct handshake_proto handshake_req_alloc_proto_2 = {
 };
 
 static struct handshake_proto handshake_req_alloc_proto_3 = {
-	.hp_handler_class	= HANDSHAKE_HANDLER_CLASS_MAX,
+	.hp_handler_class	= __HANDSHAKE_HANDLER_CLASS_MAX,
 };
 
 static struct handshake_proto handshake_req_alloc_proto_4 = {
diff --git a/net/handshake/request.c b/net/handshake/request.c
index cd30d54d0501..ff739ee81cb8 100644
--- a/net/handshake/request.c
+++ b/net/handshake/request.c
@@ -115,7 +115,7 @@ struct handshake_req *handshake_req_alloc(const struct handshake_proto *proto,
 		return NULL;
 	if (proto->hp_handler_class <= HANDSHAKE_HANDLER_CLASS_NONE)
 		return NULL;
-	if (proto->hp_handler_class >= HANDSHAKE_HANDLER_CLASS_MAX)
+	if (proto->hp_handler_class > HANDSHAKE_HANDLER_CLASS_MAX)
 		return NULL;
 	if (!proto->hp_accept || !proto->hp_done)
 		return NULL;
-- 
2.55.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH net-next 2/2] netlink: specs: handshake: do not accept the handler-class sentinel
  2026-09-04 19:04 ` [PATCH net-next 2/2] netlink: specs: handshake: do not accept the handler-class sentinel Jakub Kicinski
@ 2026-09-06 20:19   ` Asbjørn Sloth Tønnesen
  0 siblings, 0 replies; 6+ messages in thread
From: Asbjørn Sloth Tønnesen @ 2026-09-06 20:19 UTC (permalink / raw)
  To: Jakub Kicinski
  Cc: netdev, edumazet, pabeni, andrew+netdev, horms, cel,
	donald.hunter, matttbe, kernel-tls-handshake, davem

On 9/4/26 7:04 PM, Jakub Kicinski wrote:
> "max" is the exclusive upper bound of enum handshake_handler_class, not
> a class a handler can ask for, but the spec lists it as a plain entry.
> _init_checks() derives the policy limit from the highest entry, so the
> generated policy came out as NLA_POLICY_MAX(NLA_U32, 2) and
> handshake_nl_accept_doit() takes class 2 all the way into
> handshake_req_next(), which walks hn_requests under hn_lock before
> returning -EAGAIN instead of the -EINVAL a bad class deserves.
> 
> render-max is how YNL spells this, and unlike a hand written limit it
> stays correct when a second handler class is added.
> 
> It does change what HANDSHAKE_HANDLER_CLASS_MAX means - the highest
> valid class rather than one past it, as in every other Netlink/YNL
> family - with the count left as __HANDSHAKE_HANDLER_CLASS_MAX.
> The constant is uAPI, but I could not find any user space user,
> this constant seems to have been added for kernel's benefit.
> 
> I think the risk of changing this is worth taking, having MAX
> with different semantics than the rest of Netlink is very confusing.
> 
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>

LGTM.

Reviewed-by: Asbjørn Sloth Tønnesen <ast@fiberby.net>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH net-next 1/2] netlink: specs: handshake: type the remaining key serials s32
  2026-09-04 19:04 [PATCH net-next 1/2] netlink: specs: handshake: type the remaining key serials s32 Jakub Kicinski
  2026-09-04 19:04 ` [PATCH net-next 2/2] netlink: specs: handshake: do not accept the handler-class sentinel Jakub Kicinski
@ 2026-09-06 20:23 ` Asbjørn Sloth Tønnesen
  2026-09-06 23:35   ` Chuck Lever
  2026-09-09 21:00 ` patchwork-bot+netdevbpf
  2 siblings, 1 reply; 6+ messages in thread
From: Asbjørn Sloth Tønnesen @ 2026-09-06 20:23 UTC (permalink / raw)
  To: Jakub Kicinski
  Cc: netdev, edumazet, pabeni, andrew+netdev, horms, cel,
	donald.hunter, kernel-tls-handshake, davem

On 9/4/26 7:04 PM, Jakub Kicinski wrote:
> include/linux/key.h has "typedef int32_t key_serial_t" and commit
> 160f404495aa ("handshake: Fix sign of key_serial_t fields") converted
> x509.cert and x509.privkey to s32, but accept.peer-identity,
> accept.keyring and done.remote-auth were left as u32 - the same
> quantity typed both ways inside one family, and a generated user space
> struct with __s32 cert next to __u32 keyring. All three come from
> key_serial_t storage in net/handshake/tlshd.c (treq->th_peerid[],
> treq->th_keyring), and special keyrings are legitimately negative
> (KEY_SPEC_PROCESS_KEYRING is -2).
> 
> NLA_U32 and NLA_S32 have the same length and no range check here, so
> the only wire effect is how the value is printed.
> 
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>
> ---
> CC: cel@kernel.org
> CC: donald.hunter@gmail.com
> CC: kernel-tls-handshake@lists.linux.dev
> ---
>   Documentation/netlink/specs/handshake.yaml | 6 +++---
>   net/handshake/genl.c                       | 2 +-
>   2 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/Documentation/netlink/specs/handshake.yaml b/Documentation/netlink/specs/handshake.yaml
> index ffec12b46759..9ab46da04c49 100644
> --- a/Documentation/netlink/specs/handshake.yaml
> +++ b/Documentation/netlink/specs/handshake.yaml
> @@ -67,7 +67,7 @@ doc: Netlink protocol to request a transport layer security handshake.
>           enum: auth
>         -
>           name: peer-identity
> -        type: u32
> +        type: s32
>           multi-attr: true
>         -
>           name: certificate
> @@ -79,7 +79,7 @@ doc: Netlink protocol to request a transport layer security handshake.
>           type: string
>         -
>           name: keyring
> -        type: u32
> +        type: s32
>     -
>       name: done
>       attributes:
> @@ -93,7 +93,7 @@ doc: Netlink protocol to request a transport layer security handshake.
>           type: s32
>         -
>           name: remote-auth
> -        type: u32
> +        type: s32
>           multi-attr: true
>   
>   operations:
> diff --git a/net/handshake/genl.c b/net/handshake/genl.c
> index feac1ad063ee..58606c2a4600 100644
> --- a/net/handshake/genl.c
> +++ b/net/handshake/genl.c
> @@ -21,7 +21,7 @@ static const struct nla_policy handshake_accept_nl_policy[HANDSHAKE_A_ACCEPT_HAN
>   static const struct nla_policy handshake_done_nl_policy[HANDSHAKE_A_DONE_REMOTE_AUTH + 1] = {
>   	[HANDSHAKE_A_DONE_STATUS] = NLA_POLICY_MAX(NLA_U32, MAX_ERRNO),
>   	[HANDSHAKE_A_DONE_SOCKFD] = { .type = NLA_S32, },
> -	[HANDSHAKE_A_DONE_REMOTE_AUTH] = { .type = NLA_U32, },
> +	[HANDSHAKE_A_DONE_REMOTE_AUTH] = { .type = NLA_S32, },
>   };
>   
>   /* Ops table for handshake */

LGTM.

Reviewed-by: Asbjørn Sloth Tønnesen <ast@fiberby.net>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH net-next 1/2] netlink: specs: handshake: type the remaining key serials s32
  2026-09-06 20:23 ` [PATCH net-next 1/2] netlink: specs: handshake: type the remaining key serials s32 Asbjørn Sloth Tønnesen
@ 2026-09-06 23:35   ` Chuck Lever
  0 siblings, 0 replies; 6+ messages in thread
From: Chuck Lever @ 2026-09-06 23:35 UTC (permalink / raw)
  To: Asbjørn Sloth Tønnesen, Jakub Kicinski
  Cc: netdev, edumazet, Paolo Abeni, andrew+netdev, Simon Horman,
	Donald Hunter, kernel-tls-handshake, davem



On Sun, Sep 6, 2026, at 4:23 PM, Asbjørn Sloth Tønnesen wrote:
> On 9/4/26 7:04 PM, Jakub Kicinski wrote:
>> include/linux/key.h has "typedef int32_t key_serial_t" and commit
>> 160f404495aa ("handshake: Fix sign of key_serial_t fields") converted
>> x509.cert and x509.privkey to s32, but accept.peer-identity,
>> accept.keyring and done.remote-auth were left as u32 - the same
>> quantity typed both ways inside one family, and a generated user space
>> struct with __s32 cert next to __u32 keyring. All three come from
>> key_serial_t storage in net/handshake/tlshd.c (treq->th_peerid[],
>> treq->th_keyring), and special keyrings are legitimately negative
>> (KEY_SPEC_PROCESS_KEYRING is -2).
>> 
>> NLA_U32 and NLA_S32 have the same length and no range check here, so
>> the only wire effect is how the value is printed.
>> 
>> Signed-off-by: Jakub Kicinski <kuba@kernel.org>
>> ---
>> CC: cel@kernel.org
>> CC: donald.hunter@gmail.com
>> CC: kernel-tls-handshake@lists.linux.dev
>> ---
>>   Documentation/netlink/specs/handshake.yaml | 6 +++---
>>   net/handshake/genl.c                       | 2 +-
>>   2 files changed, 4 insertions(+), 4 deletions(-)
>> 
>> diff --git a/Documentation/netlink/specs/handshake.yaml b/Documentation/netlink/specs/handshake.yaml
>> index ffec12b46759..9ab46da04c49 100644
>> --- a/Documentation/netlink/specs/handshake.yaml
>> +++ b/Documentation/netlink/specs/handshake.yaml
>> @@ -67,7 +67,7 @@ doc: Netlink protocol to request a transport layer security handshake.
>>           enum: auth
>>         -
>>           name: peer-identity
>> -        type: u32
>> +        type: s32
>>           multi-attr: true
>>         -
>>           name: certificate
>> @@ -79,7 +79,7 @@ doc: Netlink protocol to request a transport layer security handshake.
>>           type: string
>>         -
>>           name: keyring
>> -        type: u32
>> +        type: s32
>>     -
>>       name: done
>>       attributes:
>> @@ -93,7 +93,7 @@ doc: Netlink protocol to request a transport layer security handshake.
>>           type: s32
>>         -
>>           name: remote-auth
>> -        type: u32
>> +        type: s32
>>           multi-attr: true
>>   
>>   operations:
>> diff --git a/net/handshake/genl.c b/net/handshake/genl.c
>> index feac1ad063ee..58606c2a4600 100644
>> --- a/net/handshake/genl.c
>> +++ b/net/handshake/genl.c
>> @@ -21,7 +21,7 @@ static const struct nla_policy handshake_accept_nl_policy[HANDSHAKE_A_ACCEPT_HAN
>>   static const struct nla_policy handshake_done_nl_policy[HANDSHAKE_A_DONE_REMOTE_AUTH + 1] = {
>>   	[HANDSHAKE_A_DONE_STATUS] = NLA_POLICY_MAX(NLA_U32, MAX_ERRNO),
>>   	[HANDSHAKE_A_DONE_SOCKFD] = { .type = NLA_S32, },
>> -	[HANDSHAKE_A_DONE_REMOTE_AUTH] = { .type = NLA_U32, },
>> +	[HANDSHAKE_A_DONE_REMOTE_AUTH] = { .type = NLA_S32, },
>>   };
>>   
>>   /* Ops table for handshake */
>
> LGTM.
>
> Reviewed-by: Asbjørn Sloth Tønnesen <ast@fiberby.net>

For some reason, this patch (and 1/2) came to my oracle.com address but
never appeared at my cel@kernel.org address.

For both: Acked-by: Chuck Lever <cel@kernel.org>


-- 
Chuck Lever (Come to NFS bake-a-thon! https://nfsv4bat.org)

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH net-next 1/2] netlink: specs: handshake: type the remaining key serials s32
  2026-09-04 19:04 [PATCH net-next 1/2] netlink: specs: handshake: type the remaining key serials s32 Jakub Kicinski
  2026-09-04 19:04 ` [PATCH net-next 2/2] netlink: specs: handshake: do not accept the handler-class sentinel Jakub Kicinski
  2026-09-06 20:23 ` [PATCH net-next 1/2] netlink: specs: handshake: type the remaining key serials s32 Asbjørn Sloth Tønnesen
@ 2026-09-09 21:00 ` patchwork-bot+netdevbpf
  2 siblings, 0 replies; 6+ messages in thread
From: patchwork-bot+netdevbpf @ 2026-09-09 21:00 UTC (permalink / raw)
  To: Jakub Kicinski
  Cc: davem, netdev, edumazet, pabeni, andrew+netdev, horms, cel,
	donald.hunter, kernel-tls-handshake

Hello:

This series was applied to netdev/net-next.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Fri,  4 Sep 2026 12:04:09 -0700 you wrote:
> include/linux/key.h has "typedef int32_t key_serial_t" and commit
> 160f404495aa ("handshake: Fix sign of key_serial_t fields") converted
> x509.cert and x509.privkey to s32, but accept.peer-identity,
> accept.keyring and done.remote-auth were left as u32 - the same
> quantity typed both ways inside one family, and a generated user space
> struct with __s32 cert next to __u32 keyring. All three come from
> key_serial_t storage in net/handshake/tlshd.c (treq->th_peerid[],
> treq->th_keyring), and special keyrings are legitimately negative
> (KEY_SPEC_PROCESS_KEYRING is -2).
> 
> [...]

Here is the summary with links:
  - [net-next,1/2] netlink: specs: handshake: type the remaining key serials s32
    https://git.kernel.org/netdev/net-next/c/f5134d734770
  - [net-next,2/2] netlink: specs: handshake: do not accept the handler-class sentinel
    https://git.kernel.org/netdev/net-next/c/fd0291fad9e7

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html



^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2026-09-09 21:01 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-09-04 19:04 [PATCH net-next 1/2] netlink: specs: handshake: type the remaining key serials s32 Jakub Kicinski
2026-09-04 19:04 ` [PATCH net-next 2/2] netlink: specs: handshake: do not accept the handler-class sentinel Jakub Kicinski
2026-09-06 20:19   ` Asbjørn Sloth Tønnesen
2026-09-06 20:23 ` [PATCH net-next 1/2] netlink: specs: handshake: type the remaining key serials s32 Asbjørn Sloth Tønnesen
2026-09-06 23:35   ` Chuck Lever
2026-09-09 21:00 ` patchwork-bot+netdevbpf

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox