[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20150908011443.GB19776@bbox>
Date: Tue, 8 Sep 2015 10:14:43 +0900
From: Minchan Kim <minchan@...nel.org>
To: Sergey Senozhatsky <sergey.senozhatsky.work@...il.com>
Cc: Luis Henriques <luis.henriques@...onical.com>,
Nitin Gupta <ngupta@...are.org>, linux-kernel@...r.kernel.org,
Sergey Senozhatsky <sergey.senozhatsky@...il.com>
Subject: Re: [PATCH] zram: don't copy invalid compression algorithms
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 <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/
Powered by blists - more mailing lists