All of lore.kernel.org
 help / color / mirror / Atom feed
From: Andrey Ryabinin <aryabinin@virtuozzo.com>
To: Kyeongdon Kim <kyeongdon.kim@lge.com>
Cc: catalin.marinas@arm.com, will.deacon@arm.com, glider@google.com,
	dvyukov@google.com, Jason@zx2c4.com, robh@kernel.org,
	ard.biesheuvel@linaro.org, linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org, kasan-dev@googlegroups.com,
	linux-mm@kvack.org
Subject: Re: [PATCH v2] arm64: kasan: add interceptors for strcmp/strncmp functions
Date: Tue, 4 Sep 2018 19:24:33 +0300	[thread overview]
Message-ID: <12d4e435-e229-b4af-4286-a53fa77cb09d@virtuozzo.com> (raw)
In-Reply-To: <6954711c-6441-04df-62a9-a83c867e06ad@virtuozzo.com>



On 09/04/2018 01:10 PM, Andrey Ryabinin wrote:
> 
> 
> On 09/04/2018 09:59 AM, Kyeongdon Kim wrote:
> 
>>>> +#undef strncmp
>>>> +int strncmp(const char *cs, const char *ct, size_t len)
>>>> +{
>>>> + check_memory_region((unsigned long)cs, len, false, _RET_IP_);
>>>> + check_memory_region((unsigned long)ct, len, false, _RET_IP_);
>>>
>>> This will cause false positives. Both 'cs', and 'ct' could be less than len bytes.
>>>
>>> There is no need in these interceptors, just use the C implementations from lib/string.c
>>> like you did in your first patch.
>>> The only thing that was wrong in the first patch is that assembly implementations
>>> were compiled out instead of being declared week.
>>>
>> Well, at first I thought so..
>> I would remove diff code in /mm/kasan/kasan.c then use C implementations in lib/string.c
>> w/ assem implementations as weak :
>>
>> diff --git a/lib/string.c b/lib/string.c
>> index 2c0900a..a18b18f 100644
>> --- a/lib/string.c
>> +++ b/lib/string.c
>> @@ -312,7 +312,7 @@ size_t strlcat(char *dest, const char *src, size_t count)
>>  EXPORT_SYMBOL(strlcat);
>>  #endif
>>
>> -#ifndef __HAVE_ARCH_STRCMP
>> +#if (defined(CONFIG_ARM64) && defined(CONFIG_KASAN)) || !defined(__HAVE_ARCH_STRCMP)
> 
> No. What part of "like you did in your first patch" is unclear to you?

Just to be absolutely clear, I meant #ifdef out __HAVE_ARCH_* defines like it has been done in this patch
http://lkml.kernel.org/r/<1534233322-106271-1-git-send-email-kyeongdon.kim@lge.com>

WARNING: multiple messages have this Message-ID (diff)
From: Andrey Ryabinin <aryabinin@virtuozzo.com>
To: Kyeongdon Kim <kyeongdon.kim@lge.com>
Cc: catalin.marinas@arm.com, will.deacon@arm.com, glider@google.com,
	dvyukov@google.com, Jason@zx2c4.com, robh@kernel.org,
	ard.biesheuvel@linaro.org, linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org, kasan-dev@googlegroups.com,
	linux-mm@kvack.org
Subject: Re: [PATCH v2] arm64: kasan: add interceptors for strcmp/strncmp functions
Date: Tue, 4 Sep 2018 19:24:33 +0300	[thread overview]
Message-ID: <12d4e435-e229-b4af-4286-a53fa77cb09d@virtuozzo.com> (raw)
In-Reply-To: <6954711c-6441-04df-62a9-a83c867e06ad@virtuozzo.com>



On 09/04/2018 01:10 PM, Andrey Ryabinin wrote:
> 
> 
> On 09/04/2018 09:59 AM, Kyeongdon Kim wrote:
> 
>>>> +#undef strncmp
>>>> +int strncmp(const char *cs, const char *ct, size_t len)
>>>> +{
>>>> + check_memory_region((unsigned long)cs, len, false, _RET_IP_);
>>>> + check_memory_region((unsigned long)ct, len, false, _RET_IP_);
>>>
>>> This will cause false positives. Both 'cs', and 'ct' could be less than len bytes.
>>>
>>> There is no need in these interceptors, just use the C implementations from lib/string.c
>>> like you did in your first patch.
>>> The only thing that was wrong in the first patch is that assembly implementations
>>> were compiled out instead of being declared week.
>>>
>> Well, at first I thought so..
>> I would remove diff code in /mm/kasan/kasan.c then use C implementations in lib/string.c
>> w/ assem implementations as weak :
>>
>> diff --git a/lib/string.c b/lib/string.c
>> index 2c0900a..a18b18f 100644
>> --- a/lib/string.c
>> +++ b/lib/string.c
>> @@ -312,7 +312,7 @@ size_t strlcat(char *dest, const char *src, size_t count)
>> A EXPORT_SYMBOL(strlcat);
>> A #endif
>>
>> -#ifndef __HAVE_ARCH_STRCMP
>> +#if (defined(CONFIG_ARM64) && defined(CONFIG_KASAN)) || !defined(__HAVE_ARCH_STRCMP)
> 
> No. What part of "like you did in your first patch" is unclear to you?

Just to be absolutely clear, I meant #ifdef out __HAVE_ARCH_* defines like it has been done in this patch
http://lkml.kernel.org/r/<1534233322-106271-1-git-send-email-kyeongdon.kim@lge.com>

WARNING: multiple messages have this Message-ID (diff)
From: aryabinin@virtuozzo.com (Andrey Ryabinin)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH v2] arm64: kasan: add interceptors for strcmp/strncmp functions
Date: Tue, 4 Sep 2018 19:24:33 +0300	[thread overview]
Message-ID: <12d4e435-e229-b4af-4286-a53fa77cb09d@virtuozzo.com> (raw)
In-Reply-To: <6954711c-6441-04df-62a9-a83c867e06ad@virtuozzo.com>



On 09/04/2018 01:10 PM, Andrey Ryabinin wrote:
> 
> 
> On 09/04/2018 09:59 AM, Kyeongdon Kim wrote:
> 
>>>> +#undef strncmp
>>>> +int strncmp(const char *cs, const char *ct, size_t len)
>>>> +{
>>>> + check_memory_region((unsigned long)cs, len, false, _RET_IP_);
>>>> + check_memory_region((unsigned long)ct, len, false, _RET_IP_);
>>>
>>> This will cause false positives. Both 'cs', and 'ct' could be less than len bytes.
>>>
>>> There is no need in these interceptors, just use the C implementations from lib/string.c
>>> like you did in your first patch.
>>> The only thing that was wrong in the first patch is that assembly implementations
>>> were compiled out instead of being declared week.
>>>
>> Well, at first I thought so..
>> I would remove diff code in /mm/kasan/kasan.c then use C implementations in lib/string.c
>> w/ assem implementations as weak :
>>
>> diff --git a/lib/string.c b/lib/string.c
>> index 2c0900a..a18b18f 100644
>> --- a/lib/string.c
>> +++ b/lib/string.c
>> @@ -312,7 +312,7 @@ size_t strlcat(char *dest, const char *src, size_t count)
>> ?EXPORT_SYMBOL(strlcat);
>> ?#endif
>>
>> -#ifndef __HAVE_ARCH_STRCMP
>> +#if (defined(CONFIG_ARM64) && defined(CONFIG_KASAN)) || !defined(__HAVE_ARCH_STRCMP)
> 
> No. What part of "like you did in your first patch" is unclear to you?

Just to be absolutely clear, I meant #ifdef out __HAVE_ARCH_* defines like it has been done in this patch
http://lkml.kernel.org/r/<1534233322-106271-1-git-send-email-kyeongdon.kim@lge.com>

  reply	other threads:[~2018-09-04 16:24 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-08-23  8:56 [PATCH v2] arm64: kasan: add interceptors for strcmp/strncmp functions Kyeongdon Kim
2018-08-23  8:56 ` Kyeongdon Kim
2018-09-03  9:02 ` Kyeongdon Kim
2018-09-03  9:02   ` Kyeongdon Kim
2018-09-03  9:02   ` Kyeongdon Kim
2018-09-03  9:13   ` Dmitry Vyukov
2018-09-03  9:13     ` Dmitry Vyukov
2018-09-04  6:29     ` Kyeongdon Kim
2018-09-04  6:29       ` Kyeongdon Kim
2018-09-04  6:29       ` Kyeongdon Kim
2018-09-03  9:40 ` Andrey Ryabinin
2018-09-03  9:40   ` Andrey Ryabinin
2018-09-04  6:59   ` Kyeongdon Kim
2018-09-04  6:59     ` Kyeongdon Kim
2018-09-04  6:59     ` Kyeongdon Kim
2018-09-04 10:10     ` Andrey Ryabinin
2018-09-04 10:10       ` Andrey Ryabinin
2018-09-04 10:10       ` Andrey Ryabinin
2018-09-04 16:24       ` Andrey Ryabinin [this message]
2018-09-04 16:24         ` Andrey Ryabinin
2018-09-04 16:24         ` Andrey Ryabinin
2018-09-05  7:44         ` Kyeongdon Kim
2018-09-05  7:44           ` Kyeongdon Kim
2018-09-05  7:44           ` Kyeongdon Kim
2018-09-06 17:06           ` Andrey Ryabinin
2018-09-06 17:06             ` Andrey Ryabinin
2018-09-06 17:06             ` Andrey Ryabinin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=12d4e435-e229-b4af-4286-a53fa77cb09d@virtuozzo.com \
    --to=aryabinin@virtuozzo.com \
    --cc=Jason@zx2c4.com \
    --cc=ard.biesheuvel@linaro.org \
    --cc=catalin.marinas@arm.com \
    --cc=dvyukov@google.com \
    --cc=glider@google.com \
    --cc=kasan-dev@googlegroups.com \
    --cc=kyeongdon.kim@lge.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=robh@kernel.org \
    --cc=will.deacon@arm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.