netdev - Re: [PATCH] appletalk: Correctly handle return value of register_snap

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]

Message-Id: <20190306.101739.246616987743523058.davem@davemloft.net>
Date:   Wed, 06 Mar 2019 10:17:39 -0800 (PST)
From:   David Miller <davem@...emloft.net>
To:     yuehaibing@...wei.com
Cc:     joe@...ches.com, gregkh@...uxfoundation.org,
        linux-kernel@...r.kernel.org, netdev@...r.kernel.org
Subject: Re: [PATCH] appletalk: Correctly handle return value of
 register_snap_client

From: Yue Haibing <yuehaibing@...wei.com>
Date: Wed, 6 Mar 2019 15:27:40 +0800

> @@ -879,15 +879,17 @@ 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)
>  {
>  	aarp_dl = register_snap_client(aarp_snap_id, aarp_rcv);
> -	if (!aarp_dl)
> -		printk(KERN_CRIT "Unable to register AARP with SNAP.\n");
> +	if (!aarp_dl) {
> +		pr_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);
> +	return register_netdevice_notifier(&aarp_notifier);
>  }
>  

Your error paths in the caller of aarp_proto_init() do not handle the case
where aarp_dl is created by register_netdevice_notifier() fails.  You have
to unregister aarp_dl if it is non-NULL.

So your out_dev label path needs to handle aarp_dl if non-NULL.

Probably best is to jump to out_aarp: instead and make aarp_cleanup_module
able to handle partial cleanups.