ping.
On 2019/3/7 10:22, Yue Haibing wrote:
> From: YueHaibing <yuehaib...@huawei.com>
>
> register_snap_client may return NULL, all the callers
> check it, but only print a warning. This will result in
> NULL pointer dereference in unregister_snap_client and other
> places.
>
> It has always been used like this since v2.6
>
> Reported-by: Dan Carpenter <dan.carpen...@oracle.com>
> Signed-off-by: YueHaibing <yuehaib...@huawei.com>
> ---
> v2: fix aarp_dl leak
> ---
> include/linux/atalk.h | 2 +-
> net/appletalk/aarp.c | 13 ++++++++++---
> net/appletalk/ddp.c | 20 ++++++++++++--------
> 3 files changed, 23 insertions(+), 12 deletions(-)
>
> diff --git a/include/linux/atalk.h b/include/linux/atalk.h
> index 5a90f28..0e5265a 100644
> --- a/include/linux/atalk.h
> +++ b/include/linux/atalk.h
> @@ -108,7 +108,7 @@ static __inline__ struct elapaarp *aarp_hdr(struct
> sk_buff *skb)
> #define AARP_RESOLVE_TIME (10 * HZ)
>
> extern struct datalink_proto *ddp_dl, *aarp_dl;
> -extern void aarp_proto_init(void);
> +extern int aarp_proto_init(void);
>
> /* Inter module exports */
>
> diff --git a/net/appletalk/aarp.c b/net/appletalk/aarp.c
> index 49a16ce..d11372d 100644
> --- a/net/appletalk/aarp.c
> +++ b/net/appletalk/aarp.c
> @@ -879,15 +879,22 @@ static struct notifier_block aarp_notifier = {
>
> static unsigned char aarp_snap_id[] = { 0x00, 0x00, 0x00, 0x80, 0xF3 };
>
> -void __init aarp_proto_init(void)
> +int __init aarp_proto_init(void)
> {
> + int rc;
> +
> aarp_dl = register_snap_client(aarp_snap_id, aarp_rcv);
> - if (!aarp_dl)
> + if (!aarp_dl) {
> printk(KERN_CRIT "Unable to register AARP with SNAP.\n");
> + return -ENOMEM;
> + }
> timer_setup(&aarp_timer, aarp_expire_timeout, 0);
> aarp_timer.expires = jiffies + sysctl_aarp_expiry_time;
> add_timer(&aarp_timer);
> - register_netdevice_notifier(&aarp_notifier);
> + rc = register_netdevice_notifier(&aarp_notifier);
> + if (rc)
> + unregister_snap_client(aarp_dl);
> + return rc;
> }
>
> /* Remove the AARP entries associated with a device. */
> diff --git a/net/appletalk/ddp.c b/net/appletalk/ddp.c
> index 795fbc6..709d254 100644
> --- a/net/appletalk/ddp.c
> +++ b/net/appletalk/ddp.c
> @@ -1904,9 +1904,6 @@ static unsigned char ddp_snap_id[] = { 0x08, 0x00,
> 0x07, 0x80, 0x9B };
> EXPORT_SYMBOL(atrtr_get_dev);
> EXPORT_SYMBOL(atalk_find_dev_addr);
>
> -static const char atalk_err_snap[] __initconst =
> - KERN_CRIT "Unable to register DDP with SNAP.\n";
> -
> /* Called by proto.c on kernel start up */
> static int __init atalk_init(void)
> {
> @@ -1921,17 +1918,22 @@ static int __init atalk_init(void)
> goto out_proto;
>
> ddp_dl = register_snap_client(ddp_snap_id, atalk_rcv);
> - if (!ddp_dl)
> - printk(atalk_err_snap);
> + if (!ddp_dl) {
> + pr_crit("Unable to register DDP with SNAP.\n");
> + goto out_sock;
> + }
>
> dev_add_pack(<alk_packet_type);
> dev_add_pack(&ppptalk_packet_type);
>
> rc = register_netdevice_notifier(&ddp_notifier);
> if (rc)
> - goto out_sock;
> + goto out_snap;
> +
> + rc = aarp_proto_init();
> + if (rc)
> + goto out_dev;
>
> - aarp_proto_init();
> rc = atalk_proc_init();
> if (rc)
> goto out_aarp;
> @@ -1945,11 +1947,13 @@ static int __init atalk_init(void)
> atalk_proc_exit();
> out_aarp:
> aarp_cleanup_module();
> +out_dev:
> unregister_netdevice_notifier(&ddp_notifier);
> -out_sock:
> +out_snap:
> dev_remove_pack(&ppptalk_packet_type);
> dev_remove_pack(<alk_packet_type);
> unregister_snap_client(ddp_dl);
> +out_sock:
> sock_unregister(PF_APPLETALK);
> out_proto:
> proto_unregister(&ddp_proto);
>
