[LEDE-DEV] [PATCH 4/5 netifd] Prevent premature device free in interface_claim_device

[Felix Fietkau]

On 2016-08-11 15:53, Hans Dedecker wrote:
> interface_set_device_config can trigger a device free (for example
> if the device is here only present in a bridge), which renders dev
> invalid and leads to segfault. Add a lock to prevent this and
> clean-up the code for readability.
> 
> Signed-off-by: Gino Peeters <peeters.gino at gmail.com>
> Signed-off-by: Hans Dedecker <dedeckeh at gmail.com>
> ---
>  interface.c | 17 ++++++++++-------
>  1 file changed, 10 insertions(+), 7 deletions(-)
> 
> diff --git a/interface.c b/interface.c
> index 0b9893c..1a31f4a 100644
> --- a/interface.c
> +++ b/interface.c
> @@ -581,7 +581,6 @@ static void
>  interface_claim_device(struct interface *iface)
>  {
>  	struct interface *parent;
> -	struct device *dev = NULL;
>  
>  	if (iface->parent_iface.iface)
>  		interface_remove_user(&iface->parent_iface);
> @@ -592,15 +591,19 @@ interface_claim_device(struct interface *iface)
>  		interface_add_user(&iface->parent_iface, parent);
>  	} else if (iface->ifname &&
>  		!(iface->proto_handler->flags & PROTO_FLAG_NODEV)) {
> +		struct device *dev = NULL;
> +
>  		dev = device_get(iface->ifname, true);
> -		interface_set_device_config(iface, dev);
> -	} else {
> -		dev = iface->ext_dev.dev;
> +		if (dev) {
> +			device_lock();
> +			interface_set_device_config(iface, dev);
> +			interface_set_main_dev(iface, dev);
> +			device_unlock();
> +		}
> +	} else if (iface->ext_dev.dev) {
> +		interface_set_main_dev(iface, iface->ext_dev.dev);
>  	}
>  
> -	if (dev)
> -		interface_set_main_dev(iface, dev);
> -
I don't consider the 'cleanup' part an improvement over the old code, so
I made a different change that simply adds the device_lock/unlock calls
around the whole block.
Applied the rest of the patches in this series.

- Felix