Return-Path: <linux-kernel-owner@vger.kernel.org>
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
	id S1754893AbbKMTJb (ORCPT <rfc822;w@1wt.eu>);
	Fri, 13 Nov 2015 14:09:31 -0500
Received: from mx1.redhat.com ([209.132.183.28]:49967 "EHLO mx1.redhat.com"
	rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP
	id S1754799AbbKMTJ3 (ORCPT <rfc822;linux-kernel@vger.kernel.org>);
	Fri, 13 Nov 2015 14:09:29 -0500
Subject: Re: [PATCHv2 2/2] arm64: Allow changing of attributes outside of
 modules
To: Xishi Qiu <qiuxishi@huawei.com>, Laura Abbott <labbott@fedoraproject.org>
References: <1447207057-11323-1-git-send-email-labbott@fedoraproject.org>
 <1447207057-11323-3-git-send-email-labbott@fedoraproject.org>
 <56459EED.4030109@huawei.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>,
        Will Deacon <will.deacon@arm.com>,
        Ard Biesheuvel <ard.biesheuvel@linaro.org>,
        Zhong Jiang <zhongjiang@huawei.com>,
        linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org,
        Kees Cook <keescook@chromium.org>, Mark Rutland <mark.rutland@arm.com>
From: Laura Abbott <labbott@redhat.com>
Message-ID: <56463567.7050801@redhat.com>
Date: Fri, 13 Nov 2015 11:09:27 -0800
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101
 Thunderbird/38.3.0
MIME-Version: 1.0
In-Reply-To: <56459EED.4030109@huawei.com>
Content-Type: text/plain; charset=windows-1252; format=flowed
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
Content-Length: 1787
Lines: 52

On 11/13/2015 12:27 AM, Xishi Qiu wrote:
> On 2015/11/11 9:57, Laura Abbott wrote:
<snip>
>>   }
>>
>>   static int change_memory_common(unsigned long addr, int numpages,
>> @@ -43,7 +180,6 @@ static int change_memory_common(unsigned long addr, int numpages,
>>   	unsigned long size = PAGE_SIZE*numpages;
>>   	unsigned long end = start + size;
>>   	int ret;
>> -	struct page_change_data data;
>>
>>   	if (!PAGE_ALIGNED(addr)) {
>>   		start &= PAGE_MASK;
>> @@ -51,17 +187,15 @@ static int change_memory_common(unsigned long addr, int numpages,
>>   		WARN_ON_ONCE(1);
>>   	}
>>
>> -	if (start < MODULES_VADDR || start >= MODULES_END)
>> +	if (start < PAGE_OFFSET && !is_vmalloc_addr((void *)start) &&
>> +		(start < MODULES_VADDR || start >= MODULES_END))
>
> How about abstracting "start < MODULES_VADDR || start >= MODULES_END" to a new function?
> e.g. is_module_addr(), however it is a little confusion with is_module_address().
>
>>   		return -EINVAL;
>>
>> -	if (end < MODULES_VADDR || end >= MODULES_END)
>> +	if (end < PAGE_OFFSET && !is_vmalloc_addr((void *)end) &&
>> +		(end < MODULES_VADDR || end >= MODULES_END))
>>   		return -EINVAL;
>>
>
> It will not filter this case, start in module range and end in vmalloc range, right?
> start and end should be both in one range.
>

The goal of this check was to prevent it from being used on userspace addresses. It's
very complicated and hard to understand. I'm going to give this some more thought about
a better way to do this check.

  
> Thanks,
> Xishi Qiu

Thanks,
Laura
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/