netdev - Re: [PATCH 05/11] leds: trigger: netdev: introduce validating requested mode

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, 1 May 2023 00:42:19 +0200
From:   Andrew Lunn <andrew@...n.ch>
To:     Christian Marangi <ansuelsmth@...il.com>
Cc:     Jonathan Corbet <corbet@....net>, Pavel Machek <pavel@....cz>,
        Lee Jones <lee@...nel.org>,
        Florian Fainelli <f.fainelli@...il.com>,
        Vladimir Oltean <olteanv@...il.com>,
        "David S. Miller" <davem@...emloft.net>,
        Eric Dumazet <edumazet@...gle.com>,
        Jakub Kicinski <kuba@...nel.org>,
        Paolo Abeni <pabeni@...hat.com>, linux-doc@...r.kernel.org,
        linux-kernel@...r.kernel.org, linux-leds@...r.kernel.org,
        netdev@...r.kernel.org
Subject: Re: [PATCH 05/11] leds: trigger: netdev: introduce validating
 requested mode

> @@ -168,7 +174,7 @@ static ssize_t netdev_led_attr_store(struct device *dev, const char *buf,
>  				     size_t size, enum led_trigger_netdev_modes attr)
>  {
>  	struct led_netdev_data *trigger_data = led_trigger_get_drvdata(dev);
> -	unsigned long state;
> +	unsigned long state, new_mode = trigger_data->mode;
>  	int ret;
>  	int bit;
>  
> @@ -186,12 +192,18 @@ static ssize_t netdev_led_attr_store(struct device *dev, const char *buf,
>  		return -EINVAL;
>  	}
>  
> -	cancel_delayed_work_sync(&trigger_data->work);
> -
>  	if (state)
> -		set_bit(bit, &trigger_data->mode);
> +		set_bit(bit, &new_mode);
>  	else
> -		clear_bit(bit, &trigger_data->mode);
> +		clear_bit(bit, &new_mode);
> +
> +	ret = validate_requested_mode(trigger_data, new_mode);
> +	if (ret)
> +		return ret;
> +
> +	cancel_delayed_work_sync(&trigger_data->work);
> +
> +	trigger_data->mode = new_mode;
>  
>  	set_baseline_state(trigger_data);

I think you need to hold the trigger_data lock here, otherwise there
are potential race conditions.

    Andrew