netdev - Re: [PATCH v2 net-next 1/5] l2tp: fix switch default error handling in l2tp_nl_cmd_session

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Mon, 15 Jan 2018 19:33:58 +0100
From:   Guillaume Nault <g.nault@...halink.fr>
To:     Lorenzo Bianconi <lorenzo.bianconi@...hat.com>
Cc:     davem@...emloft.net, netdev@...r.kernel.org, jchapman@...alix.com
Subject: Re: [PATCH v2 net-next 1/5] l2tp: fix switch default error handling
 in l2tp_nl_cmd_session_create()

On Sun, Jan 14, 2018 at 03:50:54PM +0100, Lorenzo Bianconi wrote:
> Although this issue is harmless since that code path is protected by the
> check on l2tp_nl_cmd_ops[]/l2tp_nl_cmd_ops[]->session_create(), fix error
> handling for L2TP_PWTYPE_IP/default case in l2tp_nl_cmd_session_create()
> 
> Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@...hat.com>
> ---
>  net/l2tp/l2tp_netlink.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/net/l2tp/l2tp_netlink.c b/net/l2tp/l2tp_netlink.c
> index e1ca29f79821..48b5bf30ec50 100644
> --- a/net/l2tp/l2tp_netlink.c
> +++ b/net/l2tp/l2tp_netlink.c
> @@ -635,7 +635,7 @@ static int l2tp_nl_cmd_session_create(struct sk_buff *skb, struct genl_info *inf
>  	case L2TP_PWTYPE_IP:
>  	default:
>  		ret = -EPROTONOSUPPORT;
> -		break;
> +		goto out_tunnel;
>  	}
>
Not sure if this change is really worthwhile. As you noted, this is
unreachable code. And this switch should better be removed entirely:
it doesn't do anything for supported pseudo-wires.

And if PWTYPE_ETH_VLAN were to be implemented, it should perform its
configuration consistency checking in its own PW specific code, not in
the genl handler.

Anyway, removing this switch isn't the purpose of this series, so I
think you can drop this patch.