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

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:	Tue, 8 Sep 2015 08:56:35 +0900
From:	Sergey Senozhatsky <sergey.senozhatsky.work@...il.com>
To:	Luis Henriques <luis.henriques@...onical.com>
Cc:	Minchan Kim <minchan@...nel.org>, Nitin Gupta <ngupta@...are.org>,
	Sergey Senozhatsky <sergey.senozhatsky.work@...il.com>,
	linux-kernel@...r.kernel.org,
	Sergey Senozhatsky <sergey.senozhatsky@...il.com>
Subject: Re: [PATCH] zram: don't copy invalid compression algorithms

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@...onical.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;
>  }
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/