Re: [PATCH v6] PPTP: PPP over IPv4 (Point-to-Point Tunneling Protocol)

[Eric Dumazet <eric.dumazet@gmail.com>]

Le jeudi 19 août 2010 à 08:09 +0400, Dmitry Kozlov a écrit :
> This patch contains:
> 1. pptp driver
> 2. gre demultiplexer driver for demultiplexing gre packets with different gre version
>    so ip_gre and pptp may coexists
> 3. ip_gre modification
> 4. other stuff
> 

Almost done ;)

>  
> +GRE DEMULTIPLEXER DRIVER
> +M:	Dmitry Kozlov <xeb@xxxxxxx>
> +L:	netdev@xxxxxxxxxxxxxxx
> +S:	Maintained
> +F:	net/ipv4/gre.c
> +F:	include/net/gre.h
> +
> +PPTP DRIVER
> +M:	Dmitry Kozlov <xeb@xxxxxxx>
> +L:	netdev@xxxxxxxxxxxxxxx
> +S:	Maintained
> +F:	drivers/net/pptp.c
> +W:	http://sourceforge.net/projects/accel-pptp
> +


Please fill correct addresses, not xxxxxx

> +
> +static int __init pptp_init_module(void)
> +{
> +	int err = 0;
> +	pr_info("PPTP driver version " PPTP_DRIVER_VERSION "\n");
> +
> +	if (gre_add_protocol(&gre_pptp_protocol, GREPROTO_PPTP) < 0) {
> +		pr_err("PPTP: can't add protocol\n");
> +		goto out;
> +	}
> +
> +	err = proto_register(&pptp_sk_proto, 0);
> +	if (err) {
> +		pr_err("PPTP: can't register sk_proto\n");
> +		goto out_inet_del_protocol;
> +	}
> +
> +	err = register_pppox_proto(PX_PROTO_PPTP, &pppox_pptp_proto);
> +	if (err) {
> +		pr_err("PPTP: can't register pppox_proto\n");
> +		goto out_unregister_sk_proto;
> +	}
> +
+ callid_sock = (struct pppox_sock **)__vmalloc((MAX_CALLID + 1) * sizeof(void *), GFP_KERNEL | __GFP_ZERO, PAGE_KERNEL);

No need for the cast

Please split this to :

	callid_sock = __vmalloc((MAX_CALLID + 1) * sizeof(void *),
				GFP_KERNEL | __GFP_ZERO, PAGE_KERNEL);


> +	if (!callid_sock) {
> +		pr_err("PPTP: cann't allocate memory\n");
> +		goto out_unregister_pppox_proto;
> +	}
> +
> +out:
> +	return err;
> +out_unregister_pppox_proto:
> +	unregister_pppox_proto(PX_PROTO_PPTP);
> +out_unregister_sk_proto:
> +	proto_unregister(&pptp_sk_proto);
> +out_inet_del_protocol:
> +	gre_del_protocol(&gre_pptp_protocol, GREPROTO_PPTP);
> +	return err;
> +}


You should attempt the vmalloc() at the start of this function (before
proto_register() and register_pppox_proto()), or
maybe another cpu can enter your code and try to dereference null
pointer. Dont forget to vfree() in case of error unwinding.

Thanks