Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1391994 > unrolled thread
| Started by | chengang@emindsoft.com.cn |
|---|---|
| First post | 2016-05-02 08:00 +0200 |
| Last post | 2016-05-02 14:40 +0200 |
| Articles | 5 — 3 participants |
Back to article view | Back to linux.kernel
[PATCH] include/linux/kasan.h: Notice about 0 for kasan_[dis/en]able_current() chengang@emindsoft.com.cn - 2016-05-02 08:00 +0200
Re: [PATCH] include/linux/kasan.h: Notice about 0 for kasan_[dis/en]able_current() Alexander Potapenko <glider@google.com> - 2016-05-02 12:50 +0200
Re: [PATCH] include/linux/kasan.h: Notice about 0 for kasan_[dis/en]able_current() Chen Gang <chengang@emindsoft.com.cn> - 2016-05-02 13:20 +0200
Re: [PATCH] include/linux/kasan.h: Notice about 0 for kasan_[dis/en]able_current() Alexander Potapenko <glider@google.com> - 2016-05-02 13:30 +0200
Re: [PATCH] include/linux/kasan.h: Notice about 0 for kasan_[dis/en]able_current() Chen Gang <chengang@emindsoft.com.cn> - 2016-05-02 14:40 +0200
| From | chengang@emindsoft.com.cn |
|---|---|
| Date | 2016-05-02 08:00 +0200 |
| Subject | [PATCH] include/linux/kasan.h: Notice about 0 for kasan_[dis/en]able_current() |
| Message-ID | <ruf3I-6ho-5@gated-at.bofh.it> |
From: Chen Gang <chengang@emindsoft.com.cn>
According to their comments and the kasan_depth's initialization, if
kasan_depth is zero, it means disable. So kasan_depth need consider
about the 0 overflow.
Also remove useless comments for dummy kasan_slab_free().
Signed-off-by: Chen Gang <gang.chen.5i5j@gmail.com>
---
include/linux/kasan.h | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/include/linux/kasan.h b/include/linux/kasan.h
index 645c280..37fab04 100644
--- a/include/linux/kasan.h
+++ b/include/linux/kasan.h
@@ -32,13 +32,15 @@ static inline void *kasan_mem_to_shadow(const void *addr)
/* Enable reporting bugs after kasan_disable_current() */
static inline void kasan_enable_current(void)
{
- current->kasan_depth++;
+ if (current->kasan_depth + 1)
+ current->kasan_depth++;
}
/* Disable reporting bugs for current task */
static inline void kasan_disable_current(void)
{
- current->kasan_depth--;
+ if (current->kasan_depth)
+ current->kasan_depth--;
}
void kasan_unpoison_shadow(const void *address, size_t size);
@@ -113,8 +115,6 @@ static inline void kasan_krealloc(const void *object, size_t new_size,
static inline void kasan_slab_alloc(struct kmem_cache *s, void *object,
gfp_t flags) {}
-/* kasan_slab_free() returns true if the object has been put into quarantine.
- */
static inline bool kasan_slab_free(struct kmem_cache *s, void *object)
{
return false;
--
1.9.3
[toc] | [next] | [standalone]
| From | Alexander Potapenko <glider@google.com> |
|---|---|
| Date | 2016-05-02 12:50 +0200 |
| Message-ID | <rujAm-1I4-21@gated-at.bofh.it> |
| In reply to | #1391994 |
On Mon, May 2, 2016 at 7:35 AM, <chengang@emindsoft.com.cn> wrote:
> From: Chen Gang <chengang@emindsoft.com.cn>
>
> According to their comments and the kasan_depth's initialization, if
> kasan_depth is zero, it means disable. So kasan_depth need consider
> about the 0 overflow.
>
> Also remove useless comments for dummy kasan_slab_free().
>
> Signed-off-by: Chen Gang <gang.chen.5i5j@gmail.com>
> ---
> include/linux/kasan.h | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/include/linux/kasan.h b/include/linux/kasan.h
> index 645c280..37fab04 100644
> --- a/include/linux/kasan.h
> +++ b/include/linux/kasan.h
> @@ -32,13 +32,15 @@ static inline void *kasan_mem_to_shadow(const void *addr)
> /* Enable reporting bugs after kasan_disable_current() */
> static inline void kasan_enable_current(void)
> {
> - current->kasan_depth++;
> + if (current->kasan_depth + 1)
> + current->kasan_depth++;
> }
>
> /* Disable reporting bugs for current task */
> static inline void kasan_disable_current(void)
> {
> - current->kasan_depth--;
> + if (current->kasan_depth)
> + current->kasan_depth--;
> }
>
> void kasan_unpoison_shadow(const void *address, size_t size);
> @@ -113,8 +115,6 @@ static inline void kasan_krealloc(const void *object, size_t new_size,
>
> static inline void kasan_slab_alloc(struct kmem_cache *s, void *object,
> gfp_t flags) {}
> -/* kasan_slab_free() returns true if the object has been put into quarantine.
> - */
> static inline bool kasan_slab_free(struct kmem_cache *s, void *object)
> {
> return false;
> --
> 1.9.3
>
Acked-by: Alexander Potapenko <glider@google.com>
--
Alexander Potapenko
Software Engineer
Google Germany GmbH
Erika-Mann-Straße, 33
80636 München
Geschäftsführer: Matthew Scott Sucherman, Paul Terence Manicle
Registergericht und -nummer: Hamburg, HRB 86891
Sitz der Gesellschaft: Hamburg
[toc] | [prev] | [next] | [standalone]
| From | Chen Gang <chengang@emindsoft.com.cn> |
|---|---|
| Date | 2016-05-02 13:20 +0200 |
| Message-ID | <ruk3o-2eU-15@gated-at.bofh.it> |
| In reply to | #1392119 |
On 5/2/16 18:49, Alexander Potapenko wrote: > On Mon, May 2, 2016 at 7:35 AM, <chengang@emindsoft.com.cn> wrote: >> >> According to their comments and the kasan_depth's initialization, if >> kasan_depth is zero, it means disable. So kasan_depth need consider >> about the 0 overflow. >> >> Also remove useless comments for dummy kasan_slab_free(). >> >> Signed-off-by: Chen Gang <gang.chen.5i5j@gmail.com> > > Acked-by: Alexander Potapenko <glider@google.com> > OK, thanks. Another patch thread is also related with this patch thread, please help check. And sorry, originally, I did not let the 2 patches in one patches set. Thanks. -- Chen Gang (陈刚) Managing Natural Environments is the Duty of Human Beings.
[toc] | [prev] | [next] | [standalone]
| From | Alexander Potapenko <glider@google.com> |
|---|---|
| Date | 2016-05-02 13:30 +0200 |
| Message-ID | <rukd4-2kb-13@gated-at.bofh.it> |
| In reply to | #1392138 |
On Mon, May 2, 2016 at 1:20 PM, Chen Gang <chengang@emindsoft.com.cn> wrote: > On 5/2/16 18:49, Alexander Potapenko wrote: >> On Mon, May 2, 2016 at 7:35 AM, <chengang@emindsoft.com.cn> wrote: >>> >>> According to their comments and the kasan_depth's initialization, if >>> kasan_depth is zero, it means disable. So kasan_depth need consider >>> about the 0 overflow. >>> >>> Also remove useless comments for dummy kasan_slab_free(). >>> >>> Signed-off-by: Chen Gang <gang.chen.5i5j@gmail.com> >> >> Acked-by: Alexander Potapenko <glider@google.com> Nacked-by: Alexander Potapenko <glider@google.com> >> > > OK, thanks. Well, on a second thought I take that back, there still might be problems. I haven't noticed the other CL, and was too hasty reviewing this one. As kasan_disable_current() and kasan_enable_current() always go together, we need to prevent nested calls to them from breaking everything. If we ignore some calls to kasan_disable_current() to prevent overflows, the pairing calls to kasan_enable_current() will bring |current->kasan_depth| to an invalid state. E.g. if I'm understanding your idea correctly, after the following sequence of calls: kasan_disable_current(); // #1 kasan_disable_current(); // #2 kasan_enable_current(); // #3 kasan_enable_current(); // #4 the value of |current->kasan_depth| will be 2, so a single subsequent call to kasan_disable_current() won't disable KASAN. I think we'd better add BUG checks to bail out if the value of |current->kasan_depth| is too big or too small. > Another patch thread is also related with this patch thread, please help > check. > > And sorry, originally, I did not let the 2 patches in one patches set. > > Thanks. > -- > Chen Gang (陈刚) > > Managing Natural Environments is the Duty of Human Beings. -- Alexander Potapenko Software Engineer Google Germany GmbH Erika-Mann-Straße, 33 80636 München Geschäftsführer: Matthew Scott Sucherman, Paul Terence Manicle Registergericht und -nummer: Hamburg, HRB 86891 Sitz der Gesellschaft: Hamburg
[toc] | [prev] | [next] | [standalone]
| From | Chen Gang <chengang@emindsoft.com.cn> |
|---|---|
| Date | 2016-05-02 14:40 +0200 |
| Message-ID | <ruliP-3kM-47@gated-at.bofh.it> |
| In reply to | #1392143 |
On 5/2/16 19:23, Alexander Potapenko wrote: > On Mon, May 2, 2016 at 1:20 PM, Chen Gang <chengang@emindsoft.com.cn> wrote: >> On 5/2/16 18:49, Alexander Potapenko wrote: >>> On Mon, May 2, 2016 at 7:35 AM, <chengang@emindsoft.com.cn> wrote: >>>> >>>> According to their comments and the kasan_depth's initialization, if >>>> kasan_depth is zero, it means disable. So kasan_depth need consider >>>> about the 0 overflow. >>>> >>>> Also remove useless comments for dummy kasan_slab_free(). >>>> >>>> Signed-off-by: Chen Gang <gang.chen.5i5j@gmail.com> >>> >>> Acked-by: Alexander Potapenko <glider@google.com> > Nacked-by: Alexander Potapenko <glider@google.com> >>> >> >> OK, thanks. > Well, on a second thought I take that back, there still might be problems. > I haven't noticed the other CL, and was too hasty reviewing this one. > > As kasan_disable_current() and kasan_enable_current() always go > together, we need to prevent nested calls to them from breaking > everything. > If we ignore some calls to kasan_disable_current() to prevent > overflows, the pairing calls to kasan_enable_current() will bring > |current->kasan_depth| to an invalid state. > > E.g. if I'm understanding your idea correctly, after the following > sequence of calls: > kasan_disable_current(); // #1 > kasan_disable_current(); // #2 > kasan_enable_current(); // #3 > kasan_enable_current(); // #4 > > the value of |current->kasan_depth| will be 2, so a single subsequent > call to kasan_disable_current() won't disable KASAN. > > I think we'd better add BUG checks to bail out if the value of > |current->kasan_depth| is too big or too small. > For me, BUG_ON is OK. e.g. - BUG_ON(!kasan_depth) as soon as be in kasan_enable_current(). - BUG_ON(!(kasan_depth - 1)) as soon as be in kasan_disable_current(). Welcome another members ideas, if no any additional reply within 3 days, I shall send patch v2 for it. Thanks. -- Chen Gang (陈刚) Managing Natural Environments is the Duty of Human Beings.
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web