diff mbox

[V2,02/10] kasan: MODULE_VADDR is not available on all archs

Message ID 1440577578-15813-3-git-send-email-aneesh.kumar@linux.vnet.ibm.com (mailing list archive)
State Not Applicable
Headers show

Commit Message

Aneesh Kumar K.V Aug. 26, 2015, 8:26 a.m. UTC
Conditionalize the check using #ifdef

Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
---
 mm/kasan/report.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

Comments

Andrey Ryabinin Aug. 26, 2015, 5 p.m. UTC | #1
2015-08-26 11:26 GMT+03:00 Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>:
> Conditionalize the check using #ifdef
>
> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
> ---
>  mm/kasan/report.c | 11 ++++++++---
>  1 file changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/mm/kasan/report.c b/mm/kasan/report.c
> index e07c94fbd0ac..71ce7548d914 100644
> --- a/mm/kasan/report.c
> +++ b/mm/kasan/report.c
> @@ -85,9 +85,14 @@ static void print_error_description(struct kasan_access_info *info)
>
>  static inline bool kernel_or_module_addr(const void *addr)
>  {
> -       return (addr >= (void *)_stext && addr < (void *)_end)
> -               || (addr >= (void *)MODULES_VADDR
> -                       && addr < (void *)MODULES_END);
> +       if (addr >= (void *)_stext && addr < (void *)_end)
> +               return true;
> +#if defined(CONFIG_MODULES) && defined(MODULES_VADDR)
> +       if (addr >= (void *)MODULES_VADDR
> +                       && addr < (void *)MODULES_END)
> +               return true;
> +#endif

I don't think that this is correct change.
On ppc64 modules are in VMALLOC, so you should check for this.
Yes, we don't handle VMALLOC now, but we will at some point.

So I think we should use is_module_address() here.
It will be slower, but we don't care about performance in error reporting route.
Aneesh Kumar K.V Aug. 30, 2015, 12:33 p.m. UTC | #2
Andrey Ryabinin <ryabinin.a.a@gmail.com> writes:

> 2015-08-26 11:26 GMT+03:00 Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>:
>> Conditionalize the check using #ifdef
>>
>> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
>> ---
>>  mm/kasan/report.c | 11 ++++++++---
>>  1 file changed, 8 insertions(+), 3 deletions(-)
>>
>> diff --git a/mm/kasan/report.c b/mm/kasan/report.c
>> index e07c94fbd0ac..71ce7548d914 100644
>> --- a/mm/kasan/report.c
>> +++ b/mm/kasan/report.c
>> @@ -85,9 +85,14 @@ static void print_error_description(struct kasan_access_info *info)
>>
>>  static inline bool kernel_or_module_addr(const void *addr)
>>  {
>> -       return (addr >= (void *)_stext && addr < (void *)_end)
>> -               || (addr >= (void *)MODULES_VADDR
>> -                       && addr < (void *)MODULES_END);
>> +       if (addr >= (void *)_stext && addr < (void *)_end)
>> +               return true;
>> +#if defined(CONFIG_MODULES) && defined(MODULES_VADDR)
>> +       if (addr >= (void *)MODULES_VADDR
>> +                       && addr < (void *)MODULES_END)
>> +               return true;
>> +#endif
>
> I don't think that this is correct change.
> On ppc64 modules are in VMALLOC, so you should check for this.
> Yes, we don't handle VMALLOC now, but we will at some point.
>
> So I think we should use is_module_address() here.
> It will be slower, but we don't care about performance in error reporting route.

Will fix in the next update.

-aneesh
diff mbox

Patch

diff --git a/mm/kasan/report.c b/mm/kasan/report.c
index e07c94fbd0ac..71ce7548d914 100644
--- a/mm/kasan/report.c
+++ b/mm/kasan/report.c
@@ -85,9 +85,14 @@  static void print_error_description(struct kasan_access_info *info)
 
 static inline bool kernel_or_module_addr(const void *addr)
 {
-	return (addr >= (void *)_stext && addr < (void *)_end)
-		|| (addr >= (void *)MODULES_VADDR
-			&& addr < (void *)MODULES_END);
+	if (addr >= (void *)_stext && addr < (void *)_end)
+		return true;
+#if defined(CONFIG_MODULES) && defined(MODULES_VADDR)
+	if (addr >= (void *)MODULES_VADDR
+			&& addr < (void *)MODULES_END)
+		return true;
+#endif
+	return false;
 }
 
 static inline bool init_task_stack_addr(const void *addr)