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:	Wed, 25 Jun 2014 23:37:49 +0100
From:	Russell King - ARM Linux <linux@....linux.org.uk>
To:	Sudeep Holla <sudeep.holla@....com>
Cc:	linux-kernel@...r.kernel.org,
	Heiko Carstens <heiko.carstens@...ibm.com>,
	Lorenzo Pieralisi <Lorenzo.Pieralisi@....com>,
	Will Deacon <will.deacon@....com>,
	linux-arm-kernel@...ts.infradead.org
Subject: Re: [PATCH 9/9] ARM: kernel: add outer cache support for cacheinfo
	implementation

On Wed, Jun 25, 2014 at 06:30:44PM +0100, Sudeep Holla wrote:
> diff --git a/arch/arm/mm/cache-l2x0.c b/arch/arm/mm/cache-l2x0.c
> index efc5cab..30ca151 100644
> --- a/arch/arm/mm/cache-l2x0.c
> +++ b/arch/arm/mm/cache-l2x0.c
> @@ -105,6 +105,15 @@ static inline void l2c_unlock(void __iomem *base, unsigned num)
>  	}
>  }
>  
> +static void l2x0_getinfo(struct outer_cache_info *info)
> +{
> +	if (!info)
> +		return;

Pointless NULL test.  If someone passes NULL to this function (which
you never do in this file) then we want to know about it because _that_
is a kernel bug - it is invalid to pass NULL.  Hence the kernel should
oops.

Please, don't go around adding stupid NULL tests for conditions which
should _never_ happen, instead, rely on the kernel to oops if these
invalid conditions occur.  That's why we produce a backtrace from such
events, to allow invalid conditions to be debugged and fixed.

Having stuff silently ignore in this way does not detect these bugs so
they go by unnoticed.

Take a moment to read some of the fs/ or kernel/ code, and you'll find
a lack of NULL checks in there.  That's what gives that code performance,
because it's not spending its time doing loads of useless NULL checks.

> @@ -894,6 +903,7 @@ static void __init __l2c_init(const struct l2c_init_data *data,
>  		data->enable(l2x0_base, aux, data->num_lock);
>  
>  	outer_cache = fns;
> +	outer_cache.get_info = l2x0_getinfo;

NAK.  Think about it.

-- 
FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly
improving, and getting towards what was expected from it.
--
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