linux-kernel - Re: [PATCH 9/9] ARM: kernel: add outer cache support for cacheinfo implementation

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:	Thu, 26 Jun 2014 14:02:58 +0100
From:	Sudeep Holla <sudeep.holla@....com>
To:	Russell King - ARM Linux <linux@....linux.org.uk>
CC:	Sudeep Holla <sudeep.holla@....com>,
	"linux-kernel@...r.kernel.org" <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" 
	<linux-arm-kernel@...ts.infradead.org>
Subject: Re: [PATCH 9/9] ARM: kernel: add outer cache support for cacheinfo
 implementation

Hi Russell,

On 25/06/14 23:37, Russell King - ARM Linux wrote:
> 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.
>

Understood, will get rid of it.

>> @@ -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.
>

Ah, will specify in l2c_init_data for individual implementations so that
fixups is possible if needed for get_info. Sorry for missing this.

Regards,
Sudeep

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