linux-kernel - Re: [PATCH] arch: metag: mm: hugetlbpage.c: Cleaning up inconsistent NULL checks

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, 28 May 2014 11:03:23 +0100
From:	James Hogan <james.hogan@...tec.com>
To:	Rickard Strandqvist <rickard_strandqvist@...ctrumdigital.se>
CC:	Andrew Morton <akpm@...ux-foundation.org>,
	Naoya Horiguchi <n-horiguchi@...jp.nec.com>,
	<linux-metag@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
	Paul <Paul.Burton@...tec.com>
Subject: Re: [PATCH] arch: metag: mm: hugetlbpage.c:  Cleaning up inconsistent
 NULL checks

Hi Rickard,

On 22/05/14 23:01, Rickard Strandqvist wrote:
> Cleaning up inconsistent NULL checks.
> There is otherwise a risk of a possible null pointer dereference.
> 
> Was largely found by using a static code analysis program called cppcheck.
> 
> Signed-off-by: Rickard Strandqvist <rickard_strandqvist@...ctrumdigital.se>
> ---
>  arch/metag/mm/hugetlbpage.c |    3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/metag/mm/hugetlbpage.c b/arch/metag/mm/hugetlbpage.c
> index 0424315..3f8d5cd 100644
> --- a/arch/metag/mm/hugetlbpage.c
> +++ b/arch/metag/mm/hugetlbpage.c
> @@ -188,7 +188,8 @@ new_search:
>  			}
>  		}
>  		after_huge = 0;
> -		addr = ALIGN_HUGEPT(vma->vm_end);
> +		if (vma)
> +			addr = ALIGN_HUGEPT(vma->vm_end);
>  	}
>  }
>  #endif
> 

I don't think this is a correct fix.

If !vma && !after_huge the first if block in the loop will match and the
function will return 0.
If !vma && after_huge the 3rd if block in the loop will match and the
function will return addr.

So removing the vma condition on the final if block in the loop would
probably make sense instead. Does that satisfy cppcheck?

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