[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CALCETrVvbbCWMPo7v5eYgTocaxRQPHerJ=CRjWscGxgb6QjOFA@mail.gmail.com>
Date: Sat, 29 Jun 2019 16:43:18 -0700
From: Andy Lutomirski <luto@...nel.org>
To: Yu-cheng Yu <yu-cheng.yu@...el.com>
Cc: X86 ML <x86@...nel.org>, "H. Peter Anvin" <hpa@...or.com>,
Thomas Gleixner <tglx@...utronix.de>,
Ingo Molnar <mingo@...hat.com>,
LKML <linux-kernel@...r.kernel.org>,
"open list:DOCUMENTATION" <linux-doc@...r.kernel.org>,
Linux-MM <linux-mm@...ck.org>,
linux-arch <linux-arch@...r.kernel.org>,
Linux API <linux-api@...r.kernel.org>,
Arnd Bergmann <arnd@...db.de>,
Balbir Singh <bsingharora@...il.com>,
Borislav Petkov <bp@...en8.de>,
Cyrill Gorcunov <gorcunov@...il.com>,
Dave Hansen <dave.hansen@...ux.intel.com>,
Eugene Syromiatnikov <esyr@...hat.com>,
Florian Weimer <fweimer@...hat.com>,
"H.J. Lu" <hjl.tools@...il.com>, Jann Horn <jannh@...gle.com>,
Jonathan Corbet <corbet@....net>,
Kees Cook <keescook@...omium.org>,
Mike Kravetz <mike.kravetz@...cle.com>,
Nadav Amit <nadav.amit@...il.com>,
Oleg Nesterov <oleg@...hat.com>, Pavel Machek <pavel@....cz>,
Peter Zijlstra <peterz@...radead.org>,
Randy Dunlap <rdunlap@...radead.org>,
"Ravi V. Shankar" <ravi.v.shankar@...el.com>,
Vedvyas Shanbhogue <vedvyas.shanbhogue@...el.com>,
Dave Martin <Dave.Martin@....com>
Subject: Re: [RFC PATCH 2/3] Introduce arch_prctl(ARCH_X86_CET_MARK_LEGACY_CODE)
> On Jun 28, 2019, at 12:41 PM, Yu-cheng Yu <yu-cheng.yu@...el.com> wrote:
>
> The CET legacy code bitmap covers the whole user-mode address space and is
> located at the top of the user-mode address space. It is allocated only
> when the first time arch_prctl(ARCH_X86_MARK_LEGACY_CODE) is called from
> an application.
>
> Introduce:
>
> arch_prctl(ARCH_X86_MARK_LEGACY_CODE, unsigned long *buf)
> Mark an address range as IBT legacy code.
How about defining a struct for this?
The change log should discuss where the bitmap goes and how it’s allocated.
> +static int alloc_bitmap(void)
> +{
> + unsigned long addr;
> + u64 msr_ia32_u_cet;
> +
> + addr = do_mmap_locked(NULL, IBT_BITMAP_ADDR, IBT_BITMAP_SIZE,
> + PROT_READ | PROT_WRITE,
> + MAP_ANONYMOUS | MAP_PRIVATE | MAP_FIXED_NOREPLACE,
> + VM_IBT | VM_NORESERVE, NULL);
> +
> + if (IS_ERR((void *)addr))
> + return addr;
> +
> + current->thread.cet.ibt_bitmap_addr = addr;
addr is a constant. Why are you storing it? If it ends up not being
constant, you should wire up mremap like the vDSO does.
> +static int set_user_bits(unsigned long __user *buf, unsigned long buf_size,
> + unsigned long start_bit, unsigned long end_bit, unsigned long set)
> +{
> + unsigned long start_ul, end_ul, total_ul;
> + int i, j, r;
> +
> + if (round_up(end_bit, BITS_PER_BYTE) / BITS_PER_BYTE > buf_size)
> + end_bit = buf_size * BITS_PER_BYTE - 1;
> +
> + start_ul = start_bit / BITS_PER_LONG;
> + end_ul = end_bit / BITS_PER_LONG;
> + total_ul = (end_ul - start_ul + 1);
> +
> + i = start_bit % BITS_PER_LONG;
> + j = end_bit % BITS_PER_LONG;
> +
> + r = 0;
> + put_user_try {
put_user_try is obsolete. Just use get_user(), etc.
Also, I must be missing something fundamental, because this series
claims that user code can't write directly to the bitmap. This means
that this entire function shouldn't work at all.
Powered by blists - more mailing lists