Re: [PATCH] zram: don't copy invalid compression algorithms

[Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com>]

On (09/07/15 21:48), Luis Henriques wrote:
> Validate the new compression algorithm before copying it into the zram
> 'compressor' field, keeping the old one if it's invalid.
> 

NACK.

This is intentional. We haven't returned 'invalid compression algorithm'
error from comp_algorithm_store() historically, so someone's script can
simply ignore it. However, the script will fail to init the device and
user will be able to figure out the root cause, because zram will report
to syslog an actually requested alg name.

Example

[ 1669.473296] zram: Cannot initialise llzo compressing backend


	-ss

> The error path code is also slightly refactored.
> 
> Signed-off-by: Luis Henriques <luis.henriques@canonical.com>
> ---
>  drivers/block/zram/zram_drv.c | 13 ++++++++-----
>  1 file changed, 8 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
> index 9c01f5bfa33f..33551ec9e7f5 100644
> --- a/drivers/block/zram/zram_drv.c
> +++ b/drivers/block/zram/zram_drv.c
> @@ -367,10 +367,15 @@ static ssize_t comp_algorithm_store(struct device *dev,
>  
>  	down_write(&zram->init_lock);
>  	if (init_done(zram)) {
> -		up_write(&zram->init_lock);
>  		pr_info("Can't change algorithm for initialized device\n");
> -		return -EBUSY;
> +		len = -EBUSY;
> +		goto out;
> +	}
> +	if (!zcomp_available_algorithm(buf)) {
> +		len = -EINVAL;
> +		goto out;
>  	}
> +
>  	strlcpy(zram->compressor, buf, sizeof(zram->compressor));
>  
>  	/* ignore trailing newline */
> @@ -378,9 +383,7 @@ static ssize_t comp_algorithm_store(struct device *dev,
>  	if (sz > 0 && zram->compressor[sz - 1] == '\n')
>  		zram->compressor[sz - 1] = 0x00;
>  
> -	if (!zcomp_available_algorithm(zram->compressor))
> -		len = -EINVAL;
> -
> +out:
>  	up_write(&zram->init_lock);
>  	return len;
>  }
>