linux-kernel - Re: [PATCH] kfifo: remove unnecessary type check

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:	Fri, 26 Oct 2012 08:51:06 +0200
From:	Stefani Seibold <stefani@...bold.net>
To:	Yuanhan Liu <yuanhan.liu@...ux.intel.com>
Cc:	Yuanhan Liu <yliu.null@...il.com>, linux-kernel@...r.kernel.org,
	Andrew Morton <akpm@...ux-foundation.org>,
	Wei Yang <weiyang@...ux.vnet.ibm.com>,
	Fengguang Wu <fengguang.wu@...el.com>,
	Stephen Rothwell <sfr@...b.auug.org.au>
Subject: Re: [PATCH] kfifo: remove unnecessary type check

Am Freitag, den 26.10.2012, 14:11 +0800 schrieb Yuanhan Liu:
> On Fri, Oct 26, 2012 at 07:38:31AM +0200, Stefani Seibold wrote:
> > Am Freitag, den 26.10.2012, 09:46 +0800 schrieb Yuanhan Liu:
> > > From: Yuanhan Liu <yuanhan.liu@...ux.intel.com>
> > > 
> > > Firstly, this kind of type check doesn't work. It does something similay
> > > like following:
> > > 	void * __dummy = NULL;
> > > 	__buf = __dummy;
> > > 
> > > __dummy is defined as void *. Thus it will not trigger warnings as
> > > expected.
> > > 
> > > Second, we don't need that kind of check. Since the prototype
> > > of __kfifo_out is:
> > > 	unsigned int __kfifo_out(struct __kfifo *fifo,  void *buf, unsigned int len)
> > > 
> > > buf is defined as void *, so we don't need do the type check. Remove it.
> > > 
> > > LINK: https://lkml.org/lkml/2012/10/25/386
> > > LINK: https://lkml.org/lkml/2012/10/25/584
> > > 
> > > Cc: Andrew Morton <akpm@...ux-foundation.org>
> > > Cc: Wei Yang <weiyang@...ux.vnet.ibm.com>
> > > Cc: Stefani Seibold <stefani@...bold.net>
> > > Cc: Fengguang Wu <fengguang.wu@...el.com>
> > > Cc: Stephen Rothwell <sfr@...b.auug.org.au>
> > > Signed-off-by: Yuanhan Liu <yuanhan.liu@...ux.intel.com>
> > > ---

> > 
> > Did you tried to compile the whole kernel including all the drivers with
> > your patch?
> 
> Hi Stefani,
> 
> I did a build test, it did't introduce any new compile errors and
> warnings. While, I haven't tried make allmodconfig then. Does this patch
> seems wrong to you?
> 
> Thanks,
> Yuanhan Liu

Hi Liu,

no the patch seems not wrong to me. But as you see with the previous
patch it is not easy to predict the side effects.

An allmodconfig together with C=2 is necessary to check if there is no
side effects which current users of the kfifo API. That is exactly what
i did again and again as i developed the kfifo API.

Also you have to build the kfifo samples, since this example code use
all features of the kfifo API.

And again: The kfifo is designed to do the many things at compile time,
not at runtime. If you modify the code, you have to check the compiler
assembler output for no degradation, especially in kfifo_put, kfifo_get,
kfifo_in, kfifo_out, __kfifo_in and __kfifo_out. Prevent runtime checks
if you can do it at compile time. This is the basic reasons to do it in
macros.

Greetings,
Stefani


--
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/