diff mbox

[RFC,V1,4/8] kasan: Don't use kasan shadow pointer in generic functions

Message ID 1439793400-18147-5-git-send-email-aneesh.kumar@linux.vnet.ibm.com (mailing list archive)
State RFC
Headers show

Commit Message

Aneesh Kumar K.V Aug. 17, 2015, 6:36 a.m. UTC
We can't use generic functions like print_hex_dump to access kasan
shadow region. This require us to setup another kasan shadow region
for the address passed (kasan shadow address). Most architecture won't
be able to do that. Hence remove dumping kasan shadow region dump. If
we really want to do this we will have to have a kasan internal implemen
tation of print_hex_dump for which we will disable address sanitizer
operation.

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

Comments

Andrey Ryabinin Aug. 17, 2015, 11:36 a.m. UTC | #1
On 08/17/2015 09:36 AM, Aneesh Kumar K.V wrote:
> We can't use generic functions like print_hex_dump to access kasan
> shadow region. This require us to setup another kasan shadow region
> for the address passed (kasan shadow address). Most architecture won't
> be able to do that. Hence remove dumping kasan shadow region dump. If
> we really want to do this we will have to have a kasan internal implemen
> tation of print_hex_dump for which we will disable address sanitizer
> operation.
>

I didn't understand that.
Yes, you don't have shadow for shadow. But, for shadow addresses you
return return (void *)kasan_zero_page in kasan_mem_to_shadow(), so we
should be fine to access shadow in generic code.

And with kasan_tracks_vaddr(), this should work too.
Aneesh Kumar K.V Aug. 18, 2015, 5:29 a.m. UTC | #2
Andrey Ryabinin <ryabinin.a.a@gmail.com> writes:

> On 08/17/2015 09:36 AM, Aneesh Kumar K.V wrote:
>> We can't use generic functions like print_hex_dump to access kasan
>> shadow region. This require us to setup another kasan shadow region
>> for the address passed (kasan shadow address). Most architecture won't
>> be able to do that. Hence remove dumping kasan shadow region dump. If
>> we really want to do this we will have to have a kasan internal implemen
>> tation of print_hex_dump for which we will disable address sanitizer
>> operation.
>>
>
> I didn't understand that.
> Yes, you don't have shadow for shadow. But, for shadow addresses you
> return return (void *)kasan_zero_page in kasan_mem_to_shadow(), so we
> should be fine to access shadow in generic code.
>

But in general IMHO it is not correct to pass shadow address to generic
functions, because that requires arch to setup shadow for the shadow.
With one of the initial implementation of ppc64 support, I had page
table entries setup for vmalloc and vmemmap shadow and that is when I
hit the issue. We cannot expect arch to setup shadow regions like what is
expected here. If we really need to print the shadow memory content, we
could possibly make a copy of print_hex_dump in kasan_init.c . Let me
know whether you think printing shadow area content is needed.

-aneesh
Andrey Ryabinin Aug. 18, 2015, 9:12 a.m. UTC | #3
2015-08-18 8:29 GMT+03:00 Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>:
> Andrey Ryabinin <ryabinin.a.a@gmail.com> writes:
>
>> On 08/17/2015 09:36 AM, Aneesh Kumar K.V wrote:
>>> We can't use generic functions like print_hex_dump to access kasan
>>> shadow region. This require us to setup another kasan shadow region
>>> for the address passed (kasan shadow address). Most architecture won't
>>> be able to do that. Hence remove dumping kasan shadow region dump. If
>>> we really want to do this we will have to have a kasan internal implemen
>>> tation of print_hex_dump for which we will disable address sanitizer
>>> operation.
>>>
>>
>> I didn't understand that.
>> Yes, you don't have shadow for shadow. But, for shadow addresses you
>> return return (void *)kasan_zero_page in kasan_mem_to_shadow(), so we
>> should be fine to access shadow in generic code.
>>
>
> But in general IMHO it is not correct to pass shadow address to generic
> functions, because that requires arch to setup shadow for the shadow.

Yes, we have this shadow for shadow in x86_64/arm64.

> With one of the initial implementation of ppc64 support, I had page
> table entries setup for vmalloc and vmemmap shadow and that is when I
> hit the issue. We cannot expect arch to setup shadow regions like what is
> expected here. If we really need to print the shadow memory content, we
> could possibly make a copy of print_hex_dump in kasan_init.c . Let me
> know whether you think printing shadow area content is needed.
>

It was quite useful sometimes, so I think we should keep it.
But I agree with you, that it would be better to avoid accesses to shadow memory
in generic code.
Another way to deal with this would be to copy shadow content in buffer,
and then print_hex_dump() it.

> -aneesh
>
diff mbox

Patch

diff --git a/mm/kasan/report.c b/mm/kasan/report.c
index d19d01823a68..79fbc5d14bd2 100644
--- a/mm/kasan/report.c
+++ b/mm/kasan/report.c
@@ -170,12 +170,6 @@  static void print_shadow_for_address(const void *addr)
 		snprintf(buffer, sizeof(buffer),
 			(i == 0) ? ">%p: " : " %p: ", kaddr);
 
-		kasan_disable_current();
-		print_hex_dump(KERN_ERR, buffer,
-			DUMP_PREFIX_NONE, SHADOW_BYTES_PER_ROW, 1,
-			shadow_row, SHADOW_BYTES_PER_ROW, 0);
-		kasan_enable_current();
-
 		if (row_is_guilty(shadow_row, shadow))
 			pr_err("%*c\n",
 				shadow_pointer_offset(shadow_row, shadow),