linux-kernel - Re: [PATCH 1/7] pstore: Make spinlock per zone instead of global

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Mon, 10 Oct 2016 16:44:55 -0700
From:   Kees Cook <keescook@...omium.org>
To:     Joel Fernandes <joelaf@...gle.com>
Cc:     LKML <linux-kernel@...r.kernel.org>,
        Steven Rostedt <rostedt@...dmis.org>,
        Anton Vorontsov <anton@...msg.org>,
        Colin Cross <ccross@...roid.com>,
        Tony Luck <tony.luck@...el.com>
Subject: Re: [PATCH 1/7] pstore: Make spinlock per zone instead of global

On Fri, Oct 7, 2016 at 10:28 PM, Joel Fernandes <joelaf@...gle.com> wrote:
> Currently pstore has a global spinlock for all zones. Since the zones are
> independent and modify different areas of memory, there's no need to have
> a global lock, make the lock per-zone to protect the respective zone.

This seems fine to me, though I'd like the commit message to at least
hint at what's next, and why this change is needed, rather than just
the general organizational justification.

-Kees

>
> Signed-off-by: Joel Fernandes <joelaf@...gle.com>
> ---
>  fs/pstore/ram_core.c       | 11 +++++------
>  include/linux/pstore_ram.h |  1 +
>  2 files changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/fs/pstore/ram_core.c b/fs/pstore/ram_core.c
> index 3975dee..cb92055 100644
> --- a/fs/pstore/ram_core.c
> +++ b/fs/pstore/ram_core.c
> @@ -48,8 +48,6 @@ static inline size_t buffer_start(struct persistent_ram_zone *prz)
>         return atomic_read(&prz->buffer->start);
>  }
>
> -static DEFINE_RAW_SPINLOCK(buffer_lock);
> -
>  /* increase and wrap the start pointer, returning the old value */
>  static size_t buffer_start_add(struct persistent_ram_zone *prz, size_t a)
>  {
> @@ -57,7 +55,7 @@ static size_t buffer_start_add(struct persistent_ram_zone *prz, size_t a)
>         int new;
>         unsigned long flags;
>
> -       raw_spin_lock_irqsave(&buffer_lock, flags);
> +       raw_spin_lock_irqsave(&prz->buffer_lock, flags);
>
>         old = atomic_read(&prz->buffer->start);
>         new = old + a;
> @@ -65,7 +63,7 @@ static size_t buffer_start_add(struct persistent_ram_zone *prz, size_t a)
>                 new -= prz->buffer_size;
>         atomic_set(&prz->buffer->start, new);
>
> -       raw_spin_unlock_irqrestore(&buffer_lock, flags);
> +       raw_spin_unlock_irqrestore(&prz->buffer_lock, flags);
>
>         return old;
>  }
> @@ -77,7 +75,7 @@ static void buffer_size_add(struct persistent_ram_zone *prz, size_t a)
>         size_t new;
>         unsigned long flags;
>
> -       raw_spin_lock_irqsave(&buffer_lock, flags);
> +       raw_spin_lock_irqsave(&prz->buffer_lock, flags);
>
>         old = atomic_read(&prz->buffer->size);
>         if (old == prz->buffer_size)
> @@ -89,7 +87,7 @@ static void buffer_size_add(struct persistent_ram_zone *prz, size_t a)
>         atomic_set(&prz->buffer->size, new);
>
>  exit:
> -       raw_spin_unlock_irqrestore(&buffer_lock, flags);
> +       raw_spin_unlock_irqrestore(&prz->buffer_lock, flags);
>  }
>
>  static void notrace persistent_ram_encode_rs8(struct persistent_ram_zone *prz,
> @@ -493,6 +491,7 @@ static int persistent_ram_post_init(struct persistent_ram_zone *prz, u32 sig,
>
>         prz->buffer->sig = sig;
>         persistent_ram_zap(prz);
> +       prz->buffer_lock = __RAW_SPIN_LOCK_UNLOCKED(buffer_lock);
>
>         return 0;
>  }
> diff --git a/include/linux/pstore_ram.h b/include/linux/pstore_ram.h
> index c668c86..244d242 100644
> --- a/include/linux/pstore_ram.h
> +++ b/include/linux/pstore_ram.h
> @@ -40,6 +40,7 @@ struct persistent_ram_zone {
>         void *vaddr;
>         struct persistent_ram_buffer *buffer;
>         size_t buffer_size;
> +       raw_spinlock_t buffer_lock;
>
>         /* ECC correction */
>         char *par_buffer;
> --
> 2.7.4
>



-- 
Kees Cook
Nexus Security