diff mbox

[1/1] arch/parisc: mm: fix uninitialized variable usage

Message ID 1379873866-29219-1-git-send-email-felipensp@gmail.com (mailing list archive)
State Awaiting Upstream
Headers show

Commit Message

Felipe Pena Sept. 22, 2013, 6:17 p.m. UTC
The FAULT_FLAG_WRITE flag has been set based on uninitialized variable

Signed-off-by: Felipe Pena <felipensp@gmail.com>
---
 arch/parisc/mm/fault.c |    5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

Comments

Johannes Weiner Sept. 22, 2013, 10:58 p.m. UTC | #1
Hello Felipe,

On Sun, Sep 22, 2013 at 03:17:46PM -0300, Felipe Pena wrote:
> The FAULT_FLAG_WRITE flag has been set based on uninitialized variable

Oops, you are right.

> Signed-off-by: Felipe Pena <felipensp@gmail.com>
> ---
>  arch/parisc/mm/fault.c |    5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/parisc/mm/fault.c b/arch/parisc/mm/fault.c
> index d10d27a..6b38026 100644
> --- a/arch/parisc/mm/fault.c
> +++ b/arch/parisc/mm/fault.c
> @@ -182,8 +182,6 @@ void do_page_fault(struct pt_regs *regs, unsigned long code,
>  
>  	if (user_mode(regs))
>  		flags |= FAULT_FLAG_USER;
> -	if (acc_type & VM_WRITE)
> -		flags |= FAULT_FLAG_WRITE;
>  retry:
>  	down_read(&mm->mmap_sem);
>  	vma = find_vma_prev(mm, address, &prev_vma);
> @@ -201,6 +199,9 @@ good_area:
>  	if ((vma->vm_flags & acc_type) != acc_type)
>  		goto bad_area;
>  
> +	if (acc_type & VM_WRITE)
> +		flags |= FAULT_FLAG_WRITE;

Can acc_type actually change between between the first round and a
retry?  Otherwise, it might make sense to pull this up and place it
next to the flag initialization instead of pulling one flag down.
--
To unsubscribe from this list: send the line "unsubscribe linux-parisc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Felipe Pena Sept. 23, 2013, 12:23 a.m. UTC | #2
Hello Johannes,

On Sun, Sep 22, 2013 at 7:58 PM, Johannes Weiner <hannes@cmpxchg.org> wrote:
> Hello Felipe,
>
> On Sun, Sep 22, 2013 at 03:17:46PM -0300, Felipe Pena wrote:
>> The FAULT_FLAG_WRITE flag has been set based on uninitialized variable
>
> Oops, you are right.
>
>> Signed-off-by: Felipe Pena <felipensp@gmail.com>
>> ---
>>  arch/parisc/mm/fault.c |    5 +++--
>>  1 file changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/arch/parisc/mm/fault.c b/arch/parisc/mm/fault.c
>> index d10d27a..6b38026 100644
>> --- a/arch/parisc/mm/fault.c
>> +++ b/arch/parisc/mm/fault.c
>> @@ -182,8 +182,6 @@ void do_page_fault(struct pt_regs *regs, unsigned long code,
>>
>>       if (user_mode(regs))
>>               flags |= FAULT_FLAG_USER;
>> -     if (acc_type & VM_WRITE)
>> -             flags |= FAULT_FLAG_WRITE;
>>  retry:
>>       down_read(&mm->mmap_sem);
>>       vma = find_vma_prev(mm, address, &prev_vma);
>> @@ -201,6 +199,9 @@ good_area:
>>       if ((vma->vm_flags & acc_type) != acc_type)
>>               goto bad_area;
>>
>> +     if (acc_type & VM_WRITE)
>> +             flags |= FAULT_FLAG_WRITE;
>
> Can acc_type actually change between between the first round and a
> retry?  Otherwise, it might make sense to pull this up and place it
> next to the flag initialization instead of pulling one flag down.

From what I've analyzed, this make sense. I'll make the suggested
changes and send another patch.

Thanks.
diff mbox

Patch

diff --git a/arch/parisc/mm/fault.c b/arch/parisc/mm/fault.c
index d10d27a..6b38026 100644
--- a/arch/parisc/mm/fault.c
+++ b/arch/parisc/mm/fault.c
@@ -182,8 +182,6 @@  void do_page_fault(struct pt_regs *regs, unsigned long code,
 
 	if (user_mode(regs))
 		flags |= FAULT_FLAG_USER;
-	if (acc_type & VM_WRITE)
-		flags |= FAULT_FLAG_WRITE;
 retry:
 	down_read(&mm->mmap_sem);
 	vma = find_vma_prev(mm, address, &prev_vma);
@@ -201,6 +199,9 @@  good_area:
 	if ((vma->vm_flags & acc_type) != acc_type)
 		goto bad_area;
 
+	if (acc_type & VM_WRITE)
+		flags |= FAULT_FLAG_WRITE;
+
 	/*
 	 * If for any reason at all we couldn't handle the fault, make
 	 * sure we exit gracefully rather than endlessly redo the