| Message ID | 119b4940ea365493fbf3c22f5f485ee800254fb3.1515940731.git.lorenzo.bianconi@redhat.com |
|---|---|
| State | Superseded, archived |
| Delegated to: | David Miller |
| Headers | show |
| Series | l2tp: set l2specific_len based on l2specific_type | expand |
On Sun, Jan 14, 2018 at 03:50:55PM +0100, Lorenzo Bianconi wrote: > Add sanity check on l2specific_type provided by userspace in > l2tp_nl_cmd_session_create() since just L2TP_L2SPECTYPE_DEFAULT and > L2TP_L2SPECTYPE_NONE are currently supported. > Moreover do not always initialize l2specific_type if userspace requests > a given l2-specific sublayer type > I don't understand your last sentence. l2specific_type is always initialised in your patch (or session creation is aborted). > Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@redhat.com> > --- > net/l2tp/l2tp_netlink.c | 11 +++++++++-- > 1 file changed, 9 insertions(+), 2 deletions(-) > > diff --git a/net/l2tp/l2tp_netlink.c b/net/l2tp/l2tp_netlink.c > index 48b5bf30ec50..711cf208f23a 100644 > --- a/net/l2tp/l2tp_netlink.c > +++ b/net/l2tp/l2tp_netlink.c > @@ -550,9 +550,16 @@ static int l2tp_nl_cmd_session_create(struct sk_buff *skb, struct genl_info *inf > if (info->attrs[L2TP_ATTR_DATA_SEQ]) > cfg.data_seq = nla_get_u8(info->attrs[L2TP_ATTR_DATA_SEQ]); > > - cfg.l2specific_type = L2TP_L2SPECTYPE_DEFAULT; > - if (info->attrs[L2TP_ATTR_L2SPEC_TYPE]) > + if (info->attrs[L2TP_ATTR_L2SPEC_TYPE]) { > cfg.l2specific_type = nla_get_u8(info->attrs[L2TP_ATTR_L2SPEC_TYPE]); > + if (cfg.l2specific_type != L2TP_L2SPECTYPE_DEFAULT && > + cfg.l2specific_type != L2TP_L2SPECTYPE_NONE) { > + ret = -EINVAL; > + goto out_tunnel; > + } > + } else { > + cfg.l2specific_type = L2TP_L2SPECTYPE_DEFAULT; > + } > > cfg.l2specific_len = 4; > if (info->attrs[L2TP_ATTR_L2SPEC_LEN]) > -- > 2.13.6 >
> On Sun, Jan 14, 2018 at 03:50:55PM +0100, Lorenzo Bianconi wrote: >> Add sanity check on l2specific_type provided by userspace in >> l2tp_nl_cmd_session_create() since just L2TP_L2SPECTYPE_DEFAULT and >> L2TP_L2SPECTYPE_NONE are currently supported. >> Moreover do not always initialize l2specific_type if userspace requests >> a given l2-specific sublayer type >> > I don't understand your last sentence. l2specific_type is always > initialised in your patch (or session creation is aborted). > I mean to explicitly set l2specific_type to L2TP_L2SPECTYPE_DEFAULT only if the userspace does not provide a value for it (I moved the 'default' initialization in the 'else' case) >> Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@redhat.com> >> --- >> net/l2tp/l2tp_netlink.c | 11 +++++++++-- >> 1 file changed, 9 insertions(+), 2 deletions(-) >> >> diff --git a/net/l2tp/l2tp_netlink.c b/net/l2tp/l2tp_netlink.c >> index 48b5bf30ec50..711cf208f23a 100644 >> --- a/net/l2tp/l2tp_netlink.c >> +++ b/net/l2tp/l2tp_netlink.c >> @@ -550,9 +550,16 @@ static int l2tp_nl_cmd_session_create(struct sk_buff *skb, struct genl_info *inf >> if (info->attrs[L2TP_ATTR_DATA_SEQ]) >> cfg.data_seq = nla_get_u8(info->attrs[L2TP_ATTR_DATA_SEQ]); >> >> - cfg.l2specific_type = L2TP_L2SPECTYPE_DEFAULT; >> - if (info->attrs[L2TP_ATTR_L2SPEC_TYPE]) >> + if (info->attrs[L2TP_ATTR_L2SPEC_TYPE]) { >> cfg.l2specific_type = nla_get_u8(info->attrs[L2TP_ATTR_L2SPEC_TYPE]); >> + if (cfg.l2specific_type != L2TP_L2SPECTYPE_DEFAULT && >> + cfg.l2specific_type != L2TP_L2SPECTYPE_NONE) { >> + ret = -EINVAL; >> + goto out_tunnel; >> + } >> + } else { >> + cfg.l2specific_type = L2TP_L2SPECTYPE_DEFAULT; >> + } >> >> cfg.l2specific_len = 4; >> if (info->attrs[L2TP_ATTR_L2SPEC_LEN]) >> -- >> 2.13.6 >>
On Mon, Jan 15, 2018 at 07:18:17PM +0100, Lorenzo Bianconi wrote: > > On Sun, Jan 14, 2018 at 03:50:55PM +0100, Lorenzo Bianconi wrote: > >> Add sanity check on l2specific_type provided by userspace in > >> l2tp_nl_cmd_session_create() since just L2TP_L2SPECTYPE_DEFAULT and > >> L2TP_L2SPECTYPE_NONE are currently supported. > >> Moreover do not always initialize l2specific_type if userspace requests > >> a given l2-specific sublayer type > >> > > I don't understand your last sentence. l2specific_type is always > > initialised in your patch (or session creation is aborted). > > > > I mean to explicitly set l2specific_type to L2TP_L2SPECTYPE_DEFAULT > only if the userspace does not provide a value for it (I moved the > 'default' initialization in the 'else' case) > Ok, I thought you were talking about a functional change.
diff --git a/net/l2tp/l2tp_netlink.c b/net/l2tp/l2tp_netlink.c index 48b5bf30ec50..711cf208f23a 100644 --- a/net/l2tp/l2tp_netlink.c +++ b/net/l2tp/l2tp_netlink.c @@ -550,9 +550,16 @@ static int l2tp_nl_cmd_session_create(struct sk_buff *skb, struct genl_info *inf if (info->attrs[L2TP_ATTR_DATA_SEQ]) cfg.data_seq = nla_get_u8(info->attrs[L2TP_ATTR_DATA_SEQ]); - cfg.l2specific_type = L2TP_L2SPECTYPE_DEFAULT; - if (info->attrs[L2TP_ATTR_L2SPEC_TYPE]) + if (info->attrs[L2TP_ATTR_L2SPEC_TYPE]) { cfg.l2specific_type = nla_get_u8(info->attrs[L2TP_ATTR_L2SPEC_TYPE]); + if (cfg.l2specific_type != L2TP_L2SPECTYPE_DEFAULT && + cfg.l2specific_type != L2TP_L2SPECTYPE_NONE) { + ret = -EINVAL; + goto out_tunnel; + } + } else { + cfg.l2specific_type = L2TP_L2SPECTYPE_DEFAULT; + } cfg.l2specific_len = 4; if (info->attrs[L2TP_ATTR_L2SPEC_LEN])
Add sanity check on l2specific_type provided by userspace in l2tp_nl_cmd_session_create() since just L2TP_L2SPECTYPE_DEFAULT and L2TP_L2SPECTYPE_NONE are currently supported. Moreover do not always initialize l2specific_type if userspace requests a given l2-specific sublayer type Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@redhat.com> --- net/l2tp/l2tp_netlink.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-)