linux-kernel - Re: [PATCH] zram: check comp algorithm availability earlier

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20150527055854.GE11609@blaptop>
Date:	Wed, 27 May 2015 14:58:54 +0900
From:	Minchan Kim <minchan@...nel.org>
To:	Sergey Senozhatsky <sergey.senozhatsky.work@...il.com>
Cc:	Sergey Senozhatsky <sergey.senozhatsky@...il.com>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Marcin Jabrzyk <m.jabrzyk@...sung.com>,
	Nitin Gupta <ngupta@...are.org>, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] zram: check comp algorithm availability earlier

On Wed, May 27, 2015 at 02:53:20PM +0900, Sergey Senozhatsky wrote:
> On (05/27/15 12:51), Minchan Kim wrote:
> [..]
> > > @@ -378,6 +378,12 @@ 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)) {
> > > +		pr_err("Error: unavailable compression algorithm: %s\n",
> > > +				zram->compressor);
> > > +		len = -EINVAL;
> > > +	}
> > > +
> > 
> > I'm not against this patch because it's better than old.
> > But let's think more about the pr_err part.
> > 
> > If user try to set wrong algo name, he can see EINVAL.
> > Isn't it enough?
> > 
> > I think every sane admin can think he passed wrong argument
> > if he sees -EINVAL.
> > So, I don't think we need to emit pr_err in here.
> > 
> 
> well, it's here simply to make failure investigation easier.
> one surely will know that supplied string was not recognized
> as a compression algorithm name, but what was it.. "$3 instead
> of $2... or, wait, did $i contain something wrong?". zram knew
> what was wrong.
> 
> /* and you asked to put this warn here in your previous email. */

Yes, Sorry about that. At that time, you put the warning in find_backend
and I didn't like it. Instead, I want to move it to in there.
But more thinking about it, I don't feel we need it.

> 
> 
> sure, can remove it.
> 
> 
> > The reason I am paranoid about that is that I really don't want
> > to argue with syslog info which is part of ABI or not in future.
> > If possible, I don't want to depend on pr_xxx.
> > 
> 
> just for the record...  I don't understand this part.

I meant if we remove the pr_err in future by some reason,
someone might shout

"No, it's ABI so if you guys removes it, it will break user interface's
semantic". Maybe he seems to depends on parse on dmesg.
That is not what I want.

> 
> 
> ok. I'll resend later today.
> 
> 	-ss

-- 
Kind regards,
Minchan Kim
--
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/