linux-kernel - Re: [RFC 3/6] pstore: remove max argument from ramoops_get_next

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]

Message-ID: <CAGXu5jKqVJeCeOkBsEjjc5ZzieW-N1iRZ7Qjt7KL_NWLiPLaGg@mail.gmail.com>
Date:   Fri, 26 Oct 2018 20:22:55 +0100
From:   Kees Cook <keescook@...omium.org>
To:     "Joel Fernandes (Google)" <joel@...lfernandes.org>
Cc:     LKML <linux-kernel@...r.kernel.org>, kernel-team@...roid.com,
        Anton Vorontsov <anton@...msg.org>,
        Colin Cross <ccross@...roid.com>,
        Tony Luck <tony.luck@...el.com>
Subject: Re: [RFC 3/6] pstore: remove max argument from ramoops_get_next_prz

On Fri, Oct 26, 2018 at 7:00 PM, Joel Fernandes (Google)
<joel@...lfernandes.org> wrote:
> From the code flow, the 'max' checks are already being done on the prz
> passed to ramoops_get_next_prz. Lets remove it to simplify this function
> and reduce its arguments.
>
> Signed-off-by: Joel Fernandes (Google) <joel@...lfernandes.org>
> ---
>  fs/pstore/ram.c | 14 ++++++--------
>  1 file changed, 6 insertions(+), 8 deletions(-)
>
> diff --git a/fs/pstore/ram.c b/fs/pstore/ram.c
> index cbfdf4b8e89d..3055e05acab1 100644
> --- a/fs/pstore/ram.c
> +++ b/fs/pstore/ram.c
> @@ -124,14 +124,14 @@ static int ramoops_pstore_open(struct pstore_info *psi)
>  }
>
>  static struct persistent_ram_zone *
> -ramoops_get_next_prz(struct persistent_ram_zone *przs[], uint *c, uint max,
> +ramoops_get_next_prz(struct persistent_ram_zone *przs[], uint *c,
>                      u64 *id, enum pstore_type_id *typep, bool update)
>  {
>         struct persistent_ram_zone *prz;
>         int i = (*c)++;
>
>         /* Give up if we never existed or have hit the end. */
> -       if (!przs || i >= max)
> +       if (!przs)
>                 return NULL;
>
>         prz = przs[i];
> @@ -254,8 +254,7 @@ static ssize_t ramoops_pstore_read(struct pstore_record *record)
>         /* Find the next valid persistent_ram_zone for DMESG */
>         while (cxt->dump_read_cnt < cxt->max_dump_cnt && !prz) {
>                 prz = ramoops_get_next_prz(cxt->dprzs, &cxt->dump_read_cnt,
> -                                          cxt->max_dump_cnt, &record->id,
> -                                          &record->type, 1);
> +                                          &record->id, &record->type, 1);
>                 if (!prz_ok(prz))
>                         continue;
>                 header_length = ramoops_read_kmsg_hdr(persistent_ram_old(prz),
> @@ -271,17 +270,17 @@ static ssize_t ramoops_pstore_read(struct pstore_record *record)
>
>         if (!prz_ok(prz))
>                 prz = ramoops_get_next_prz(&cxt->cprz, &cxt->console_read_cnt,
> -                                          1, &record->id, &record->type, 0);
> +                                          &record->id, &record->type, 0);
>
>         if (!prz_ok(prz))
>                 prz = ramoops_get_next_prz(&cxt->mprz, &cxt->pmsg_read_cnt,
> -                                          1, &record->id, &record->type, 0);
> +                                          &record->id, &record->type, 0);
>
>         /* ftrace is last since it may want to dynamically allocate memory. */
>         if (!prz_ok(prz)) {
>                 if (!(cxt->flags & RAMOOPS_FLAG_FTRACE_PER_CPU)) {
>                         prz = ramoops_get_next_prz(cxt->fprzs,
> -                                       &cxt->ftrace_read_cnt, 1, &record->id,
> +                                       &cxt->ftrace_read_cnt, &record->id,
>                                         &record->type, 0);
>                 } else {
>                         /*
> @@ -299,7 +298,6 @@ static ssize_t ramoops_pstore_read(struct pstore_record *record)
>                         while (cxt->ftrace_read_cnt < cxt->max_ftrace_cnt) {
>                                 prz_next = ramoops_get_next_prz(cxt->fprzs,
>                                                 &cxt->ftrace_read_cnt,
> -                                               cxt->max_ftrace_cnt,
>                                                 &record->id,
>                                                 &record->type, 0);
>
> --
> 2.19.1.568.g152ad8e336-goog
>

Yup, looks good.

-- 
Kees Cook