| Message ID | 20230630012241.2822225-1-linma@zju.edu.cn (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [v3] net: xfrm: Amend XFRMA_SEC_CTX nla_policy structure | expand |
On Fri, Jun 30, 2023 at 09:22:41AM +0800, Lin Ma wrote: > According to all consumers code of attrs[XFRMA_SEC_CTX], like > > * verify_sec_ctx_len(), convert to xfrm_user_sec_ctx* > * xfrm_state_construct(), call security_xfrm_state_alloc whose prototype > is int security_xfrm_state_alloc(.., struct xfrm_user_sec_ctx *sec_ctx); > * copy_from_user_sec_ctx(), convert to xfrm_user_sec_ctx * > ... > > It seems that the expected parsing result for XFRMA_SEC_CTX should be > structure xfrm_user_sec_ctx, and the current xfrm_sec_ctx is confusing > and misleading (Luckily, they happen to have same size 8 bytes). > > This commit amend the policy structure to xfrm_user_sec_ctx to avoid > ambiguity. > > Fixes: cf5cb79f6946 ("[XFRM] netlink: Establish an attribute policy") > Signed-off-by: Lin Ma <linma@zju.edu.cn> > --- > V1 -> V2: also amend compat_policy XFRMA_SEC_CTX > V2 -> V3: fix typo > > net/xfrm/xfrm_compat.c | 2 +- > net/xfrm/xfrm_user.c | 3 ++- > 2 files changed, 3 insertions(+), 2 deletions(-) > > diff --git a/net/xfrm/xfrm_compat.c b/net/xfrm/xfrm_compat.c > index 8cbf45a8bcdc..655fe4ff8621 100644 > --- a/net/xfrm/xfrm_compat.c > +++ b/net/xfrm/xfrm_compat.c > @@ -108,7 +108,7 @@ static const struct nla_policy compat_policy[XFRMA_MAX+1] = { > [XFRMA_ALG_COMP] = { .len = sizeof(struct xfrm_algo) }, > [XFRMA_ENCAP] = { .len = sizeof(struct xfrm_encap_tmpl) }, > [XFRMA_TMPL] = { .len = sizeof(struct xfrm_user_tmpl) }, > - [XFRMA_SEC_CTX] = { .len = sizeof(struct xfrm_sec_ctx) }, > + [XFRMA_SEC_CTX] = { .len = sizeof(struct xfrm_user_sec_ctx) }, > [XFRMA_LTIME_VAL] = { .len = sizeof(struct xfrm_lifetime_cur) }, > [XFRMA_REPLAY_VAL] = { .len = sizeof(struct xfrm_replay_state) }, > [XFRMA_REPLAY_THRESH] = { .type = NLA_U32 }, > diff --git a/net/xfrm/xfrm_user.c b/net/xfrm/xfrm_user.c > index c34a2a06ca94..0c997a114d13 100644 > --- a/net/xfrm/xfrm_user.c > +++ b/net/xfrm/xfrm_user.c > @@ -3015,7 +3015,7 @@ const struct nla_policy xfrma_policy[XFRMA_MAX+1] = { > [XFRMA_ALG_COMP] = { .len = sizeof(struct xfrm_algo) }, > [XFRMA_ENCAP] = { .len = sizeof(struct xfrm_encap_tmpl) }, > [XFRMA_TMPL] = { .len = sizeof(struct xfrm_user_tmpl) }, > - [XFRMA_SEC_CTX] = { .len = sizeof(struct xfrm_sec_ctx) }, > + [XFRMA_SEC_CTX] = { .len = sizeof(struct xfrm_user_sec_ctx) }, > [XFRMA_LTIME_VAL] = { .len = sizeof(struct xfrm_lifetime_cur) }, > [XFRMA_REPLAY_VAL] = { .len = sizeof(struct xfrm_replay_state) }, > [XFRMA_REPLAY_THRESH] = { .type = NLA_U32 }, > @@ -3035,6 +3035,7 @@ const struct nla_policy xfrma_policy[XFRMA_MAX+1] = { > [XFRMA_SET_MARK] = { .type = NLA_U32 }, > [XFRMA_SET_MARK_MASK] = { .type = NLA_U32 }, > [XFRMA_IF_ID] = { .type = NLA_U32 }, > + [XFRMA_MTIMER_THRESH] = { .type = NLA_U32 }, The above line is apparently missing, but does it really belong to this patch? At least it is not mentioned in the commit message.
Hello Steffen, > > + [XFRMA_MTIMER_THRESH] = { .type = NLA_U32 }, > > The above line is apparently missing, but does it really belong to this > patch? At least it is not mentioned in the commit message. Nope, my bad, it is about another ongoing patch. T.T Sorry for the confusing patch, will resend it. Regards Lin
diff --git a/net/xfrm/xfrm_compat.c b/net/xfrm/xfrm_compat.c index 8cbf45a8bcdc..655fe4ff8621 100644 --- a/net/xfrm/xfrm_compat.c +++ b/net/xfrm/xfrm_compat.c @@ -108,7 +108,7 @@ static const struct nla_policy compat_policy[XFRMA_MAX+1] = { [XFRMA_ALG_COMP] = { .len = sizeof(struct xfrm_algo) }, [XFRMA_ENCAP] = { .len = sizeof(struct xfrm_encap_tmpl) }, [XFRMA_TMPL] = { .len = sizeof(struct xfrm_user_tmpl) }, - [XFRMA_SEC_CTX] = { .len = sizeof(struct xfrm_sec_ctx) }, + [XFRMA_SEC_CTX] = { .len = sizeof(struct xfrm_user_sec_ctx) }, [XFRMA_LTIME_VAL] = { .len = sizeof(struct xfrm_lifetime_cur) }, [XFRMA_REPLAY_VAL] = { .len = sizeof(struct xfrm_replay_state) }, [XFRMA_REPLAY_THRESH] = { .type = NLA_U32 }, diff --git a/net/xfrm/xfrm_user.c b/net/xfrm/xfrm_user.c index c34a2a06ca94..0c997a114d13 100644 --- a/net/xfrm/xfrm_user.c +++ b/net/xfrm/xfrm_user.c @@ -3015,7 +3015,7 @@ const struct nla_policy xfrma_policy[XFRMA_MAX+1] = { [XFRMA_ALG_COMP] = { .len = sizeof(struct xfrm_algo) }, [XFRMA_ENCAP] = { .len = sizeof(struct xfrm_encap_tmpl) }, [XFRMA_TMPL] = { .len = sizeof(struct xfrm_user_tmpl) }, - [XFRMA_SEC_CTX] = { .len = sizeof(struct xfrm_sec_ctx) }, + [XFRMA_SEC_CTX] = { .len = sizeof(struct xfrm_user_sec_ctx) }, [XFRMA_LTIME_VAL] = { .len = sizeof(struct xfrm_lifetime_cur) }, [XFRMA_REPLAY_VAL] = { .len = sizeof(struct xfrm_replay_state) }, [XFRMA_REPLAY_THRESH] = { .type = NLA_U32 }, @@ -3035,6 +3035,7 @@ const struct nla_policy xfrma_policy[XFRMA_MAX+1] = { [XFRMA_SET_MARK] = { .type = NLA_U32 }, [XFRMA_SET_MARK_MASK] = { .type = NLA_U32 }, [XFRMA_IF_ID] = { .type = NLA_U32 }, + [XFRMA_MTIMER_THRESH] = { .type = NLA_U32 }, }; EXPORT_SYMBOL_GPL(xfrma_policy);
According to all consumers code of attrs[XFRMA_SEC_CTX], like * verify_sec_ctx_len(), convert to xfrm_user_sec_ctx* * xfrm_state_construct(), call security_xfrm_state_alloc whose prototype is int security_xfrm_state_alloc(.., struct xfrm_user_sec_ctx *sec_ctx); * copy_from_user_sec_ctx(), convert to xfrm_user_sec_ctx * ... It seems that the expected parsing result for XFRMA_SEC_CTX should be structure xfrm_user_sec_ctx, and the current xfrm_sec_ctx is confusing and misleading (Luckily, they happen to have same size 8 bytes). This commit amend the policy structure to xfrm_user_sec_ctx to avoid ambiguity. Fixes: cf5cb79f6946 ("[XFRM] netlink: Establish an attribute policy") Signed-off-by: Lin Ma <linma@zju.edu.cn> --- V1 -> V2: also amend compat_policy XFRMA_SEC_CTX V2 -> V3: fix typo net/xfrm/xfrm_compat.c | 2 +- net/xfrm/xfrm_user.c | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-)