linux-kernel - Re: [arc-linux-dev] [PATCH] ARC: Fix array-out-of-bounds issue in tlb overlap handler

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] [day] [month] [year] [list]

Message-ID: <C2D7FE5348E1B147BCA15975FBA2307514E9B0@IN01WEMBXA.internal.synopsys.com>
Date:	Thu, 26 Sep 2013 14:26:22 +0000
From:	Vineet Gupta <Vineet.Gupta1@...opsys.com>
To:	"arc-linux-dev@...opsys.com" <arc-linux-dev@...opsys.com>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
CC:	Mischa Jonker <Mischa.Jonker@...opsys.com>
Subject: Re: [arc-linux-dev] [PATCH] ARC: Fix array-out-of-bounds issue in
 tlb overlap handler

On 09/26/2013 07:36 PM, Mischa Jonker wrote:
> Even after fixing the issue, GCC 4.8 still complains with a warning,
> but with the fix, I think it's a false positive. Therefore I have
> also added a #pragma to ignore the warning.
>
> Signed-off-by: Mischa Jonker <mjonker@...opsys.com>
> ---
>  arch/arc/mm/tlb.c |    9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arc/mm/tlb.c b/arch/arc/mm/tlb.c
> index 129af50..5af7c98 100644
> --- a/arch/arc/mm/tlb.c
> +++ b/arch/arc/mm/tlb.c
> @@ -635,13 +635,20 @@ void do_tlb_overlap_fault(unsigned long cause, unsigned long address,
>  			continue;
>  
>  		/* Scan the set for duplicate ways: needs a nested loop */
> -		for (way = 0; way < mmu->ways; way++) {
> +		for (way = 0; way < (mmu->ways - 1); way++) {
>  			if (!pd0[way])
>  				continue;

While the fix is correct, it's not fixing out-of-bound array access since that is
not what was happening. For the last way, the nested loop will not enter at all -
your fix prevents it from trying to do that - so please fix the changlog.

>  			for (n = way + 1; n < mmu->ways; n++) {
> +/*
> + * GCC 4.8 does not understand that way < (mmu->ways - 1), and as such
> + * n < mmu->ways. So, ignore the warning
> + */
> +#pragma GCC diagnostic push
> +#pragma GCC diagnostic ignored "-Warray-bounds"
>  				if ((pd0[way] & PAGE_MASK) ==
>  				    (pd0[n] & PAGE_MASK)) {
> +#pragma GCC diagnostic pop
>  
>  					if (dup_pd_verbose) {
>  						pr_info("Duplicate PD's @"

I'd prefer the following as it's less uglier.

-       unsigned int pd0[4], pd1[4];    /* assume max 4 ways */
        unsigned long flags, is_valid;
        struct cpuinfo_arc_mmu *mmu = &cpuinfo_arc700[smp_processor_id()].mmu;
+      unsigned int pd0[mmu->ways], pd1[mmu->ways];
 
-Vineet
--
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/