From: Kees Cook <keescook@chromium.org>
To: Dave Martin <Dave.Martin@arm.com>
Cc: linux-kernel@vger.kernel.org, linux-arch@vger.kernel.org,
Thomas Gleixner <tglx@linutronix.de>,
Jann Horn <jannh@google.com>, "H.J. Lu" <hjl.tools@gmail.com>,
Eugene Syromiatnikov <esyr@redhat.com>,
Florian Weimer <fweimer@redhat.com>,
Yu-cheng Yu <yu-cheng.yu@intel.com>,
Peter Zijlstra <peterz@infradead.org>
Subject: Re: [RFC PATCH v2 2/2] ELF: Add ELF program property parsing support
Date: Thu, 29 Aug 2019 22:37:45 -0700 [thread overview]
Message-ID: <201908292224.007EB4D5@keescook> (raw)
In-Reply-To: <1566581020-9953-3-git-send-email-Dave.Martin@arm.com>
On Fri, Aug 23, 2019 at 06:23:40PM +0100, Dave Martin wrote:
> ELF program properties will needed for detecting whether to enable
> optional architecture or ABI features for a new ELF process.
>
> For now, there are no generic properties that we care about, so do
> nothing unless CONFIG_ARCH_USE_GNU_PROPERTY=y.
>
> Otherwise, the presence of properties using the PT_PROGRAM_PROPERTY
> phdrs entry (if any), and notify each property to the arch code.
>
> For now, the added code is not used.
>
> Signed-off-by: Dave Martin <Dave.Martin@arm.com>
Reviewed-by: Kees Cook <keescook@chromium.org>
Note below...
> [...]
> +static int parse_elf_property(const char *data, size_t *off, size_t datasz,
> + struct arch_elf_state *arch,
> + bool have_prev_type, u32 *prev_type)
> +{
> + size_t size, step;
> + const struct gnu_property *pr;
> + int ret;
> +
> + if (*off == datasz)
> + return -ENOENT;
> +
> + if (WARN_ON(*off > datasz || *off % elf_gnu_property_align))
> + return -EIO;
> +
> + size = datasz - *off;
> + if (size < sizeof(*pr))
> + return -EIO;
> +
> + pr = (const struct gnu_property *)(data + *off);
> + if (pr->pr_datasz > size - sizeof(*pr))
> + return -EIO;
> +
> + step = round_up(sizeof(*pr) + pr->pr_datasz, elf_gnu_property_align);
> + if (step > size)
> + return -EIO;
> +
> + /* Properties are supposed to be unique and sorted on pr_type: */
> + if (have_prev_type && pr->pr_type <= *prev_type)
> + return -EIO;
> + *prev_type = pr->pr_type;
> +
> + ret = arch_parse_elf_property(pr->pr_type,
> + data + *off + sizeof(*pr),
> + pr->pr_datasz, ELF_COMPAT, arch);
I find it slightly hard to read the "cursor" motion in this parse. It
feels strange, for example, to refer twice to "data + *off" with the
second including consumed *pr size. Everything is fine AFAICT in the math,
though, and I haven't been able to construct a convincingly "cleaner"
version. Maybe:
data += *off;
pr = (const struct gnu_property *)data;
data += sizeof(*pr);
...
ret = arch_parse_elf_property(pr->pr_type, data,
pr->pr_datasz, ELF_COMPAT, arch);
But that feels disjoint from the "step" calculation, so... I think what
you have is fine. :)
--
Kees Cook
next prev parent reply other threads:[~2019-08-30 5:37 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-08-23 17:23 [RFC PATCH v2 0/2] ELF: Alternate program property parser Dave Martin
2019-08-23 17:23 ` [RFC PATCH v2 1/2] ELF: UAPI and Kconfig additions for ELF program properties Dave Martin
2019-08-23 17:23 ` [RFC PATCH v2 2/2] ELF: Add ELF program property parsing support Dave Martin
2019-08-30 5:37 ` Kees Cook [this message]
2019-08-30 8:34 ` Dave Martin
2019-08-30 17:03 ` Yu-cheng Yu
2019-09-02 9:28 ` Dave Martin
2019-09-03 22:29 ` Yu-cheng Yu
2019-09-04 11:05 ` Dave Martin
2019-09-04 16:50 ` Kees Cook
2019-09-05 7:57 ` Dave Martin
2019-10-09 12:59 ` Dave Martin
2019-10-10 21:00 ` Kees Cook
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=201908292224.007EB4D5@keescook \
--to=keescook@chromium.org \
--cc=Dave.Martin@arm.com \
--cc=esyr@redhat.com \
--cc=fweimer@redhat.com \
--cc=hjl.tools@gmail.com \
--cc=jannh@google.com \
--cc=linux-arch@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=peterz@infradead.org \
--cc=tglx@linutronix.de \
--cc=yu-cheng.yu@intel.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).