* Re: [PATCH v7 03/14] x86/cet/ibt: Add IBT legacy code bitmap setup function
From: Dave Hansen @ 2019-06-07 16:39 UTC (permalink / raw)
To: Andy Lutomirski, Yu-cheng Yu
Cc: Peter Zijlstra, x86, H. Peter Anvin, Thomas Gleixner, Ingo Molnar,
linux-kernel, linux-doc, linux-mm, linux-arch, linux-api,
Arnd Bergmann, Balbir Singh, Borislav Petkov, Cyrill Gorcunov,
Dave Hansen, Eugene Syromiatnikov, Florian Weimer, H.J. Lu,
Jann Horn, Jonathan Corbet, Kees Cook, Mike Kravetz, Nadav Amit,
Oleg Nesterov, Pavel Machek, Randy Dunlap, Ravi V. Shankar,
Vedvyas Shanbhogue, Dave Martin
In-Reply-To: <76B7B1AE-3AEA-4162-B539-990EF3CCE2C2@amacapital.net>
On 6/7/19 9:35 AM, Andy Lutomirski wrote:
> One might reasonably wonder why this state is privileged in the first
> place and, given that, why we’re allowing it to be written like
> this.
I think it's generally a good architectural practice to make things like
this privileged. They're infrequent so can survive the cost of a trip
in/out of the kernel and are a great choke point to make sure the OS is
involved. I wish we had the same for MPX or pkeys per-task "setup".
^ permalink raw reply
* Re: [PATCH v7 03/14] x86/cet/ibt: Add IBT legacy code bitmap setup function
From: Yu-cheng Yu @ 2019-06-07 16:45 UTC (permalink / raw)
To: Andy Lutomirski
Cc: Peter Zijlstra, x86, H. Peter Anvin, Thomas Gleixner, Ingo Molnar,
linux-kernel, linux-doc, linux-mm, linux-arch, linux-api,
Arnd Bergmann, Balbir Singh, Borislav Petkov, Cyrill Gorcunov,
Dave Hansen, Eugene Syromiatnikov, Florian Weimer, H.J. Lu,
Jann Horn, Jonathan Corbet, Kees Cook, Mike Kravetz, Nadav Amit,
Oleg Nesterov, Pavel Machek, Randy Dunlap, Ravi V. Shankar,
Vedvyas Shanbhogue, Dave Martin
In-Reply-To: <76B7B1AE-3AEA-4162-B539-990EF3CCE2C2@amacapital.net>
On Fri, 2019-06-07 at 09:35 -0700, Andy Lutomirski wrote:
> > On Jun 7, 2019, at 9:23 AM, Yu-cheng Yu <yu-cheng.yu@intel.com> wrote:
> >
> > > On Fri, 2019-06-07 at 10:08 +0200, Peter Zijlstra wrote:
> > > > On Thu, Jun 06, 2019 at 01:09:15PM -0700, Yu-cheng Yu wrote:
> > > > Indirect Branch Tracking (IBT) provides an optional legacy code bitmap
> > > > that allows execution of legacy, non-IBT compatible library by an
> > > > IBT-enabled application. When set, each bit in the bitmap indicates
> > > > one page of legacy code.
> > > >
> > > > The bitmap is allocated and setup from the application.
> > > > +int cet_setup_ibt_bitmap(unsigned long bitmap, unsigned long size)
> > > > +{
> > > > + u64 r;
> > > > +
> > > > + if (!current->thread.cet.ibt_enabled)
> > > > + return -EINVAL;
> > > > +
> > > > + if (!PAGE_ALIGNED(bitmap) || (size > TASK_SIZE_MAX))
> > > > + return -EINVAL;
> > > > +
> > > > + current->thread.cet.ibt_bitmap_addr = bitmap;
> > > > + current->thread.cet.ibt_bitmap_size = size;
> > > > +
> > > > + /*
> > > > + * Turn on IBT legacy bitmap.
> > > > + */
> > > > + modify_fpu_regs_begin();
> > > > + rdmsrl(MSR_IA32_U_CET, r);
> > > > + r |= (MSR_IA32_CET_LEG_IW_EN | bitmap);
> > > > + wrmsrl(MSR_IA32_U_CET, r);
> > > > + modify_fpu_regs_end();
> > > > +
> > > > + return 0;
> > > > +}
> > >
> > > So you just program a random user supplied address into the hardware.
> > > What happens if there's not actually anything at that address or the
> > > user munmap()s the data after doing this?
> >
> > This function checks the bitmap's alignment and size, and anything else is
> > the
> > app's responsibility. What else do you think the kernel should check?
> >
>
> One might reasonably wonder why this state is privileged in the first place
> and, given that, why we’re allowing it to be written like this.
>
> Arguably we should have another prctl to lock these values (until exec) as a
> gardening measure.
We can prevent the bitmap from being set more than once. I will test it.
Yu-cheng
^ permalink raw reply
* Re: [PATCH v7 03/14] x86/cet/ibt: Add IBT legacy code bitmap setup function
From: Andy Lutomirski @ 2019-06-07 17:05 UTC (permalink / raw)
To: Yu-cheng Yu
Cc: Peter Zijlstra, x86, H. Peter Anvin, Thomas Gleixner, Ingo Molnar,
linux-kernel, linux-doc, linux-mm, linux-arch, linux-api,
Arnd Bergmann, Balbir Singh, Borislav Petkov, Cyrill Gorcunov,
Dave Hansen, Eugene Syromiatnikov, Florian Weimer, H.J. Lu,
Jann Horn, Jonathan Corbet, Kees Cook, Mike Kravetz, Nadav Amit,
Oleg Nesterov, Pavel Machek, Randy Dunlap, Ravi V. Shankar,
Vedvyas Shanbhogue, Dave Martin
In-Reply-To: <ac8827d7b516f4b58e1df20f45b94998d36c418c.camel@intel.com>
> On Jun 7, 2019, at 9:45 AM, Yu-cheng Yu <yu-cheng.yu@intel.com> wrote:
>
> On Fri, 2019-06-07 at 09:35 -0700, Andy Lutomirski wrote:
>>> On Jun 7, 2019, at 9:23 AM, Yu-cheng Yu <yu-cheng.yu@intel.com> wrote:
>>>
>>>>> On Fri, 2019-06-07 at 10:08 +0200, Peter Zijlstra wrote:
>>>>> On Thu, Jun 06, 2019 at 01:09:15PM -0700, Yu-cheng Yu wrote:
>>>>> Indirect Branch Tracking (IBT) provides an optional legacy code bitmap
>>>>> that allows execution of legacy, non-IBT compatible library by an
>>>>> IBT-enabled application. When set, each bit in the bitmap indicates
>>>>> one page of legacy code.
>>>>>
>>>>> The bitmap is allocated and setup from the application.
>>>>> +int cet_setup_ibt_bitmap(unsigned long bitmap, unsigned long size)
>>>>> +{
>>>>> + u64 r;
>>>>> +
>>>>> + if (!current->thread.cet.ibt_enabled)
>>>>> + return -EINVAL;
>>>>> +
>>>>> + if (!PAGE_ALIGNED(bitmap) || (size > TASK_SIZE_MAX))
>>>>> + return -EINVAL;
>>>>> +
>>>>> + current->thread.cet.ibt_bitmap_addr = bitmap;
>>>>> + current->thread.cet.ibt_bitmap_size = size;
>>>>> +
>>>>> + /*
>>>>> + * Turn on IBT legacy bitmap.
>>>>> + */
>>>>> + modify_fpu_regs_begin();
>>>>> + rdmsrl(MSR_IA32_U_CET, r);
>>>>> + r |= (MSR_IA32_CET_LEG_IW_EN | bitmap);
>>>>> + wrmsrl(MSR_IA32_U_CET, r);
>>>>> + modify_fpu_regs_end();
>>>>> +
>>>>> + return 0;
>>>>> +}
>>>>
>>>> So you just program a random user supplied address into the hardware.
>>>> What happens if there's not actually anything at that address or the
>>>> user munmap()s the data after doing this?
>>>
>>> This function checks the bitmap's alignment and size, and anything else is
>>> the
>>> app's responsibility. What else do you think the kernel should check?
>>>
>>
>> One might reasonably wonder why this state is privileged in the first place
>> and, given that, why we’re allowing it to be written like this.
>>
>> Arguably we should have another prctl to lock these values (until exec) as a
>> gardening measure.
>
> We can prevent the bitmap from being set more than once. I will test it.
>
I think it would be better to make locking an explicit opt-in.
^ permalink raw reply
* [PATCH] lib/string_helpers: fix some kerneldoc warnings
From: Jonathan Corbet @ 2019-06-07 17:09 UTC (permalink / raw)
To: Andrew Morton; +Cc: LKML, linux-doc
Due to some sad limitations in how kerneldoc comments are parsed, the
documentation in lib/string_helpers.c generates these warnings:
./lib/string_helpers.c:236: WARNING: Unexpected indentation.
./lib/string_helpers.c:241: WARNING: Block quote ends without a blank line; unexpected unindent.
./lib/string_helpers.c:446: WARNING: Unexpected indentation.
./lib/string_helpers.c:451: WARNING: Block quote ends without a blank line; unexpected unindent.
./lib/string_helpers.c:474: WARNING: Unexpected indentation.
Rework the comments to obtain something like the desired result.
Signed-off-by: Jonathan Corbet <corbet@lwn.net>
---
lib/string_helpers.c | 77 +++++++++++++++++++++++---------------------
1 file changed, 40 insertions(+), 37 deletions(-)
diff --git a/lib/string_helpers.c b/lib/string_helpers.c
index 29c490e5d478..a9f5ef9289e6 100644
--- a/lib/string_helpers.c
+++ b/lib/string_helpers.c
@@ -230,35 +230,36 @@ static bool unescape_special(char **src, char **dst)
* @src: source buffer (escaped)
* @dst: destination buffer (unescaped)
* @size: size of the destination buffer (0 to unlimit)
- * @flags: combination of the flags (bitwise OR):
- * %UNESCAPE_SPACE:
+ * @flags: combination of the flags.
+ *
+ * Description:
+ * The function unquotes characters in the given string.
+ *
+ * Because the size of the output will be the same as or less than the size of
+ * the input, the transformation may be performed in place.
+ *
+ * Caller must provide valid source and destination pointers. Be aware that
+ * destination buffer will always be NULL-terminated. Source string must be
+ * NULL-terminated as well. The supported flags are::
+ *
+ * UNESCAPE_SPACE:
* '\f' - form feed
* '\n' - new line
* '\r' - carriage return
* '\t' - horizontal tab
* '\v' - vertical tab
- * %UNESCAPE_OCTAL:
+ * UNESCAPE_OCTAL:
* '\NNN' - byte with octal value NNN (1 to 3 digits)
- * %UNESCAPE_HEX:
+ * UNESCAPE_HEX:
* '\xHH' - byte with hexadecimal value HH (1 to 2 digits)
- * %UNESCAPE_SPECIAL:
+ * UNESCAPE_SPECIAL:
* '\"' - double quote
* '\\' - backslash
* '\a' - alert (BEL)
* '\e' - escape
- * %UNESCAPE_ANY:
+ * UNESCAPE_ANY:
* all previous together
*
- * Description:
- * The function unquotes characters in the given string.
- *
- * Because the size of the output will be the same as or less than the size of
- * the input, the transformation may be performed in place.
- *
- * Caller must provide valid source and destination pointers. Be aware that
- * destination buffer will always be NULL-terminated. Source string must be
- * NULL-terminated as well.
- *
* Return:
* The amount of the characters processed to the destination buffer excluding
* trailing '\0' is returned.
@@ -440,7 +441,29 @@ static bool escape_hex(unsigned char c, char **dst, char *end)
* @isz: source buffer size
* @dst: destination buffer (escaped)
* @osz: destination buffer size
- * @flags: combination of the flags (bitwise OR):
+ * @flags: combination of the flags
+ * @only: NULL-terminated string containing characters used to limit
+ * the selected escape class. If characters are included in @only
+ * that would not normally be escaped by the classes selected
+ * in @flags, they will be copied to @dst unescaped.
+ *
+ * Description:
+ * The process of escaping byte buffer includes several parts. They are applied
+ * in the following sequence.
+ *
+ * 1. The character is matched to the printable class, if asked, and in
+ * case of match it passes through to the output.
+ * 2. The character is not matched to the one from @only string and thus
+ * must go as-is to the output.
+ * 3. The character is checked if it falls into the class given by @flags.
+ * %ESCAPE_OCTAL and %ESCAPE_HEX are going last since they cover any
+ * character. Note that they actually can't go together, otherwise
+ * %ESCAPE_HEX will be ignored.
+ *
+ * Caller must provide valid source and destination pointers. Be aware that
+ * destination buffer will not be NULL-terminated, thus caller have to append
+ * it if needs. The supported flags are::
+ *
* %ESCAPE_SPACE: (special white space, not space itself)
* '\f' - form feed
* '\n' - new line
@@ -463,26 +486,6 @@ static bool escape_hex(unsigned char c, char **dst, char *end)
* all previous together
* %ESCAPE_HEX:
* '\xHH' - byte with hexadecimal value HH (2 digits)
- * @only: NULL-terminated string containing characters used to limit
- * the selected escape class. If characters are included in @only
- * that would not normally be escaped by the classes selected
- * in @flags, they will be copied to @dst unescaped.
- *
- * Description:
- * The process of escaping byte buffer includes several parts. They are applied
- * in the following sequence.
- * 1. The character is matched to the printable class, if asked, and in
- * case of match it passes through to the output.
- * 2. The character is not matched to the one from @only string and thus
- * must go as-is to the output.
- * 3. The character is checked if it falls into the class given by @flags.
- * %ESCAPE_OCTAL and %ESCAPE_HEX are going last since they cover any
- * character. Note that they actually can't go together, otherwise
- * %ESCAPE_HEX will be ignored.
- *
- * Caller must provide valid source and destination pointers. Be aware that
- * destination buffer will not be NULL-terminated, thus caller have to append
- * it if needs.
*
* Return:
* The total size of the escaped output that would be generated for
--
2.21.0
^ permalink raw reply related
* Re: [PATCH] Documentation: DMA-API: fix a function name of max_mapping_size
From: Jonathan Corbet @ 2019-06-07 17:10 UTC (permalink / raw)
To: Yoshihiro Shimoda; +Cc: jroedel, hch, m.szyprowski, linux-doc, iommu
In-Reply-To: <1559893633-6852-1-git-send-email-yoshihiro.shimoda.uh@renesas.com>
On Fri, 7 Jun 2019 16:47:13 +0900
Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> wrote:
> The exported function name is dma_max_mapping_size(), not
> dma_direct_max_mapping_size() so that this patch fixes
> the function name in the documentation.
>
> Fixes: 133d624b1cee ("dma: Introduce dma_max_mapping_size()")
> Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> ---
> Documentation/DMA-API.txt | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/Documentation/DMA-API.txt b/Documentation/DMA-API.txt
> index 0076150..e47c63b 100644
> --- a/Documentation/DMA-API.txt
> +++ b/Documentation/DMA-API.txt
> @@ -198,7 +198,7 @@ call to set the mask to the value returned.
> ::
>
> size_t
> - dma_direct_max_mapping_size(struct device *dev);
> + dma_max_mapping_size(struct device *dev);
>
> Returns the maximum size of a mapping for the device. The size parameter
> of the mapping functions like dma_map_single(), dma_map_page() and
Applied, thanks.
jon
^ permalink raw reply
* Re: [PATCH] Documentation/dm-init: fix multi device example
From: Jonathan Corbet @ 2019-06-07 17:13 UTC (permalink / raw)
To: Helen Koike
Cc: dm-devel, swboyd, wad, keescook, snitzer, linux-doc,
richard.weinberger, linux-kernel, linux-lvm, enric.balletbo,
kernel, agk
In-Reply-To: <20190604182719.15944-1-helen.koike@collabora.com>
On Tue, 4 Jun 2019 15:27:19 -0300
Helen Koike <helen.koike@collabora.com> wrote:
> The example in the docs regarding multiple device-mappers is invalid (it
> has a wrong number of arguments), it's a left over from previous
> versions of the patch.
> Replace the example with an valid and tested one.
>
> Signed-off-by: Helen Koike <helen.koike@collabora.com>
Applied, thanks.
jon
^ permalink raw reply
* Re: [PATCH] Documentation: fix typo CLOCK_MONONOTNIC_COARSE
From: Jonathan Corbet @ 2019-06-07 17:22 UTC (permalink / raw)
To: Aurelien Thierry
Cc: Arnd Bergmann, John Stultz, Thomas Gleixner, Linus Walleij,
Randy Dunlap, linux-doc, trivial
In-Reply-To: <593f6e44-af55-ac7c-1c67-12e515028fe5@quoscient.io>
On Fri, 7 Jun 2019 10:07:02 +0200
Aurelien Thierry <aurelien.thierry@quoscient.io> wrote:
> Fix typo in documentation file timekeeping.rst: CLOCK_MONONOTNIC_COARSE
> should be CLOCK_MONOTONIC_COARSE.
>
> Signed-off-by: Aurelien Thierry <aurelien.thierry@quoscient.io>
> ---
> Documentation/core-api/timekeeping.rst | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/Documentation/core-api/timekeeping.rst
> b/Documentation/core-api/timekeeping.rst
> index 93cbeb9daec0..5f87d9c8b04d 100644
> --- a/Documentation/core-api/timekeeping.rst
> +++ b/Documentation/core-api/timekeeping.rst
> @@ -111,7 +111,7 @@ Some additional variants exist for more specialized
> cases:
> void ktime_get_coarse_raw_ts64( struct timespec64 * )
>
> These are quicker than the non-coarse versions, but less accurate,
> - corresponding to CLOCK_MONONOTNIC_COARSE and CLOCK_REALTIME_COARSE
> + corresponding to CLOCK_MONOTONIC_COARSE and CLOCK_REALTIME_COARSE
> in user space, along with the equivalent boottime/tai/raw
> timebase not available in user space.
Patch applied; congratulations on what appears to be your first kernel
patch!
Note that this patch was badly corrupted by your mailer; it took me a
while to unmangle it. Before sending your next one, I'd recommend being
sure that you can email it to yourself and apply the result. There are
some hints in Documentation/process/email-clients.rst.
Thanks,
jon
^ permalink raw reply
* Re: [PATCH trivial] Documentation: tee: Grammar s/the its/its/
From: Jonathan Corbet @ 2019-06-07 17:23 UTC (permalink / raw)
To: Geert Uytterhoeven; +Cc: Jens Wiklander, Jiri Kosina, linux-doc, linux-kernel
In-Reply-To: <20190607110729.12734-1-geert+renesas@glider.be>
On Fri, 7 Jun 2019 13:07:29 +0200
Geert Uytterhoeven <geert+renesas@glider.be> wrote:
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
> Documentation/tee.txt | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/Documentation/tee.txt b/Documentation/tee.txt
> index 56ea85ffebf24545..afacdf2fd1de5455 100644
> --- a/Documentation/tee.txt
> +++ b/Documentation/tee.txt
> @@ -32,7 +32,7 @@ User space (the client) connects to the driver by opening /dev/tee[0-9]* or
> memory.
>
> - TEE_IOC_VERSION lets user space know which TEE this driver handles and
> - the its capabilities.
> + its capabilities.
>
> - TEE_IOC_OPEN_SESSION opens a new session to a Trusted Application.
Applied, thanks.
jon
^ permalink raw reply
* Re: [PATCH trivial] Documentation: net: dsa: Grammar s/the its/its/
From: Jonathan Corbet @ 2019-06-07 17:24 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: David S . Miller, Jiri Kosina, netdev, linux-doc, linux-kernel
In-Reply-To: <20190607110842.12876-1-geert+renesas@glider.be>
On Fri, 7 Jun 2019 13:08:42 +0200
Geert Uytterhoeven <geert+renesas@glider.be> wrote:
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
> Documentation/networking/dsa/dsa.rst | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/networking/dsa/dsa.rst b/Documentation/networking/dsa/dsa.rst
> index ca87068b9ab904a9..563d56c6a25c924e 100644
> --- a/Documentation/networking/dsa/dsa.rst
> +++ b/Documentation/networking/dsa/dsa.rst
> @@ -531,7 +531,7 @@ Bridge VLAN filtering
> a software implementation.
>
> .. note:: VLAN ID 0 corresponds to the port private database, which, in the context
> - of DSA, would be the its port-based VLAN, used by the associated bridge device.
> + of DSA, would be its port-based VLAN, used by the associated bridge device.
>
> - ``port_fdb_del``: bridge layer function invoked when the bridge wants to remove a
> Forwarding Database entry, the switch hardware should be programmed to delete
> @@ -554,7 +554,7 @@ Bridge VLAN filtering
> associated with this VLAN ID.
>
> .. note:: VLAN ID 0 corresponds to the port private database, which, in the context
> - of DSA, would be the its port-based VLAN, used by the associated bridge device.
> + of DSA, would be its port-based VLAN, used by the associated bridge device.
Applied, thanks.
jon
^ permalink raw reply
* Re: [PATCH trivial] KVM: arm/arm64: Always capitalize ITS
From: Jonathan Corbet @ 2019-06-07 17:24 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Paolo Bonzini, Radim Krčmář, Jiri Kosina, kvm,
linux-doc, linux-kernel
In-Reply-To: <20190607112951.14418-1-geert+renesas@glider.be>
On Fri, 7 Jun 2019 13:29:51 +0200
Geert Uytterhoeven <geert+renesas@glider.be> wrote:
> All but one reference is capitalized. Fix the remaining one.
>
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
> Documentation/virtual/kvm/devices/arm-vgic-its.txt | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/Documentation/virtual/kvm/devices/arm-vgic-its.txt b/Documentation/virtual/kvm/devices/arm-vgic-its.txt
> index 4f0c9fc403656d29..eeaa95b893a89b7a 100644
> --- a/Documentation/virtual/kvm/devices/arm-vgic-its.txt
> +++ b/Documentation/virtual/kvm/devices/arm-vgic-its.txt
> @@ -103,7 +103,7 @@ Groups:
> The following ordering must be followed when restoring the GIC and the ITS:
> a) restore all guest memory and create vcpus
> b) restore all redistributors
> -c) provide the its base address
> +c) provide the ITS base address
> (KVM_DEV_ARM_VGIC_GRP_ADDR)
> d) restore the ITS in the following order:
> 1. Restore GITS_CBASER
Applied, thanks.
jon
^ permalink raw reply
* Re: [PATCH] docs: Kbuild/Makefile: allow check for missing docs at build time
From: Jonathan Corbet @ 2019-06-07 17:34 UTC (permalink / raw)
To: Mauro Carvalho Chehab
Cc: Linux Doc Mailing List, Mauro Carvalho Chehab, linux-kernel,
Masahiro Yamada, Randy Dunlap
In-Reply-To: <8ac254a7cf0569f1eeced9c9263cd7b0bfe4ed78.1559651025.git.mchehab+samsung@kernel.org>
On Tue, 4 Jun 2019 09:26:27 -0300
Mauro Carvalho Chehab <mchehab+samsung@kernel.org> wrote:
> While this doesn't make sense for production Kernels, in order to
> avoid regressions when documents are touched, let's add a
> check target at the make file.
>
> Signed-off-by: Mauro Carvalho Chehab <mchehab+samsung@kernel.org>
Applied, thanks.
jon
^ permalink raw reply
* Re: [PATCH] docs: clk: fix struct syntax
From: Jonathan Corbet @ 2019-06-07 17:35 UTC (permalink / raw)
To: Luca Ceresoli; +Cc: linux-doc, linux-kernel
In-Reply-To: <20190531143016.23185-1-luca@lucaceresoli.net>
On Fri, 31 May 2019 16:30:16 +0200
Luca Ceresoli <luca@lucaceresoli.net> wrote:
> The clk_foo_ops struct example has syntax errors. Fix it so it can be
> copy-pasted and used more easily.
>
> Signed-off-by: Luca Ceresoli <luca@lucaceresoli.net>
Applied, thanks.
jon
^ permalink raw reply
* Re: [PATCH] doc:it_IT: fix file references
From: Jonathan Corbet @ 2019-06-07 17:36 UTC (permalink / raw)
To: Federico Vaga; +Cc: linux-doc, linux-kernel, Mauro Carvalho Chehab
In-Reply-To: <20190530201455.12412-1-federico.vaga@vaga.pv.it>
On Thu, 30 May 2019 22:14:54 +0200
Federico Vaga <federico.vaga@vaga.pv.it> wrote:
> Fix italian translation file references based on
> `scripts/documentation-file-ref-check` output.
>
> Signed-off-by: Federico Vaga <federico.vaga@vaga.pv.it>
Applied, thanks.
jon
^ permalink raw reply
* Re: [PATCH] doc:it_IT: documentation alignment
From: Jonathan Corbet @ 2019-06-07 17:37 UTC (permalink / raw)
To: Federico Vaga; +Cc: linux-doc, linux-kernel, Mauro Carvalho Chehab
In-Reply-To: <20190530201455.12412-2-federico.vaga@vaga.pv.it>
On Thu, 30 May 2019 22:14:55 +0200
Federico Vaga <federico.vaga@vaga.pv.it> wrote:
> Documentation alignment for the following changes:
> a700767a7682 (doc/docs-next) docs: requirements.txt: recommend Sphinx 1.7.9
>
> Signed-off-by: Federico Vaga <federico.vaga@vaga.pv.it>
Applied, thanks.
jon
^ permalink raw reply
* Re: [PATCH v2] Documentation: nvdimm: Fix typo
From: Jonathan Corbet @ 2019-06-07 17:39 UTC (permalink / raw)
To: Shiyang Ruan; +Cc: linux-doc, linux-kernel, linux-nvdimm
In-Reply-To: <20190509074049.12192-1-ruansy.fnst@cn.fujitsu.com>
On Thu, 9 May 2019 15:40:49 +0800
Shiyang Ruan <ruansy.fnst@cn.fujitsu.com> wrote:
> Remove the extra 'we '.
>
> Signed-off-by: Shiyang Ruan <ruansy.fnst@cn.fujitsu.com>
> ---
> Documentation/nvdimm/nvdimm.txt | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/nvdimm/nvdimm.txt b/Documentation/nvdimm/nvdimm.txt
> index e894de69915a..1669f626b037 100644
> --- a/Documentation/nvdimm/nvdimm.txt
> +++ b/Documentation/nvdimm/nvdimm.txt
> @@ -284,8 +284,8 @@ A bus has a 1:1 relationship with an NFIT. The current expectation for
> ACPI based systems is that there is only ever one platform-global NFIT.
> That said, it is trivial to register multiple NFITs, the specification
> does not preclude it. The infrastructure supports multiple busses and
> -we we use this capability to test multiple NFIT configurations in the
> -unit test.
> +we use this capability to test multiple NFIT configurations in the unit
> +test.
Applied, thanks.
I note this has languished for a bit; please don't hesitate to ping after
a week or so if you don't get a response on a patch posting.
jon
^ permalink raw reply
* Re: [PATCH] linux: README: reduced README size by 1 byte by removing unnecessary space character
From: Markus Heiser @ 2019-06-07 17:43 UTC (permalink / raw)
To: Alex, paulmck; +Cc: linux-doc, linux-kernel, trivial, Aaron A Montoya
In-Reply-To: <20190607055917.17040-1-awaitingvictory@gmail.com>
Am 07.06.19 um 07:59 schrieb Alex:
> From: Aaron A Montoya <aaron.a.montoya@gmail.com>
>
> On line 9 of the README there is an unnecessary extra space character,
> after the period, that adds 1 byte of size to the file. By removing the
> unnecessary space, Linux downloads will be 1 byte smaller and therefor be
> faster to download and take up less space on a user's system, all while
> correcting a sentence structure issue. This change is one of the few
> optimizations with practically no downsides, besides taking time out
> of the hardworking Linux maintainers day to implement the change.
> Remove extra space after the period on line 9 of the README.
> Commit 0619770a02a30834 ("Removed unnecessary space character from README")
>
best joke today but wrong in two ways ;)
1. size matter? The commit (-message) adds to more bytes to the repo ;)
2. Be correct formated? The extra Space is for 'sentence spacing' [1]
e.g. emacs text modes are formating such spacing.
[1] https://en.wikipedia.org/wiki/Sentence_spacing
-- Markus --
^ permalink raw reply
* Re: [PATCH v7 03/14] x86/cet/ibt: Add IBT legacy code bitmap setup function
From: Peter Zijlstra @ 2019-06-07 17:43 UTC (permalink / raw)
To: Yu-cheng Yu
Cc: x86, H. Peter Anvin, Thomas Gleixner, Ingo Molnar, linux-kernel,
linux-doc, linux-mm, linux-arch, linux-api, Arnd Bergmann,
Andy Lutomirski, Balbir Singh, Borislav Petkov, Cyrill Gorcunov,
Dave Hansen, Eugene Syromiatnikov, Florian Weimer, H.J. Lu,
Jann Horn, Jonathan Corbet, Kees Cook, Mike Kravetz, Nadav Amit,
Oleg Nesterov, Pavel Machek, Randy Dunlap, Ravi V. Shankar,
Vedvyas Shanbhogue, Dave Martin
In-Reply-To: <aa8a92ef231d512b5c9855ef416db050b5ab59a6.camel@intel.com>
On Fri, Jun 07, 2019 at 09:23:43AM -0700, Yu-cheng Yu wrote:
> On Fri, 2019-06-07 at 10:08 +0200, Peter Zijlstra wrote:
> > On Thu, Jun 06, 2019 at 01:09:15PM -0700, Yu-cheng Yu wrote:
> > > Indirect Branch Tracking (IBT) provides an optional legacy code bitmap
> > > that allows execution of legacy, non-IBT compatible library by an
> > > IBT-enabled application. When set, each bit in the bitmap indicates
> > > one page of legacy code.
> > >
> > > The bitmap is allocated and setup from the application.
> > > +int cet_setup_ibt_bitmap(unsigned long bitmap, unsigned long size)
> > > +{
> > > + u64 r;
> > > +
> > > + if (!current->thread.cet.ibt_enabled)
> > > + return -EINVAL;
> > > +
> > > + if (!PAGE_ALIGNED(bitmap) || (size > TASK_SIZE_MAX))
> > > + return -EINVAL;
> > > +
> > > + current->thread.cet.ibt_bitmap_addr = bitmap;
> > > + current->thread.cet.ibt_bitmap_size = size;
> > > +
> > > + /*
> > > + * Turn on IBT legacy bitmap.
> > > + */
> > > + modify_fpu_regs_begin();
> > > + rdmsrl(MSR_IA32_U_CET, r);
> > > + r |= (MSR_IA32_CET_LEG_IW_EN | bitmap);
> > > + wrmsrl(MSR_IA32_U_CET, r);
> > > + modify_fpu_regs_end();
> > > +
> > > + return 0;
> > > +}
> >
> > So you just program a random user supplied address into the hardware.
> > What happens if there's not actually anything at that address or the
> > user munmap()s the data after doing this?
>
> This function checks the bitmap's alignment and size, and anything else is the
> app's responsibility. What else do you think the kernel should check?
I've no idea what the kernel should do; since you failed to answer the
question what happens when you point this to garbage.
Does it then fault or what?
^ permalink raw reply
* Re: [PATCH] Documentation: xfs: Fix typo
From: Jonathan Corbet @ 2019-06-07 17:44 UTC (permalink / raw)
To: Shiyang Ruan; +Cc: linux-doc, linux-kernel, linux-xfs
In-Reply-To: <20190509030549.2253-1-ruansy.fnst@cn.fujitsu.com>
On Thu, 9 May 2019 11:05:49 +0800
Shiyang Ruan <ruansy.fnst@cn.fujitsu.com> wrote:
> In "Y+P" of this line, there are two non-ASCII characters(0xd9 0x8d)
> following behind the 'Y'. Shown as a small '=' under the '+' in VIM
> and a '賺' in webpage[1].
>
> I think it's a mistake and remove these strange characters.
>
> [1]: https://www.kernel.org/doc/Documentation/filesystems/xfs-delayed-logging-design.txt
>
> Signed-off-by: Shiyang Ruan <ruansy.fnst@cn.fujitsu.com>
> ---
> Documentation/filesystems/xfs-delayed-logging-design.txt | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/Documentation/filesystems/xfs-delayed-logging-design.txt b/Documentation/filesystems/xfs-delayed-logging-design.txt
> index 2ce36439c09f..9a6dd289b17b 100644
> --- a/Documentation/filesystems/xfs-delayed-logging-design.txt
> +++ b/Documentation/filesystems/xfs-delayed-logging-design.txt
> @@ -34,7 +34,7 @@ transaction:
> D A+B+C+D X+n+m+o
> <object written to disk>
> E E Y (> X+n+m+o)
> - F E+F Yٍ+p
> + F E+F Y+p
OK, that does look funky, applied.
This patch probably should have been copied to the XFS list (added), even
though get_maintainer.pl doesn't know that.
Thanks,
jon
^ permalink raw reply
* Re: [PATCH 2/3] treewide: trivial: fix s/poped/popped/ typo
From: Jonathan Corbet @ 2019-06-07 17:50 UTC (permalink / raw)
To: George G. Davis
Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, H. Peter Anvin,
maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT), Jiri Kosina,
Masami Hiramatsu, Steven Rostedt, Andi Kleen, Jann Horn,
Nadav Amit, open list:DOCUMENTATION, open list
In-Reply-To: <1559766612-12178-2-git-send-email-george_davis@mentor.com>
On Wed, 5 Jun 2019 16:30:10 -0400
"George G. Davis" <george_davis@mentor.com> wrote:
> Fix a couple of s/poped/popped/ typos.
>
> Cc: Jiri Kosina <trivial@kernel.org>
> Signed-off-by: George G. Davis <george_davis@mentor.com>
I've applied this one, thanks.
jon
^ permalink raw reply
* Re: [PATCH v2 00/22] Some documentation fixes
From: Jonathan Corbet @ 2019-06-07 17:55 UTC (permalink / raw)
To: Mauro Carvalho Chehab
Cc: Linux Doc Mailing List, Mauro Carvalho Chehab, linux-kernel,
Alexei Starovoitov, Daniel Borkmann, Martin KaFai Lau, Song Liu,
Yonghong Song, netdev, bpf
In-Reply-To: <cover.1559656538.git.mchehab+samsung@kernel.org>
On Tue, 4 Jun 2019 11:17:34 -0300
Mauro Carvalho Chehab <mchehab+samsung@kernel.org> wrote:
> Fix several warnings and broken links.
>
> This series was generated against linux-next, but was rebased to be applied at
> docs-next. It should apply cleanly on either tree.
>
> There's a git tree with all of them applied on the top of docs/docs-next
> at:
>
> https://git.linuxtv.org/mchehab/experimental.git/log/?h=fix_doc_links_v2
So I'll admit I've kind of lost track of which of these are applied, which
have comments, etc. When you feel things have settled, can you get me an
updated set and I'll get them applied?
Thanks,
jon
^ permalink raw reply
* Re: [PATCH] Documentation: {u,k}probes: add tracing_on before tracing
From: Jonathan Corbet @ 2019-06-07 17:57 UTC (permalink / raw)
To: lecopzer.chen
Cc: linux-kernel, linux-doc, mhiramat, srv_heupstream, yj.chiang
In-Reply-To: <1557397876-18153-1-git-send-email-lecopzer.chen@mediatek.com>
On Thu, 9 May 2019 18:31:16 +0800
<lecopzer.chen@mediatek.com> wrote:
> From: Lecopzer Chen <lecopzer.chen@mediatek.com>
>
> After following the document step by step, the `cat trace` can't be
> worked without enabling tracing_on and might mislead newbies about
> the functionality.
>
> Signed-off-by: Lecopzer Chen <lecopzer.chen@mediatek.com>
Applied, thanks.
jon
^ permalink raw reply
* Re: [PATCH v7 03/14] x86/cet/ibt: Add IBT legacy code bitmap setup function
From: Dave Hansen @ 2019-06-07 17:59 UTC (permalink / raw)
To: Peter Zijlstra, Yu-cheng Yu
Cc: x86, H. Peter Anvin, Thomas Gleixner, Ingo Molnar, linux-kernel,
linux-doc, linux-mm, linux-arch, linux-api, Arnd Bergmann,
Andy Lutomirski, Balbir Singh, Borislav Petkov, Cyrill Gorcunov,
Dave Hansen, Eugene Syromiatnikov, Florian Weimer, H.J. Lu,
Jann Horn, Jonathan Corbet, Kees Cook, Mike Kravetz, Nadav Amit,
Oleg Nesterov, Pavel Machek, Randy Dunlap, Ravi V. Shankar,
Vedvyas Shanbhogue, Dave Martin
In-Reply-To: <20190607174336.GM3436@hirez.programming.kicks-ass.net>
On 6/7/19 10:43 AM, Peter Zijlstra wrote:
> I've no idea what the kernel should do; since you failed to answer the
> question what happens when you point this to garbage.
>
> Does it then fault or what?
Yeah, I think you'll fault with a rather mysterious CR2 value since
you'll go look at the instruction that faulted and not see any
references to the CR2 value.
I think this new MSR probably needs to get included in oops output when
CET is enabled.
Why don't we require that a VMA be in place for the entire bitmap?
Don't we need a "get" prctl function too in case something like a JIT is
running and needs to find the location of this bitmap to set bits itself?
Or, do we just go whole-hog and have the kernel manage the bitmap
itself. Our interface here could be:
prctl(PR_MARK_CODE_AS_LEGACY, start, size);
and then have the kernel allocate and set the bitmap for those code
locations.
^ permalink raw reply
* Re: [PATCH v7 22/27] binfmt_elf: Extract .note.gnu.property from an ELF file
From: Dave Martin @ 2019-06-07 18:01 UTC (permalink / raw)
To: Yu-cheng Yu
Cc: x86, H. Peter Anvin, Thomas Gleixner, Ingo Molnar, linux-kernel,
linux-doc, linux-mm, linux-arch, linux-api, Arnd Bergmann,
Andy Lutomirski, Balbir Singh, Borislav Petkov, Cyrill Gorcunov,
Dave Hansen, Eugene Syromiatnikov, Florian Weimer, H.J. Lu,
Jann Horn, Jonathan Corbet, Kees Cook, Mike Kravetz, Nadav Amit,
Oleg Nesterov, Pavel Machek, Peter Zijlstra, Randy Dunlap,
Ravi V. Shankar, Vedvyas Shanbhogue
In-Reply-To: <20190606200646.3951-23-yu-cheng.yu@intel.com>
On Thu, Jun 06, 2019 at 01:06:41PM -0700, Yu-cheng Yu wrote:
> An ELF file's .note.gnu.property indicates features the executable file
> can support. For example, the property GNU_PROPERTY_X86_FEATURE_1_AND
> indicates the file supports GNU_PROPERTY_X86_FEATURE_1_IBT and/or
> GNU_PROPERTY_X86_FEATURE_1_SHSTK.
>
> With this patch, if an arch needs to setup features from ELF properties,
> it needs CONFIG_ARCH_USE_GNU_PROPERTY to be set, and a specific
> arch_setup_property().
>
> For example, for X86_64:
>
> int arch_setup_property(void *ehdr, void *phdr, struct file *f, bool inter)
> {
> int r;
> uint32_t property;
>
> r = get_gnu_property(ehdr, phdr, f, GNU_PROPERTY_X86_FEATURE_1_AND,
> &property);
> ...
> }
Although this code works for the simple case, I have some concerns about
some aspects of the implementation here. There appear to be some bounds
checking / buffer overrun issues, and the code seems quite complex.
Maybe this patch tries too hard to be compatible with toolchains that do
silly things such as embedding huge notes in an executable, or mixing
NT_GNU_PROPERTY_TYPE_0 in a single PT_NOTE with a load of junk not
relevant to the loader. I wonder whether Linux can dictate what
interpretation(s) of the ELF specs it is prepared to support, rather than
trying to support absolutely anything.
I've commented on some potential issues below, but my review isn't
exhaustive -- I may also have simply not understood the code in some
cases, so I apologise in advance for that!
I've also marked a few coding style nits that make the code harder to
read than necessary (but this is partly a matter of taste).
Comments below.
Cheers
---Dave
>
> Signed-off-by: H.J. Lu <hjl.tools@gmail.com>
> Signed-off-by: Yu-cheng Yu <yu-cheng.yu@intel.com>
> ---
> fs/Kconfig.binfmt | 3 +
> fs/Makefile | 1 +
> fs/binfmt_elf.c | 13 ++
> fs/gnu_property.c | 351 +++++++++++++++++++++++++++++++++++++++
> include/linux/elf.h | 12 ++
> include/uapi/linux/elf.h | 14 ++
> 6 files changed, 394 insertions(+)
> create mode 100644 fs/gnu_property.c
>
> diff --git a/fs/Kconfig.binfmt b/fs/Kconfig.binfmt
> index f87ddd1b6d72..397138ab305b 100644
> --- a/fs/Kconfig.binfmt
> +++ b/fs/Kconfig.binfmt
> @@ -36,6 +36,9 @@ config COMPAT_BINFMT_ELF
> config ARCH_BINFMT_ELF_STATE
> bool
>
> +config ARCH_USE_GNU_PROPERTY
> + bool
> +
> config BINFMT_ELF_FDPIC
> bool "Kernel support for FDPIC ELF binaries"
> default y if !BINFMT_ELF
> diff --git a/fs/Makefile b/fs/Makefile
> index c9aea23aba56..b69f18c14e09 100644
> --- a/fs/Makefile
> +++ b/fs/Makefile
> @@ -44,6 +44,7 @@ obj-$(CONFIG_BINFMT_ELF) += binfmt_elf.o
> obj-$(CONFIG_COMPAT_BINFMT_ELF) += compat_binfmt_elf.o
> obj-$(CONFIG_BINFMT_ELF_FDPIC) += binfmt_elf_fdpic.o
> obj-$(CONFIG_BINFMT_FLAT) += binfmt_flat.o
> +obj-$(CONFIG_ARCH_USE_GNU_PROPERTY) += gnu_property.o
>
> obj-$(CONFIG_FS_MBCACHE) += mbcache.o
> obj-$(CONFIG_FS_POSIX_ACL) += posix_acl.o
> diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c
> index 8264b468f283..c3ea73787e93 100644
> --- a/fs/binfmt_elf.c
> +++ b/fs/binfmt_elf.c
> @@ -1080,6 +1080,19 @@ static int load_elf_binary(struct linux_binprm *bprm)
> goto out_free_dentry;
> }
>
> + if (interpreter) {
> + retval = arch_setup_property(&loc->interp_elf_ex,
> + interp_elf_phdata,
> + interpreter, true);
> + } else {
> + retval = arch_setup_property(&loc->elf_ex,
> + elf_phdata,
> + bprm->file, false);
> + }
> +
> + if (retval < 0)
> + goto out_free_dentry;
> +
> if (interpreter) {
> unsigned long interp_map_addr = 0;
>
> diff --git a/fs/gnu_property.c b/fs/gnu_property.c
> new file mode 100644
> index 000000000000..9c4d1d5ebf00
> --- /dev/null
> +++ b/fs/gnu_property.c
> @@ -0,0 +1,351 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * Extract an ELF file's .note.gnu.property.
> + *
> + * The path from the ELF header to the note section is the following:
> + * elfhdr->elf_phdr->elf_note->property[].
> + */
> +
> +#include <uapi/linux/elf-em.h>
> +#include <linux/processor.h>
> +#include <linux/binfmts.h>
> +#include <linux/elf.h>
> +#include <linux/slab.h>
> +#include <linux/fs.h>
> +#include <linux/uaccess.h>
> +#include <linux/string.h>
> +#include <linux/compat.h>
> +
> +/*
> + * The .note.gnu.property layout:
> + *
> + * struct elf_note {
> + * u32 n_namesz; --> sizeof(n_name[]); always (4)
> + * u32 n_ndescsz;--> sizeof(property[])
> + * u32 n_type; --> always NT_GNU_PROPERTY_TYPE_0
> + * };
> + * char n_name[4]; --> always 'GNU\0'
> + *
> + * struct {
> + * struct gnu_property {
> + * u32 pr_type;
> + * u32 pr_datasz;
> + * };
> + * u8 pr_data[pr_datasz];
> + * }[];
> + */
> +
> +#define BUF_SIZE (PAGE_SIZE / 4)
Nit: magic number in disguise. What does the size of ELF notes have
to do with the page size?
> +
> +typedef bool (test_item_fn)(void *buf, u32 *arg, u32 type);
> +typedef void *(next_item_fn)(void *buf, u32 *arg, u32 type);
> +
> +static inline bool test_note_type(void *buf, u32 *align, u32 note_type)
> +{
> + struct elf_note *n = buf;
> +
> + return ((n->n_type == note_type) && (n->n_namesz == 4) &&
> + (memcmp(n + 1, "GNU", 4) == 0));
> +}
> +
> +static inline void *next_note(void *buf, u32 *align, u32 note_type)
> +{
> + struct elf_note *n = buf;
> + u64 size;
> +
> + if (check_add_overflow((u64)sizeof(*n), (u64)n->n_namesz, &size))
> + return NULL;
sizeof(*n) is a small integer under our control, and n->n_namesz is a
u32.
So, I'm not sure how we would overflow 64 bits here, although if we can
get arbitrarily close to ~(u64)0 then:
> +
> + size = round_up(size, *align);
this can overflow too.
> +
> + if (check_add_overflow(size, (u64)n->n_descsz, &size))
> + return NULL;
> +
> + size = round_up(size, *align);
Similarly here.
> +
> + if (buf + size < buf)
Isn't this undefined behaviour of it overflows? If so, the compiler can
probably delete the check entirely, making it useless. Does UBSAN warn
about it?
> + return NULL;
> + else
> + return (buf + size);
Nit: Unnecessary () (There are surplus () all over this patch; I won't
comment on them all.)
> +}
> +
> +static inline bool test_property(void *buf, u32 *max_type, u32 pr_type)
> +{
> + struct gnu_property *pr = buf;
> +
> + /*
> + * Property types must be in ascending order.
> + * Keep track of the max when testing each.
> + */
> + if (pr->pr_type > *max_type)
> + *max_type = pr->pr_type;
Is this worthwhile? In general we don't try very hard to check that the
ELF file is well-formed.
Ideally we could search by binary chop, but the property size is
variable, so the sortedness is useless to us (yay).
> +
> + return (pr->pr_type == pr_type);
> +}
> +
> +static inline void *next_property(void *buf, u32 *max_type, u32 pr_type)
Nit: does this need to be inline? The compiler's guess is usually good
enough...
> +{
> + struct gnu_property *pr = buf;
> +
> + if ((buf + sizeof(*pr) + pr->pr_datasz < buf) ||
Nit: random extra space, redundant (), etc.
> + (pr->pr_type > pr_type) ||
> + (pr->pr_type > *max_type))
> + return NULL;
> + else
> + return (buf + sizeof(*pr) + pr->pr_datasz);
We can exceed the underlying buffer bounds here, which is technically
undefined behaviour.
I suspect we may be relying on similar tricks all over the kernel, but
IT MAy be best avoided anyway.
If we always pass in the buffer base pointer and the size of the buffer, say
static int next_property(void *buf, size_t *offset,
size_t bufsz, ...)
then we may be able to use direct comparisons that can't overflow
rather than relying on potentially undefined behaviour. For example:
size_t o = *offset;
if (o > bufsz || sizeof (*pr) > bufsz - o)
return -1;
pr = buf + o;
if (pr->pr_type > pr_type || pr->pr_type > *max_type)
return -1;
if (pr->pr_datasz > bufsz - o - sizeof (*pr))
return -1;
*offset = o + sizeof (*pr) + pr->pr_datasz;
return 0;
(There may be neater ways to do this.)
> +}
> +
> +/*
> + * Scan 'buf' for a pattern; return true if found.
> + * *pos is the distance from the beginning of buf to where
> + * the searched item or the next item is located.
> + */
> +static int scan(u8 *buf, u32 buf_size, int item_size, test_item_fn test_item,
> + next_item_fn next_item, u32 *arg, u32 type, u32 *pos)
> +{
> + int found = 0;
> + u8 *p, *max;
> +
> + max = buf + buf_size;
> + if (max < buf)
See comment about undefined behaviour above.
Also, I'm not sure this check adds anything. We know buf_size is
<= BUF_SIZE (though we could stick a WARN_ON() here and bail out if we
want to make absolutely sure).
If buf is always the base pointer returned by kmalloc(BUF_SIZE), then
I think buf_size can never go outside its bounds?
> + return 0;
> +
> + p = buf;
> +
> + while ((p + item_size < max) && (p + item_size > buf)) {
^ <= ? ^ undefined behaviour?
> + if (test_item(p, arg, type)) {
> + found = 1;
> + break;
> + }
> +
> + p = next_item(p, arg, type);
> + }
> +
> + *pos = (p + item_size <= buf) ? 0 : (u32)(p - buf);
Can this be written more simply, say:
if (p + item_size > buf)
*pos += p - buf;
Also, since next_property() adds pr_datasz onto buf, could we get
unlucky and wrap past (void *)~0UL? Then (u32)(p - buf) may be giant.
Not sure whether this breaks code elsewhere.
> + return found;
> +}
> +
> +/*
> + * Search an NT_GNU_PROPERTY_TYPE_0 for the property of 'pr_type'.
> + */
> +static int find_property(struct file *file, unsigned long desc_size,
> + loff_t file_offset, u8 *buf,
> + u32 pr_type, u32 *property)
> +{
> + u32 buf_pos;
> + unsigned long read_size;
> + unsigned long done;
> + int found = 0;
> + int ret = 0;
> + u32 last_pr = 0;
> +
> + *property = 0;
> + buf_pos = 0;
> +
> + for (done = 0; done < desc_size; done += buf_pos) {
> + read_size = desc_size - done;
> + if (read_size > BUF_SIZE)
> + read_size = BUF_SIZE;
> +
> + ret = kernel_read(file, buf, read_size, &file_offset);
> +
> + if (ret != read_size)
> + return (ret < 0) ? ret : -EIO;
> +
> + ret = 0;
> + found = scan(buf, read_size, sizeof(struct gnu_property),
> + test_property, next_property,
> + &last_pr, pr_type, &buf_pos);
> +
> + if ((!buf_pos) || found)
> + break;
> +
> + file_offset += buf_pos - read_size;
> + }
> +
> + if (found) {
> + struct gnu_property *pr =
> + (struct gnu_property *)(buf + buf_pos);
> +
> + if (pr->pr_datasz == 4) {
> + u32 *max = (u32 *)(buf + read_size);
> + u32 *data = (u32 *)((u8 *)pr + sizeof(*pr));
> +
> + if (data + 1 <= max) {
> + *property = *data;
> + } else {
> + file_offset += buf_pos - read_size;
> + file_offset += sizeof(*pr);
> + ret = kernel_read(file, property, 4,
> + &file_offset);
> + }
> + }
> + }
> +
> + return ret;
> +}
> +
> +/*
> + * Search a PT_NOTE segment for NT_GNU_PROPERTY_TYPE_0.
> + */
> +static int find_note_type_0(struct file *file, loff_t file_offset,
> + unsigned long note_size, u32 align,
> + u32 pr_type, u32 *property)
> +{
> + u8 *buf;
> + u32 buf_pos;
> + unsigned long read_size;
> + unsigned long done;
> + int found = 0;
> + int ret = 0;
> +
> + buf = kmalloc(BUF_SIZE, GFP_KERNEL);
> + if (!buf)
> + return -ENOMEM;
Do we really need to alloc/free this once per note?
> +
> + *property = 0;
> + buf_pos = 0;
> +
> + for (done = 0; done < note_size; done += buf_pos) {
> + read_size = note_size - done;
> + if (read_size > BUF_SIZE)
> + read_size = BUF_SIZE;
> +
> + ret = kernel_read(file, buf, read_size, &file_offset);
> +
> + if (ret != read_size) {
> + ret = (ret < 0) ? ret : -EIO;
> + kfree(buf);
> + return ret;
> + }
> +
> + /*
> + * item_size = sizeof(struct elf_note) + elf_note.n_namesz.
> + * n_namesz is 4 for the note type we look for.
> + */
> + ret = scan(buf, read_size, sizeof(struct elf_note) + 4,
> + test_note_type, next_note,
> + &align, NT_GNU_PROPERTY_TYPE_0, &buf_pos);
> +
> + file_offset += buf_pos - read_size;
> +
> + if (ret && !found) {
> + struct elf_note *n =
> + (struct elf_note *)(buf + buf_pos);
> + u64 start = round_up(sizeof(*n) + n->n_namesz, align);
> + u64 total = 0;
> +
> + if (check_add_overflow(start, (u64)n->n_descsz, &total)) {
> + ret = -EINVAL;
> + break;
> + }
> + total = round_up(total, align);
> +
> + ret = find_property(file, n->n_descsz,
> + file_offset + start,
> + buf, pr_type, property);
> + found++;
> + file_offset += total;
> + buf_pos += total;
> + } else if (!buf_pos || ret) {
> + ret = 0;
> + *property = 0;
> + break;
> + }
> + }
Do we really need this complexity? How big are the notes realistically
going to be?
Since a file with bloated notes is going to be inefficient to exec
anyway if we have to scan all the way through them, would it be better
just to choke on it and force the toolchain to do something more
sensible?
This in one reason why it would be good for the kernel to require
PT_GNU_PROPERTY if possible, so we know the precise offset and size
without having to search...
> +
> + kfree(buf);
> + return ret;
> +}
> +
> +/*
> + * Look at an ELF file's PT_NOTE segments, then NT_GNU_PROPERTY_TYPE_0, then
> + * the property of pr_type.
> + *
> + * Input:
> + * file: the file to search;
> + * phdr: the file's elf header;
> + * phnum: number of entries in phdr;
> + * pr_type: the property type.
> + *
> + * Output:
> + * The property found.
> + *
> + * Return:
> + * Zero or error.
> + */
> +static int scan_segments_64(struct file *file, struct elf64_phdr *phdr,
> + int phnum, u32 pr_type, u32 *property)
> +{
> + int i;
> + int err = 0;
> +
> + for (i = 0; i < phnum; i++, phdr++) {
> + if ((phdr->p_type != PT_NOTE) || (phdr->p_align != 8))
> + continue;
> +
> + /*
> + * Search the PT_NOTE segment for NT_GNU_PROPERTY_TYPE_0.
> + */
> + err = find_note_type_0(file, phdr->p_offset, phdr->p_filesz,
> + phdr->p_align, pr_type, property);
> + if (err)
> + return err;
> + }
> +
> + return 0;
> +}
> +
> +static int scan_segments_32(struct file *file, struct elf32_phdr *phdr,
> + int phnum, u32 pr_type, u32 *property)
> +{
> + int i;
> + int err = 0;
> +
> + for (i = 0; i < phnum; i++, phdr++) {
> + if ((phdr->p_type != PT_NOTE) || (phdr->p_align != 4))
> + continue;
> +
> + /*
> + * Search the PT_NOTE segment for NT_GNU_PROPERTY_TYPE_0.
> + */
> + err = find_note_type_0(file, phdr->p_offset, phdr->p_filesz,
> + phdr->p_align, pr_type, property);
> + if (err)
> + return err;
> + }
> +
> + return 0;
> +}
> +
> +int get_gnu_property(void *ehdr_p, void *phdr_p, struct file *f,
> + u32 pr_type, u32 *property)
> +{
> + struct elf64_hdr *ehdr64 = ehdr_p;
> + int err = 0;
> +
> + *property = 0;
> +
> + if (ehdr64->e_ident[EI_CLASS] == ELFCLASS64) {
> + struct elf64_phdr *phdr64 = phdr_p;
> +
> + err = scan_segments_64(f, phdr64, ehdr64->e_phnum,
> + pr_type, property);
> + if (err < 0)
> + goto out;
> + } else {
> + struct elf32_hdr *ehdr32 = ehdr_p;
> +
> + if (ehdr32->e_ident[EI_CLASS] == ELFCLASS32) {
> + struct elf32_phdr *phdr32 = phdr_p;
> +
> + err = scan_segments_32(f, phdr32, ehdr32->e_phnum,
> + pr_type, property);
> + if (err < 0)
> + goto out;
> + }
> + }
> +
> +out:
> + return err;
> +}
> diff --git a/include/linux/elf.h b/include/linux/elf.h
> index e3649b3e970e..c15febebe7f2 100644
> --- a/include/linux/elf.h
> +++ b/include/linux/elf.h
> @@ -56,4 +56,16 @@ static inline int elf_coredump_extra_notes_write(struct coredump_params *cprm) {
> extern int elf_coredump_extra_notes_size(void);
> extern int elf_coredump_extra_notes_write(struct coredump_params *cprm);
> #endif
> +
> +#ifdef CONFIG_ARCH_USE_GNU_PROPERTY
> +extern int arch_setup_property(void *ehdr, void *phdr, struct file *f,
> + bool interp);
> +extern int get_gnu_property(void *ehdr_p, void *phdr_p, struct file *f,
> + u32 pr_type, u32 *feature);
> +#else
> +static inline int arch_setup_property(void *ehdr, void *phdr, struct file *f,
> + bool interp) { return 0; }
> +static inline int get_gnu_property(void *ehdr_p, void *phdr_p, struct file *f,
> + u32 pr_type, u32 *feature) { return 0; }
> +#endif
> #endif /* _LINUX_ELF_H */
> diff --git a/include/uapi/linux/elf.h b/include/uapi/linux/elf.h
> index 34c02e4290fe..316177ce9e76 100644
> --- a/include/uapi/linux/elf.h
> +++ b/include/uapi/linux/elf.h
> @@ -372,6 +372,7 @@ typedef struct elf64_shdr {
> #define NT_PRFPREG 2
> #define NT_PRPSINFO 3
> #define NT_TASKSTRUCT 4
> +#define NT_GNU_PROPERTY_TYPE_0 5
Should this be in a separate block. This required n_name = "GNU",
whereas the rest are "LINUX" notes AFAIK: it's really a separate
namespace.
I think the gap between 4 and 6 may be just coincidence: glibc's elf.h
already has NT_PLATFORM here (whatever that is).
> #define NT_AUXV 6
> /*
> * Note to userspace developers: size of NT_SIGINFO note may increase
> @@ -443,4 +444,17 @@ typedef struct elf64_note {
> Elf64_Word n_type; /* Content type */
> } Elf64_Nhdr;
>
> +/* NT_GNU_PROPERTY_TYPE_0 header */
> +struct gnu_property {
> + __u32 pr_type;
> + __u32 pr_datasz;
> +};
> +
> +/* .note.gnu.property types */
> +#define GNU_PROPERTY_X86_FEATURE_1_AND (0xc0000002)
> +
> +/* Bits of GNU_PROPERTY_X86_FEATURE_1_AND */
> +#define GNU_PROPERTY_X86_FEATURE_1_IBT (0x00000001)
> +#define GNU_PROPERTY_X86_FEATURE_1_SHSTK (0x00000002)
> +
Redundant (). The rest of the file doesn't have them; can we conform to
the prevailing style there?
> #endif /* _UAPI_LINUX_ELF_H */
> --
> 2.17.1
>
^ permalink raw reply
* Re: [PATCH v7 03/14] x86/cet/ibt: Add IBT legacy code bitmap setup function
From: Andy Lutomirski @ 2019-06-07 18:29 UTC (permalink / raw)
To: Dave Hansen
Cc: Peter Zijlstra, Yu-cheng Yu, x86, H. Peter Anvin, Thomas Gleixner,
Ingo Molnar, linux-kernel, linux-doc, linux-mm, linux-arch,
linux-api, Arnd Bergmann, Balbir Singh, Borislav Petkov,
Cyrill Gorcunov, Dave Hansen, Eugene Syromiatnikov,
Florian Weimer, H.J. Lu, Jann Horn, Jonathan Corbet, Kees Cook,
Mike Kravetz, Nadav Amit, Oleg Nesterov, Pavel Machek,
Randy Dunlap, Ravi V. Shankar, Vedvyas Shanbhogue, Dave Martin
In-Reply-To: <b3de4110-5366-fdc7-a960-71dea543a42f@intel.com>
> On Jun 7, 2019, at 10:59 AM, Dave Hansen <dave.hansen@intel.com> wrote:
>
>> On 6/7/19 10:43 AM, Peter Zijlstra wrote:
>> I've no idea what the kernel should do; since you failed to answer the
>> question what happens when you point this to garbage.
>>
>> Does it then fault or what?
>
> Yeah, I think you'll fault with a rather mysterious CR2 value since
> you'll go look at the instruction that faulted and not see any
> references to the CR2 value.
>
> I think this new MSR probably needs to get included in oops output when
> CET is enabled.
This shouldn’t be able to OOPS because it only happens at CPL 3, right? We should put it into core dumps, though.
>
> Why don't we require that a VMA be in place for the entire bitmap?
> Don't we need a "get" prctl function too in case something like a JIT is
> running and needs to find the location of this bitmap to set bits itself?
>
> Or, do we just go whole-hog and have the kernel manage the bitmap
> itself. Our interface here could be:
>
> prctl(PR_MARK_CODE_AS_LEGACY, start, size);
>
> and then have the kernel allocate and set the bitmap for those code
> locations.
Given that the format depends on the VA size, this might be a good idea. I bet we can reuse the special mapping infrastructure for this — the VMA could
be a MAP_PRIVATE special mapping named [cet_legacy_bitmap] or similar, and we can even make special rules to core dump it intelligently if needed. And we can make mremap() on it work correctly if anyone (CRIU?) cares.
Hmm. Can we be creative and skip populating it with zeros? The CPU should only ever touch a page if we miss an ENDBR on it, so, in normal operation, we don’t need anything to be there. We could try to prevent anyone from *reading* it outside of ENDBR tracking if we want to avoid people accidentally wasting lots of memory by forcing it to be fully populated when the read it.
The one downside is this forces it to be per-mm, but that seems like a generally reasonable model anyway.
This also gives us an excellent opportunity to make it read-only as seen from userspace to prevent exploits from just poking it full of ones before redirecting execution.
^ permalink raw reply
* Re: [PATCH] Documentation: xfs: Fix typo
From: Darrick J. Wong @ 2019-06-07 18:34 UTC (permalink / raw)
To: Jonathan Corbet; +Cc: Shiyang Ruan, linux-doc, linux-kernel, linux-xfs
In-Reply-To: <20190607114415.32cb32dd@lwn.net>
On Fri, Jun 07, 2019 at 11:44:15AM -0600, Jonathan Corbet wrote:
> On Thu, 9 May 2019 11:05:49 +0800
> Shiyang Ruan <ruansy.fnst@cn.fujitsu.com> wrote:
>
> > In "Y+P" of this line, there are two non-ASCII characters(0xd9 0x8d)
> > following behind the 'Y'. Shown as a small '=' under the '+' in VIM
> > and a '賺' in webpage[1].
> >
> > I think it's a mistake and remove these strange characters.
> >
> > [1]: https://www.kernel.org/doc/Documentation/filesystems/xfs-delayed-logging-design.txt
> >
> > Signed-off-by: Shiyang Ruan <ruansy.fnst@cn.fujitsu.com>
> > ---
> > Documentation/filesystems/xfs-delayed-logging-design.txt | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/Documentation/filesystems/xfs-delayed-logging-design.txt b/Documentation/filesystems/xfs-delayed-logging-design.txt
> > index 2ce36439c09f..9a6dd289b17b 100644
> > --- a/Documentation/filesystems/xfs-delayed-logging-design.txt
> > +++ b/Documentation/filesystems/xfs-delayed-logging-design.txt
> > @@ -34,7 +34,7 @@ transaction:
> > D A+B+C+D X+n+m+o
> > <object written to disk>
> > E E Y (> X+n+m+o)
> > - F E+F Yٍ+p
> > + F E+F Y+p
>
> OK, that does look funky, applied.
>
> This patch probably should have been copied to the XFS list (added), even
> though get_maintainer.pl doesn't know that.
Yeah, it's "Y+p" not "Y<weird plusequals thing>p" in the xfs
documentation repo:
https://git.kernel.org/pub/scm/fs/xfs/xfs-documentation.git/tree/design/XFS_Filesystem_Structure/delayed_logging.asciidoc
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
I doubt the value of maintaining duplicate copies of this document in
the kernel and the xfs documentation repo, and since the xfs docs and
kernel licences aren't compatible maybe we should withdraw one...
...but since Dave is the author I'm gonna punt to him. IMHO either we
should claim responsibility for those files in MAINTAINERS or drop them.
:)
Thanks for the heads-up,
--D
> Thanks,
>
> jon
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox