[RFC,ipsec-next,0/6] xfrm: start adding netlink extack support

Message

Sabrina Dubroca Nov. 11, 2021, 12:02 p.m. UTC

XFRM states and policies are complex objects, and there are many
reasons why the kernel can reject userspace's request to create
one. This series makes it a bit clearer by providing extended ack
messages for policy creation.

A few other operations that reuse the same helper functions are also
getting partial extack support in this series. More patches will
follow to complete extack support, in particular for state creation.

Note: The policy->share attribute seems to be entirely ignored in the
kernel outside of checking its value in verify_newpolicy_info(). There
are some (very) old comments in copy_from_user_policy and
copy_to_user_policy suggesting that it should at least be copied
to/from userspace. I don't know what it was intended for.

Sabrina Dubroca (6):
  xfrm: propagate extack to all netlink doit handlers
  xfrm: add extack support to verify_newpolicy_info
  xfrm: add extack to verify_policy_dir
  xfrm: add extack to validate_tmpl
  xfrm: add extack to verify_policy_type
  xfrm: add extack to verify_sec_ctx_len

 net/xfrm/xfrm_user.c | 163 +++++++++++++++++++++++++++----------------
 1 file changed, 103 insertions(+), 60 deletions(-)

Comments

Steffen Klassert Dec. 2, 2021, 2:08 p.m. UTC | #1

On Thu, Nov 11, 2021 at 01:02:41PM +0100, Sabrina Dubroca wrote:
> XFRM states and policies are complex objects, and there are many
> reasons why the kernel can reject userspace's request to create
> one. This series makes it a bit clearer by providing extended ack
> messages for policy creation.
> 
> A few other operations that reuse the same helper functions are also
> getting partial extack support in this series. More patches will
> follow to complete extack support, in particular for state creation.
> 
> Note: The policy->share attribute seems to be entirely ignored in the
> kernel outside of checking its value in verify_newpolicy_info(). There
> are some (very) old comments in copy_from_user_policy and
> copy_to_user_policy suggesting that it should at least be copied
> to/from userspace. I don't know what it was intended for.
> 
> Sabrina Dubroca (6):
>   xfrm: propagate extack to all netlink doit handlers
>   xfrm: add extack support to verify_newpolicy_info
>   xfrm: add extack to verify_policy_dir
>   xfrm: add extack to validate_tmpl
>   xfrm: add extack to verify_policy_type
>   xfrm: add extack to verify_sec_ctx_len
> 
>  net/xfrm/xfrm_user.c | 163 +++++++++++++++++++++++++++----------------
>  1 file changed, 103 insertions(+), 60 deletions(-)

Looks good to me, thanks!