Re: [PATCH v2 net-next 2/5] l2tp: double-check l2specific_type provided by userspace

[Guillaume Nault <g.nault@alphalink.fr>]

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
>