From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752032AbbIHBOE (ORCPT ); Mon, 7 Sep 2015 21:14:04 -0400 Received: from mail-pa0-f54.google.com ([209.85.220.54]:32865 "EHLO mail-pa0-f54.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751222AbbIHBOD (ORCPT ); Mon, 7 Sep 2015 21:14:03 -0400 Date: Tue, 8 Sep 2015 10:14:43 +0900 From: Minchan Kim To: Sergey Senozhatsky Cc: Luis Henriques , Nitin Gupta , linux-kernel@vger.kernel.org, Sergey Senozhatsky Subject: Re: [PATCH] zram: don't copy invalid compression algorithms Message-ID: <20150908011443.GB19776@bbox> References: <1441658910-10226-1-git-send-email-luis.henriques@canonical.com> <20150907235635.GA6896@swordfish> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20150907235635.GA6896@swordfish> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hey Sergey, On Tue, Sep 08, 2015 at 08:56:35AM +0900, Sergey Senozhatsky wrote: > 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 I don't understand your concern. To me, this patch makes sense to me. Could you explain your point clearly, again? Thanks. > > > -ss > > > The error path code is also slightly refactored. > > > > Signed-off-by: Luis Henriques > > --- > > 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; > > } > >