* Re: [PATCH v4 17/18] kernel/sysctl-test: Add null pointer test for sysctl.c:proc_dointvec()
From: Brendan Higgins @ 2019-06-11 17:58 UTC (permalink / raw)
To: Stephen Boyd
Cc: Iurii Zaikin, frowand.list, gregkh, jpoimboe, keescook,
kieran.bingham, mcgrof, peterz, robh, shuah, tytso,
yamada.masahiro, devicetree, dri-devel, kunit-dev, linux-doc,
linux-fsdevel, linux-kbuild, linux-kernel, linux-kselftest,
linux-nvdimm, linux-um, Alexander.Levin, Tim.Bird, amir73il,
dan.carpenter, daniel, jdike, joel, julia.lawall, khilman,
knut.omang, logang, mpe, pmladek, rdunlap, richard, rientjes,
rostedt, wfg
In-Reply-To: <20190607190047.C3E7A20868@mail.kernel.org>
On Fri, Jun 07, 2019 at 12:00:47PM -0700, Stephen Boyd wrote:
> Quoting Iurii Zaikin (2019-06-05 18:29:42)
> > On Fri, May 17, 2019 at 11:22 AM Stephen Boyd <sboyd@kernel.org> wrote:
> > >
> > > Quoting Brendan Higgins (2019-05-14 15:17:10)
> > > > diff --git a/kernel/sysctl-test.c b/kernel/sysctl-test.c
> > > > new file mode 100644
> > > > index 0000000000000..fe0f2bae66085
> > > > --- /dev/null
> > > > +++ b/kernel/sysctl-test.c
> > > > +
> > > > +
> > > > +static void sysctl_test_dointvec_happy_single_negative(struct kunit *test)
> > > > +{
> > > > + struct ctl_table table = {
> > > > + .procname = "foo",
> > > > + .data = &test_data.int_0001,
> > > > + .maxlen = sizeof(int),
> > > > + .mode = 0644,
> > > > + .proc_handler = proc_dointvec,
> > > > + .extra1 = &i_zero,
> > > > + .extra2 = &i_one_hundred,
> > > > + };
> > > > + char input[] = "-9";
> > > > + size_t len = sizeof(input) - 1;
> > > > + loff_t pos = 0;
> > > > +
> > > > + table.data = kunit_kzalloc(test, sizeof(int), GFP_USER);
> > > > + KUNIT_EXPECT_EQ(test, 0, proc_dointvec(&table, 1, input, &len, &pos));
> > > > + KUNIT_EXPECT_EQ(test, sizeof(input) - 1, len);
> > > > + KUNIT_EXPECT_EQ(test, sizeof(input) - 1, pos);
> > > > + KUNIT_EXPECT_EQ(test, -9, *(int *)table.data);
> > >
> > > Is the casting necessary? Or can the macro do a type coercion of the
> > > second parameter based on the first type?
> > Data field is defined as void* so I believe casting is necessary to
> > dereference it as a pointer to an array of ints. I don't think the
> > macro should do any type coercion that == operator wouldn't do.
> > I did change the cast to make it more clear that it's a pointer to an
> > array of ints being dereferenced.
>
> Ok, I still wonder if we should make KUNIT_EXPECT_EQ check the types on
> both sides and cause a build warning/error if the types aren't the same.
> This would be similar to our min/max macros that complain about
> mismatched types in the comparisons. Then if a test developer needs to
> convert one type or the other they could do so with a
> KUNIT_EXPECT_EQ_T() macro that lists the types to coerce both sides to
> explicitly.
Do you think it would be better to do a phony compare similar to how
min/max used to work prior to 4.17, or to use the new __typecheck(...)
macro? This might seem like a dumb question (and maybe it is), but Iurii
and I thought the former created an error message that was a bit easier
to understand, whereas __typecheck is obviously superior in terms of
code reuse.
This is what we are thinking right now; if you don't have any complaints
I will squash it into the relevant commits on the next revision:
---
From: Iurii Zaikin <yzaikin@google.com>
Adds a warning message when comparing values of different types similar
to what min() / max() macros do.
Signed-off-by: Iurii Zaikin <yzaikin@google.com>
---
include/kunit/test.h | 15 +++++++++++++++
1 file changed, 15 insertions(+)
diff --git a/include/kunit/test.h b/include/kunit/test.h
index 511c9e85401a6..791e22fba5620 100644
--- a/include/kunit/test.h
+++ b/include/kunit/test.h
@@ -335,6 +335,13 @@ void __printf(3, 4) kunit_printk(const char *level,
#define kunit_err(test, fmt, ...) \
kunit_printk(KERN_ERR, test, fmt, ##__VA_ARGS__)
+/*
+ * 'Unnecessary' cast serves to generate a compile-time warning in case
+ * of comparing incompatible types. Inspired by include/linux/kernel.h
+ */
+#define __kunit_typecheck(lhs, rhs) \
+ ((void) (&(lhs) == &(rhs)))
+
static inline struct kunit_stream *kunit_expect_start(struct kunit *test,
const char *file,
const char *line)
@@ -514,6 +521,7 @@ static inline void kunit_expect_ptr_binary(struct kunit *test,
#define KUNIT_EXPECT_BINARY(test, left, condition, right) do { \
typeof(left) __left = (left); \
typeof(right) __right = (right); \
+ __kunit_typecheck(__left, __right); \
kunit_expect_binary(test, \
(long long) __left, #left, \
(long long) __right, #right, \
@@ -524,6 +532,7 @@ static inline void kunit_expect_ptr_binary(struct kunit *test,
#define KUNIT_EXPECT_BINARY_MSG(test, left, condition, right, fmt, ...) do { \
typeof(left) __left = (left); \
typeof(right) __right = (right); \
+ __kunit_typecheck(__left, __right); \
kunit_expect_binary_msg(test, \
(long long) __left, #left, \
(long long) __right, #right, \
@@ -538,6 +547,7 @@ static inline void kunit_expect_ptr_binary(struct kunit *test,
#define KUNIT_EXPECT_PTR_BINARY(test, left, condition, right) do { \
typeof(left) __left = (left); \
typeof(right) __right = (right); \
+ __kunit_typecheck(__left, __right); \
kunit_expect_ptr_binary(test, \
(void *) __left, #left, \
(void *) __right, #right, \
@@ -553,6 +563,7 @@ static inline void kunit_expect_ptr_binary(struct kunit *test,
...) do { \
typeof(left) __left = (left); \
typeof(right) __right = (right); \
+ __kunit_typecheck(__left, __right); \
kunit_expect_ptr_binary_msg(test, \
(void *) __left, #left, \
(void *) __right, #right, \
@@ -1013,6 +1024,7 @@ static inline void kunit_assert_ptr_binary(struct kunit *test,
#define KUNIT_ASSERT_BINARY(test, left, condition, right) do { \
typeof(left) __left = (left); \
typeof(right) __right = (right); \
+ __kunit_typecheck(__left, __right); \
kunit_assert_binary(test, \
(long long) __left, #left, \
(long long) __right, #right, \
@@ -1023,6 +1035,7 @@ static inline void kunit_assert_ptr_binary(struct kunit *test,
#define KUNIT_ASSERT_BINARY_MSG(test, left, condition, right, fmt, ...) do { \
typeof(left) __left = (left); \
typeof(right) __right = (right); \
+ __kunit_typecheck(__left, __right); \
kunit_assert_binary_msg(test, \
(long long) __left, #left, \
(long long) __right, #right, \
@@ -1037,6 +1050,7 @@ static inline void kunit_assert_ptr_binary(struct kunit *test,
#define KUNIT_ASSERT_PTR_BINARY(test, left, condition, right) do { \
typeof(left) __left = (left); \
typeof(right) __right = (right); \
+ __kunit_typecheck(__left, __right); \
kunit_assert_ptr_binary(test, \
(void *) __left, #left, \
(void *) __right, #right, \
@@ -1051,6 +1065,7 @@ static inline void kunit_assert_ptr_binary(struct kunit *test,
fmt, ...) do { \
typeof(left) __left = (left); \
typeof(right) __right = (right); \
+ __kunit_typecheck(__left, __right); \
kunit_assert_ptr_binary_msg(test, \
(void *) __left, #left, \
(void *) __right, #right, \
--
2.22.0.rc2.383.gf4fbbf30c2-goog
^ permalink raw reply related
* Re: [PATCH v7 25/27] mm/mmap: Add Shadow stack pages to memory accounting
From: Dave Hansen @ 2019-06-11 17:55 UTC (permalink / raw)
To: Yu-cheng Yu, 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,
Dave Martin
In-Reply-To: <20190606200646.3951-26-yu-cheng.yu@intel.com>
On 6/6/19 1:06 PM, Yu-cheng Yu wrote:
> --- a/mm/mmap.c
> +++ b/mm/mmap.c
> @@ -1703,6 +1703,9 @@ static inline int accountable_mapping(struct file *file, vm_flags_t vm_flags)
> if (file && is_file_hugepages(file))
> return 0;
>
> + if (arch_copy_pte_mapping(vm_flags))
> + return 1;
> +
> return (vm_flags & (VM_NORESERVE | VM_SHARED | VM_WRITE)) == VM_WRITE;
> }
>
> @@ -3319,6 +3322,8 @@ void vm_stat_account(struct mm_struct *mm, vm_flags_t flags, long npages)
> mm->stack_vm += npages;
> else if (is_data_mapping(flags))
> mm->data_vm += npages;
> + else if (arch_copy_pte_mapping(flags))
> + mm->data_vm += npages;
> }
This classifies shadow stack as data instead of stack. That seems a wee
bit counterintuitive. Why did you make this choice?
^ permalink raw reply
* Re: [PATCH v3 06/20] docs: mark orphan documents as such
From: Mauro Carvalho Chehab @ 2019-06-11 17:05 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Linux Doc Mailing List, Mauro Carvalho Chehab,
Linux Kernel Mailing List, Jonathan Corbet, Frederic Barrat,
Andrew Donnellan, Maxime Coquelin, Alexandre Torgue, David Airlie,
Daniel Vetter, Maarten Lankhorst, Maxime Ripard, Sean Paul,
Georgi Djakov, Matan Ziv-Av, Benjamin Herrenschmidt,
Paul Mackerras, Michael Ellerman,
open list:LINUX FOR POWERPC PA SEMI PWRFICIENT, linux-stm32,
linux-arm Mailing List, dri-devel, Linux PM, Platform Driver
In-Reply-To: <CAHp75VfTNJOGZx-PoUXLRvzghqf6bVUdJ+yFjE9hNtDLCQ1=UA@mail.gmail.com>
Em Tue, 11 Jun 2019 19:52:04 +0300
Andy Shevchenko <andy.shevchenko@gmail.com> escreveu:
> On Fri, Jun 7, 2019 at 10:04 PM Mauro Carvalho Chehab
> <mchehab+samsung@kernel.org> wrote:
> > Sphinx doesn't like orphan documents:
>
> > Documentation/laptops/lg-laptop.rst: WARNING: document isn't included in any toctree
>
> > Documentation/laptops/lg-laptop.rst | 2 ++
>
> > diff --git a/Documentation/laptops/lg-laptop.rst b/Documentation/laptops/lg-laptop.rst
> > index aa503ee9b3bc..f2c2ffe31101 100644
> > --- a/Documentation/laptops/lg-laptop.rst
> > +++ b/Documentation/laptops/lg-laptop.rst
> > @@ -1,5 +1,7 @@
> > .. SPDX-License-Identifier: GPL-2.0+
> >
> > +:orphan:
> > +
> > LG Gram laptop extra features
> > =============================
> >
>
> Can we rather create a toc tree there?
> It was a first document in reST format in that folder.
Sure, but:
1) I have a patch converting the other files on this dir to rst:
https://git.linuxtv.org/mchehab/experimental.git/commit/?h=convert_rst_renames_v4.1&id=abc13233035fdfdbc5ef2f2fbd3d127a1ab15530
2) It probably makes sense to move the entire dir to
Documentation/admin-guide.
So, I would prefer to have the :orphan: here while (1) is not merged.
Thanks,
Mauro
^ permalink raw reply
* Re: [PATCH v3 06/20] docs: mark orphan documents as such
From: Andy Shevchenko @ 2019-06-11 16:52 UTC (permalink / raw)
To: Mauro Carvalho Chehab
Cc: Linux Doc Mailing List, Mauro Carvalho Chehab,
Linux Kernel Mailing List, Jonathan Corbet, Frederic Barrat,
Andrew Donnellan, Maxime Coquelin, Alexandre Torgue, David Airlie,
Daniel Vetter, Maarten Lankhorst, Maxime Ripard, Sean Paul,
Georgi Djakov, Matan Ziv-Av, Benjamin Herrenschmidt,
Paul Mackerras, Michael Ellerman,
open list:LINUX FOR POWERPC PA SEMI PWRFICIENT, linux-stm32,
linux-arm Mailing List, dri-devel, Linux PM, Platform Driver
In-Reply-To: <0bea1c7c4fc06c7edabbf3185c0cbbc6e85eafd0.1559933665.git.mchehab+samsung@kernel.org>
On Fri, Jun 7, 2019 at 10:04 PM Mauro Carvalho Chehab
<mchehab+samsung@kernel.org> wrote:
> Sphinx doesn't like orphan documents:
> Documentation/laptops/lg-laptop.rst: WARNING: document isn't included in any toctree
> Documentation/laptops/lg-laptop.rst | 2 ++
> diff --git a/Documentation/laptops/lg-laptop.rst b/Documentation/laptops/lg-laptop.rst
> index aa503ee9b3bc..f2c2ffe31101 100644
> --- a/Documentation/laptops/lg-laptop.rst
> +++ b/Documentation/laptops/lg-laptop.rst
> @@ -1,5 +1,7 @@
> .. SPDX-License-Identifier: GPL-2.0+
>
> +:orphan:
> +
> LG Gram laptop extra features
> =============================
>
Can we rather create a toc tree there?
It was a first document in reST format in that folder.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* [PATCH v5 05/11] drm: Add helpers to kick off self refresh mode in drivers
From: Sean Paul @ 2019-06-11 16:08 UTC (permalink / raw)
To: dri-devel
Cc: Sean Paul, Daniel Vetter, Jose Souza, Zain Wang, Tomasz Figa,
Ville Syrjälä, Heiko Stuebner, David Airlie,
Maarten Lankhorst, Maxime Ripard, Sean Paul, Jonathan Corbet,
linux-doc
In-Reply-To: <20190611160844.257498-1-sean@poorly.run>
From: Sean Paul <seanpaul@chromium.org>
This patch adds a new drm helper library to help drivers implement
self refresh. Drivers choosing to use it will register crtcs and
will receive callbacks when it's time to enter or exit self refresh
mode.
In its current form, it has a timer which will trigger after a
driver-specified amount of inactivity. When the timer triggers, the
helpers will submit a new atomic commit to shut the refreshing pipe
off. On the next atomic commit, the drm core will revert the self
refresh state and bring everything back up to be actively driven.
From the driver's perspective, this works like a regular disable/enable
cycle. The driver need only check the 'self_refresh_active' state in
crtc_state. It should initiate self refresh mode on the panel and enter
an off or low-power state.
Changes in v2:
- s/psr/self_refresh/ (Daniel)
- integrated the psr exit into the commit that wakes it up (Jose/Daniel)
- made the psr state per-crtc (Jose/Daniel)
Changes in v3:
- Remove the self_refresh_(active|changed) from connector state (Daniel)
- Simplify loop in drm_self_refresh_helper_alter_state (Daniel)
- Improve self_refresh_aware comment (Daniel)
- s/self_refresh_state/self_refresh_data/ (Daniel)
Changes in v4:
- Move docbook location below panel (Daniel)
- Improve docbook with references and more detailed explanation (Daniel)
- Instead of register/unregister, use init/cleanup (Daniel)
Changes in v5:
- Resolved conflict in drm_atomic_helper.c #include block
- Resolved conflict in rst with HDCP helper docs
Link to v1: https://patchwork.freedesktop.org/patch/msgid/20190228210939.83386-2-sean@poorly.run
Link to v2: https://patchwork.freedesktop.org/patch/msgid/20190326204509.96515-1-sean@poorly.run
Link to v3: https://patchwork.freedesktop.org/patch/msgid/20190502194956.218441-6-sean@poorly.run
Link to v4: https://patchwork.freedesktop.org/patch/msgid/20190508160920.144739-6-sean@poorly.run
Cc: Daniel Vetter <daniel@ffwll.ch>
Cc: Jose Souza <jose.souza@intel.com>
Cc: Zain Wang <wzz@rock-chips.com>
Cc: Tomasz Figa <tfiga@chromium.org>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Tested-by: Heiko Stuebner <heiko@sntech.de>
Reviewed-by: Daniel Vetter <daniel@ffwll.ch>
Signed-off-by: Sean Paul <seanpaul@chromium.org>
---
Documentation/gpu/drm-kms-helpers.rst | 9 +
drivers/gpu/drm/Makefile | 2 +-
drivers/gpu/drm/drm_atomic.c | 2 +
drivers/gpu/drm/drm_atomic_helper.c | 35 +++-
drivers/gpu/drm/drm_atomic_state_helper.c | 4 +
drivers/gpu/drm/drm_atomic_uapi.c | 7 +-
drivers/gpu/drm/drm_self_refresh_helper.c | 213 ++++++++++++++++++++++
include/drm/drm_atomic.h | 15 ++
include/drm/drm_connector.h | 14 ++
include/drm/drm_crtc.h | 19 ++
include/drm/drm_self_refresh_helper.h | 22 +++
11 files changed, 337 insertions(+), 5 deletions(-)
create mode 100644 drivers/gpu/drm/drm_self_refresh_helper.c
create mode 100644 include/drm/drm_self_refresh_helper.h
diff --git a/Documentation/gpu/drm-kms-helpers.rst b/Documentation/gpu/drm-kms-helpers.rst
index 0fe726a6ee678..b327bbc111821 100644
--- a/Documentation/gpu/drm-kms-helpers.rst
+++ b/Documentation/gpu/drm-kms-helpers.rst
@@ -181,6 +181,15 @@ Panel Helper Reference
.. kernel-doc:: drivers/gpu/drm/drm_panel_orientation_quirks.c
:export:
+Panel Self Refresh Helper Reference
+===================================
+
+.. kernel-doc:: drivers/gpu/drm/drm_self_refresh_helper.c
+ :doc: overview
+
+.. kernel-doc:: drivers/gpu/drm/drm_self_refresh_helper.c
+ :export:
+
HDCP Helper Functions Reference
===============================
diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
index d36feb4a62330..9d630a28a7880 100644
--- a/drivers/gpu/drm/Makefile
+++ b/drivers/gpu/drm/Makefile
@@ -43,7 +43,7 @@ drm_kms_helper-y := drm_crtc_helper.o drm_dp_helper.o drm_dsc.o drm_probe_helper
drm_simple_kms_helper.o drm_modeset_helper.o \
drm_scdc_helper.o drm_gem_framebuffer_helper.o \
drm_atomic_state_helper.o drm_damage_helper.o \
- drm_format_helper.o
+ drm_format_helper.o drm_self_refresh_helper.o
drm_kms_helper-$(CONFIG_DRM_PANEL_BRIDGE) += bridge/panel.o
drm_kms_helper-$(CONFIG_DRM_FBDEV_EMULATION) += drm_fb_helper.o
diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index a4e779deab0fb..419381abbdd16 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -384,6 +384,7 @@ static void drm_atomic_crtc_print_state(struct drm_printer *p,
drm_printf(p, "crtc[%u]: %s\n", crtc->base.id, crtc->name);
drm_printf(p, "\tenable=%d\n", state->enable);
drm_printf(p, "\tactive=%d\n", state->active);
+ drm_printf(p, "\tself_refresh_active=%d\n", state->self_refresh_active);
drm_printf(p, "\tplanes_changed=%d\n", state->planes_changed);
drm_printf(p, "\tmode_changed=%d\n", state->mode_changed);
drm_printf(p, "\tactive_changed=%d\n", state->active_changed);
@@ -999,6 +1000,7 @@ static void drm_atomic_connector_print_state(struct drm_printer *p,
drm_printf(p, "connector[%u]: %s\n", connector->base.id, connector->name);
drm_printf(p, "\tcrtc=%s\n", state->crtc ? state->crtc->name : "(null)");
+ drm_printf(p, "\tself_refresh_aware=%d\n", state->self_refresh_aware);
if (connector->connector_type == DRM_MODE_CONNECTOR_WRITEBACK)
if (state->writeback_job && state->writeback_job->fb)
diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
index e58be69960692..bc43cc08cdc11 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -33,6 +33,7 @@
#include <drm/drm_damage_helper.h>
#include <drm/drm_device.h>
#include <drm/drm_plane_helper.h>
+#include <drm/drm_self_refresh_helper.h>
#include <drm/drm_print.h>
#include <drm/drm_vblank.h>
#include <drm/drm_writeback.h>
@@ -953,10 +954,33 @@ int drm_atomic_helper_check(struct drm_device *dev,
if (state->legacy_cursor_update)
state->async_update = !drm_atomic_helper_async_check(dev, state);
+ drm_self_refresh_helper_alter_state(state);
+
return ret;
}
EXPORT_SYMBOL(drm_atomic_helper_check);
+static bool
+crtc_needs_disable(struct drm_crtc_state *old_state,
+ struct drm_crtc_state *new_state)
+{
+ /*
+ * No new_state means the crtc is off, so the only criteria is whether
+ * it's currently active or in self refresh mode.
+ */
+ if (!new_state)
+ return drm_atomic_crtc_effectively_active(old_state);
+
+ /*
+ * We need to run through the crtc_funcs->disable() function if the crtc
+ * is currently on, if it's transitioning to self refresh mode, or if
+ * it's in self refresh mode and needs to be fully disabled.
+ */
+ return old_state->active ||
+ (old_state->self_refresh_active && !new_state->enable) ||
+ new_state->self_refresh_active;
+}
+
static void
disable_outputs(struct drm_device *dev, struct drm_atomic_state *old_state)
{
@@ -977,7 +1001,14 @@ disable_outputs(struct drm_device *dev, struct drm_atomic_state *old_state)
old_crtc_state = drm_atomic_get_old_crtc_state(old_state, old_conn_state->crtc);
- if (!old_crtc_state->active ||
+ if (new_conn_state->crtc)
+ new_crtc_state = drm_atomic_get_new_crtc_state(
+ old_state,
+ new_conn_state->crtc);
+ else
+ new_crtc_state = NULL;
+
+ if (!crtc_needs_disable(old_crtc_state, new_crtc_state) ||
!drm_atomic_crtc_needs_modeset(old_conn_state->crtc->state))
continue;
@@ -1023,7 +1054,7 @@ disable_outputs(struct drm_device *dev, struct drm_atomic_state *old_state)
if (!drm_atomic_crtc_needs_modeset(new_crtc_state))
continue;
- if (!old_crtc_state->active)
+ if (!crtc_needs_disable(old_crtc_state, new_crtc_state))
continue;
funcs = crtc->helper_private;
diff --git a/drivers/gpu/drm/drm_atomic_state_helper.c b/drivers/gpu/drm/drm_atomic_state_helper.c
index 97ab26679b969..7d7347a6f1943 100644
--- a/drivers/gpu/drm/drm_atomic_state_helper.c
+++ b/drivers/gpu/drm/drm_atomic_state_helper.c
@@ -129,6 +129,10 @@ void __drm_atomic_helper_crtc_duplicate_state(struct drm_crtc *crtc,
state->commit = NULL;
state->event = NULL;
state->pageflip_flags = 0;
+
+ /* Self refresh should be canceled when a new update is available */
+ state->active = drm_atomic_crtc_effectively_active(state);
+ state->self_refresh_active = false;
}
EXPORT_SYMBOL(__drm_atomic_helper_crtc_duplicate_state);
diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c
index eb22e8bdd8531..abe38bdf85ae6 100644
--- a/drivers/gpu/drm/drm_atomic_uapi.c
+++ b/drivers/gpu/drm/drm_atomic_uapi.c
@@ -490,7 +490,7 @@ drm_atomic_crtc_get_property(struct drm_crtc *crtc,
struct drm_mode_config *config = &dev->mode_config;
if (property == config->prop_active)
- *val = state->active;
+ *val = drm_atomic_crtc_effectively_active(state);
else if (property == config->prop_mode_id)
*val = (state->mode_blob) ? state->mode_blob->base.id : 0;
else if (property == config->prop_vrr_enabled)
@@ -788,7 +788,10 @@ drm_atomic_connector_get_property(struct drm_connector *connector,
if (property == config->prop_crtc_id) {
*val = (state->crtc) ? state->crtc->base.id : 0;
} else if (property == config->dpms_property) {
- *val = connector->dpms;
+ if (state->crtc && state->crtc->state->self_refresh_active)
+ *val = DRM_MODE_DPMS_ON;
+ else
+ *val = connector->dpms;
} else if (property == config->tv_select_subconnector_property) {
*val = state->tv.subconnector;
} else if (property == config->tv_left_margin_property) {
diff --git a/drivers/gpu/drm/drm_self_refresh_helper.c b/drivers/gpu/drm/drm_self_refresh_helper.c
new file mode 100644
index 0000000000000..c7083c5694fee
--- /dev/null
+++ b/drivers/gpu/drm/drm_self_refresh_helper.c
@@ -0,0 +1,213 @@
+/* SPDX-License-Identifier: MIT */
+/*
+ * Copyright (C) 2019 Google, Inc.
+ *
+ * Authors:
+ * Sean Paul <seanpaul@chromium.org>
+ */
+#include <drm/drm_atomic.h>
+#include <drm/drm_atomic_helper.h>
+#include <drm/drm_connector.h>
+#include <drm/drm_crtc.h>
+#include <drm/drm_device.h>
+#include <drm/drm_mode_config.h>
+#include <drm/drm_modeset_lock.h>
+#include <drm/drm_print.h>
+#include <drm/drm_self_refresh_helper.h>
+#include <linux/bitops.h>
+#include <linux/slab.h>
+#include <linux/workqueue.h>
+
+/**
+ * DOC: overview
+ *
+ * This helper library provides an easy way for drivers to leverage the atomic
+ * framework to implement panel self refresh (SR) support. Drivers are
+ * responsible for initializing and cleaning up the SR helpers on load/unload
+ * (see &drm_self_refresh_helper_init/&drm_self_refresh_helper_cleanup).
+ * The connector is responsible for setting
+ * &drm_connector_state.self_refresh_aware to true at runtime if it is SR-aware
+ * (meaning it knows how to initiate self refresh on the panel).
+ *
+ * Once a crtc has enabled SR using &drm_self_refresh_helper_init, the
+ * helpers will monitor activity and call back into the driver to enable/disable
+ * SR as appropriate. The best way to think about this is that it's a DPMS
+ * on/off request with &drm_crtc_state.self_refresh_active set in crtc state
+ * that tells you to disable/enable SR on the panel instead of power-cycling it.
+ *
+ * During SR, drivers may choose to fully disable their crtc/encoder/bridge
+ * hardware (in which case no driver changes are necessary), or they can inspect
+ * &drm_crtc_state.self_refresh_active if they want to enter low power mode
+ * without full disable (in case full disable/enable is too slow).
+ *
+ * SR will be deactivated if there are any atomic updates affecting the
+ * pipe that is in SR mode. If a crtc is driving multiple connectors, all
+ * connectors must be SR aware and all will enter/exit SR mode at the same time.
+ *
+ * If the crtc and connector are SR aware, but the panel connected does not
+ * support it (or is otherwise unable to enter SR), the driver should fail
+ * atomic_check when &drm_crtc_state.self_refresh_active is true.
+ */
+
+struct drm_self_refresh_data {
+ struct drm_crtc *crtc;
+ struct delayed_work entry_work;
+ struct drm_atomic_state *save_state;
+ unsigned int entry_delay_ms;
+};
+
+static void drm_self_refresh_helper_entry_work(struct work_struct *work)
+{
+ struct drm_self_refresh_data *sr_data = container_of(
+ to_delayed_work(work),
+ struct drm_self_refresh_data, entry_work);
+ struct drm_crtc *crtc = sr_data->crtc;
+ struct drm_device *dev = crtc->dev;
+ struct drm_modeset_acquire_ctx ctx;
+ struct drm_atomic_state *state;
+ struct drm_connector *conn;
+ struct drm_connector_state *conn_state;
+ struct drm_crtc_state *crtc_state;
+ int i, ret;
+
+ drm_modeset_acquire_init(&ctx, 0);
+
+ state = drm_atomic_state_alloc(dev);
+ if (!state) {
+ ret = -ENOMEM;
+ goto out;
+ }
+
+retry:
+ state->acquire_ctx = &ctx;
+
+ crtc_state = drm_atomic_get_crtc_state(state, crtc);
+ if (IS_ERR(crtc_state)) {
+ ret = PTR_ERR(crtc_state);
+ goto out;
+ }
+
+ if (!crtc_state->enable)
+ goto out;
+
+ ret = drm_atomic_add_affected_connectors(state, crtc);
+ if (ret)
+ goto out;
+
+ for_each_new_connector_in_state(state, conn, conn_state, i) {
+ if (!conn_state->self_refresh_aware)
+ goto out;
+ }
+
+ crtc_state->active = false;
+ crtc_state->self_refresh_active = true;
+
+ ret = drm_atomic_commit(state);
+ if (ret)
+ goto out;
+
+out:
+ if (ret == -EDEADLK) {
+ drm_atomic_state_clear(state);
+ ret = drm_modeset_backoff(&ctx);
+ if (!ret)
+ goto retry;
+ }
+
+ drm_atomic_state_put(state);
+ drm_modeset_drop_locks(&ctx);
+ drm_modeset_acquire_fini(&ctx);
+}
+
+/**
+ * drm_self_refresh_helper_alter_state - Alters the atomic state for SR exit
+ * @state: the state currently being checked
+ *
+ * Called at the end of atomic check. This function checks the state for flags
+ * incompatible with self refresh exit and changes them. This is a bit
+ * disingenuous since userspace is expecting one thing and we're giving it
+ * another. However in order to keep self refresh entirely hidden from
+ * userspace, this is required.
+ *
+ * At the end, we queue up the self refresh entry work so we can enter PSR after
+ * the desired delay.
+ */
+void drm_self_refresh_helper_alter_state(struct drm_atomic_state *state)
+{
+ struct drm_crtc *crtc;
+ struct drm_crtc_state *crtc_state;
+ int i;
+
+ if (state->async_update || !state->allow_modeset) {
+ for_each_old_crtc_in_state(state, crtc, crtc_state, i) {
+ if (crtc_state->self_refresh_active) {
+ state->async_update = false;
+ state->allow_modeset = true;
+ break;
+ }
+ }
+ }
+
+ for_each_new_crtc_in_state(state, crtc, crtc_state, i) {
+ struct drm_self_refresh_data *sr_data;
+
+ /* Don't trigger the entry timer when we're already in SR */
+ if (crtc_state->self_refresh_active)
+ continue;
+
+ sr_data = crtc->self_refresh_data;
+ if (!sr_data)
+ continue;
+
+ mod_delayed_work(system_wq, &sr_data->entry_work,
+ msecs_to_jiffies(sr_data->entry_delay_ms));
+ }
+}
+EXPORT_SYMBOL(drm_self_refresh_helper_alter_state);
+
+/**
+ * drm_self_refresh_helper_init - Initializes self refresh helpers for a crtc
+ * @crtc: the crtc which supports self refresh supported displays
+ * @entry_delay_ms: amount of inactivity to wait before entering self refresh
+ */
+int drm_self_refresh_helper_init(struct drm_crtc *crtc,
+ unsigned int entry_delay_ms)
+{
+ struct drm_self_refresh_data *sr_data = crtc->self_refresh_data;
+
+ /* Helper is already initialized */
+ if (WARN_ON(sr_data))
+ return -EINVAL;
+
+ sr_data = kzalloc(sizeof(*sr_data), GFP_KERNEL);
+ if (!sr_data)
+ return -ENOMEM;
+
+ INIT_DELAYED_WORK(&sr_data->entry_work,
+ drm_self_refresh_helper_entry_work);
+ sr_data->entry_delay_ms = entry_delay_ms;
+ sr_data->crtc = crtc;
+
+ crtc->self_refresh_data = sr_data;
+ return 0;
+}
+EXPORT_SYMBOL(drm_self_refresh_helper_init);
+
+/**
+ * drm_self_refresh_helper_cleanup - Cleans up self refresh helpers for a crtc
+ * @crtc: the crtc to cleanup
+ */
+void drm_self_refresh_helper_cleanup(struct drm_crtc *crtc)
+{
+ struct drm_self_refresh_data *sr_data = crtc->self_refresh_data;
+
+ /* Helper is already uninitialized */
+ if (sr_data)
+ return;
+
+ crtc->self_refresh_data = NULL;
+
+ cancel_delayed_work_sync(&sr_data->entry_work);
+ kfree(sr_data);
+}
+EXPORT_SYMBOL(drm_self_refresh_helper_cleanup);
diff --git a/include/drm/drm_atomic.h b/include/drm/drm_atomic.h
index f122156478010..927e1205d7aa4 100644
--- a/include/drm/drm_atomic.h
+++ b/include/drm/drm_atomic.h
@@ -957,4 +957,19 @@ drm_atomic_crtc_needs_modeset(const struct drm_crtc_state *state)
state->connectors_changed;
}
+/**
+ * drm_atomic_crtc_effectively_active - compute whether crtc is actually active
+ * @state: &drm_crtc_state for the CRTC
+ *
+ * When in self refresh mode, the crtc_state->active value will be false, since
+ * the crtc is off. However in some cases we're interested in whether the crtc
+ * is active, or effectively active (ie: it's connected to an active display).
+ * In these cases, use this function instead of just checking active.
+ */
+static inline bool
+drm_atomic_crtc_effectively_active(const struct drm_crtc_state *state)
+{
+ return state->active || state->self_refresh_active;
+}
+
#endif /* DRM_ATOMIC_H_ */
diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
index 071143bc0ebd1..c6f8486d8b8f3 100644
--- a/include/drm/drm_connector.h
+++ b/include/drm/drm_connector.h
@@ -548,6 +548,20 @@ struct drm_connector_state {
/** @tv: TV connector state */
struct drm_tv_connector_state tv;
+ /**
+ * @self_refresh_aware:
+ *
+ * This tracks whether a connector is aware of the self refresh state.
+ * It should be set to true for those connector implementations which
+ * understand the self refresh state. This is needed since the crtc
+ * registers the self refresh helpers and it doesn't know if the
+ * connectors downstream have implemented self refresh entry/exit.
+ *
+ * Drivers should set this to true in atomic_check if they know how to
+ * handle self_refresh requests.
+ */
+ bool self_refresh_aware;
+
/**
* @picture_aspect_ratio: Connector property to control the
* HDMI infoframe aspect ratio setting.
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index dc42b9e353333..4fda610d76f26 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -54,6 +54,7 @@ struct drm_mode_set;
struct drm_file;
struct drm_clip_rect;
struct drm_printer;
+struct drm_self_refresh_data;
struct device_node;
struct dma_fence;
struct edid;
@@ -300,6 +301,17 @@ struct drm_crtc_state {
*/
bool vrr_enabled;
+ /**
+ * @self_refresh_active:
+ *
+ * Used by the self refresh helpers to denote when a self refresh
+ * transition is occuring. This will be set on enable/disable callbacks
+ * when self refresh is being enabled or disabled. In some cases, it may
+ * not be desirable to fully shut off the crtc during self refresh.
+ * CRTC's can inspect this flag and determine the best course of action.
+ */
+ bool self_refresh_active;
+
/**
* @event:
*
@@ -1088,6 +1100,13 @@ struct drm_crtc {
* The name of the CRTC's fence timeline.
*/
char timeline_name[32];
+
+ /**
+ * @self_refresh_data: Holds the state for the self refresh helpers
+ *
+ * Initialized via drm_self_refresh_helper_register().
+ */
+ struct drm_self_refresh_data *self_refresh_data;
};
/**
diff --git a/include/drm/drm_self_refresh_helper.h b/include/drm/drm_self_refresh_helper.h
new file mode 100644
index 0000000000000..405e86fb8461e
--- /dev/null
+++ b/include/drm/drm_self_refresh_helper.h
@@ -0,0 +1,22 @@
+/* SPDX-License-Identifier: MIT */
+/*
+ * Copyright (C) 2019 Google, Inc.
+ *
+ * Authors:
+ * Sean Paul <seanpaul@chromium.org>
+ */
+#ifndef DRM_SELF_REFRESH_HELPER_H_
+#define DRM_SELF_REFRESH_HELPER_H_
+
+struct drm_atomic_state;
+struct drm_connector;
+struct drm_device;
+struct drm_modeset_acquire_ctx;
+
+void drm_self_refresh_helper_alter_state(struct drm_atomic_state *state);
+
+int drm_self_refresh_helper_init(struct drm_crtc *crtc,
+ unsigned int entry_delay_ms);
+
+void drm_self_refresh_helper_cleanup(struct drm_crtc *crtc);
+#endif
--
Sean Paul, Software Engineer, Google / Chromium OS
^ permalink raw reply related
* Re: [PATCH v3 33/33] docs: EDID/HOWTO.txt: convert it and rename to howto.rst
From: Jonathan Corbet @ 2019-06-11 15:37 UTC (permalink / raw)
To: Mauro Carvalho Chehab
Cc: Daniel Vetter, Linux Doc Mailing List, Mauro Carvalho Chehab,
linux-kernel, Maarten Lankhorst, Maxime Ripard, Sean Paul,
David Airlie, dri-devel
In-Reply-To: <20190611060215.232af2bb@coco.lan>
On Tue, 11 Jun 2019 06:02:15 -0300
Mauro Carvalho Chehab <mchehab+samsung@kernel.org> wrote:
> Jon, please correct me if I' wrong, bu I guess the plan is to place them
> somewhere under Documentation/admin-guide/.
That makes sense to me.
> If so, perhaps creating a Documentation/admin-guide/drm dir there and
> place docs like EDID/HOWTO.txt, svga.txt, etc would work.
Maybe "graphics" or "display" rather than "drm", which may not entirely
applicable to all of those docs or as familiar to all admins?
> Btw, that's one of the reasons[1] why I opted to keep the files where they
> are: properly organizing the converted documents call for such kind
> of discussions. On my experience, discussing names and directory locations
> can generate warm discussions and take a lot of time to reach consensus.
Moving docs is a pain; my life would certainly be easier if I were happy
to just let everything lie where it fell :) But it's far from the hardest
problem we solve in kernel development, I assume we can figure it out.
Thanks,
jon
^ permalink raw reply
* Re: [PATCH v12 03/13] dt-bindings: Add doc for the Ingenic TCU drivers
From: Rob Herring @ 2019-06-11 14:57 UTC (permalink / raw)
To: Paul Cercueil
Cc: Mark Rutland, Ralf Baechle, Paul Burton, James Hogan,
Jonathan Corbet, Daniel Lezcano, Thomas Gleixner,
Michael Turquette, Stephen Boyd, Jason Cooper, Marc Zyngier,
Lee Jones, Mathieu Malaterre, linux-kernel@vger.kernel.org,
devicetree, linux-mips, Linux Doc Mailing List, linux-clk, od
In-Reply-To: <1558811596.2016.1@crapouillou.net>
On Sat, May 25, 2019 at 1:13 PM Paul Cercueil <paul@crapouillou.net> wrote:
>
>
>
> Le ven. 24 mai 2019 à 22:21, Rob Herring <robh@kernel.org> a écrit :
> > On Tue, May 21, 2019 at 04:51:31PM +0200, Paul Cercueil wrote:
> >> Add documentation about how to properly use the Ingenic TCU
> >> (Timer/Counter Unit) drivers from devicetree.
> >>
> >> Signed-off-by: Paul Cercueil <paul@crapouillou.net>
> >> ---
> >>
> >> Notes:
> >> v4: New patch in this series. Corresponds to V2 patches 3-4-5
> >> with
> >> added content.
> >>
> >> v5: - Edited PWM/watchdog DT bindings documentation to point to
> >> the new
> >> document.
> >> - Moved main document to
> >> Documentation/devicetree/bindings/timer/ingenic,tcu.txt
> >> - Updated documentation to reflect the new devicetree bindings.
> >>
> >> v6: - Removed PWM/watchdog documentation files as asked by
> >> upstream
> >> - Removed doc about properties that should be implicit
> >> - Removed doc about ingenic,timer-channel /
> >> ingenic,clocksource-channel as they are gone
> >> - Fix WDT clock name in the binding doc
> >> - Fix lengths of register areas in watchdog/pwm nodes
> >>
> >> v7: No change
> >>
> >> v8: - Fix address of the PWM node
> >> - Added doc about system timer and clocksource children nodes
> >>
> >> v9: - Remove doc about system timer and clocksource children
> >> nodes...
> >> - Add doc about ingenic,pwm-channels-mask property
> >>
> >> v10: No change
> >>
> >> v11: Fix info about default value of ingenic,pwm-channels-mask
> >>
> >> v12: Drop sub-nodes for now; they will be introduced in a
> >> follow-up
> >> patchset.
> >
> > Why? I believe I acked them.
>
> The patchset was too big, and I've already been trying to get it
> upstream for
> more than one year now. So I cut it in half in hope that it'll be
> easier to
> upstream it that way.
You can drop the driver part and keep the binding. Unlike drivers, we
don't want bindings to needlessly evolve, and you don't have to wait
til a driver implements some functionality to add that to the binding.
Rob
^ permalink raw reply
* RE: [PATCH V14 2/5] thermal: of-thermal: add API for getting sensor ID from DT
From: Anson Huang @ 2019-06-11 12:37 UTC (permalink / raw)
To: Aisheng Dong, robh+dt@kernel.org, mark.rutland@arm.com,
corbet@lwn.net, shawnguo@kernel.org, s.hauer@pengutronix.de,
kernel@pengutronix.de, festevam@gmail.com,
catalin.marinas@arm.com, will.deacon@arm.com, rui.zhang@intel.com,
edubezval@gmail.com, daniel.lezcano@linaro.org,
ulf.hansson@linaro.org, Peng Fan, mchehab+samsung@kernel.org,
linux@roeck-us.net, Daniel Baluta, maxime.ripard@bootlin.com,
olof@lixom.net, jagan@amarulasolutions.com,
horms+renesas@verge.net.au, Leonard Crestez,
bjorn.andersson@linaro.org, dinguyen@kernel.org,
enric.balletbo@collabora.com, devicetree@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-doc@vger.kernel.org,
linux-arm-kernel@lists.infradead.org, linux-pm@vger.kernel.org
Cc: dl-linux-imx
In-Reply-To: <AM0PR04MB42118F3F8E87A72758D4A7B780ED0@AM0PR04MB4211.eurprd04.prod.outlook.com>
Hi, Aisheng
> -----Original Message-----
> From: Aisheng Dong
> Sent: Tuesday, June 11, 2019 7:47 PM
> To: Anson Huang <anson.huang@nxp.com>; robh+dt@kernel.org;
> mark.rutland@arm.com; corbet@lwn.net; shawnguo@kernel.org;
> s.hauer@pengutronix.de; kernel@pengutronix.de; festevam@gmail.com;
> catalin.marinas@arm.com; will.deacon@arm.com; rui.zhang@intel.com;
> edubezval@gmail.com; daniel.lezcano@linaro.org; ulf.hansson@linaro.org;
> Peng Fan <peng.fan@nxp.com>; mchehab+samsung@kernel.org;
> linux@roeck-us.net; Daniel Baluta <daniel.baluta@nxp.com>;
> maxime.ripard@bootlin.com; olof@lixom.net; jagan@amarulasolutions.com;
> horms+renesas@verge.net.au; Leonard Crestez <leonard.crestez@nxp.com>;
> bjorn.andersson@linaro.org; dinguyen@kernel.org;
> enric.balletbo@collabora.com; devicetree@vger.kernel.org; linux-
> kernel@vger.kernel.org; linux-doc@vger.kernel.org; linux-arm-
> kernel@lists.infradead.org; linux-pm@vger.kernel.org
> Cc: dl-linux-imx <linux-imx@nxp.com>
> Subject: RE: [PATCH V14 2/5] thermal: of-thermal: add API for getting sensor
> ID from DT
>
> Hi Anson,
>
> The implementation looks good to me.
> A few minor comments on the doc:
This patch is pending for so long and even till now I am NOT sure if everyone is OK for
this change, so I will wait for some comments if there is any, and will improve the doc
together in next version.
Thanks,
Anson
>
> > From: Anson.Huang@nxp.com [mailto:Anson.Huang@nxp.com]
> > Sent: Monday, June 10, 2019 10:53 AM
> >
> > On some platforms like i.MX8QXP, the thermal driver needs a real HW
> > sensor ID from DT thermal zone, the HW sensor ID is used to get
> > temperature from SCU firmware, and the virtual sensor ID starting from
> > 0 to N is NOT used at all,
>
> This API is unware of HW ID or Virtual ID. So we probably no need to
> mention It here.
>
> > this patch adds new API thermal_zone_of_get_sensor_id() to provide the
> > feature of getting sensor ID from DT thermal zone's node.
> >
>
> "It's useful for thermal driver to register the specific thermal zone devices
> from DT in a common way."
>
> > Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
> > ---
> > Changes since V13:
> > - add new API into the thermal API doc.
> > ---
> > Documentation/thermal/sysfs-api.txt | 8 +++++
> > drivers/thermal/of-thermal.c | 66
> > ++++++++++++++++++++++++++++---------
> > include/linux/thermal.h | 10 ++++++
> > 3 files changed, 68 insertions(+), 16 deletions(-)
> >
> > diff --git a/Documentation/thermal/sysfs-api.txt
> > b/Documentation/thermal/sysfs-api.txt
> > index c3fa500..8d7f1b1 100644
> > --- a/Documentation/thermal/sysfs-api.txt
> > +++ b/Documentation/thermal/sysfs-api.txt
> > @@ -159,6 +159,14 @@ temperature) and throttle appropriate devices.
> > for the thermal zone device, which might be useful for platform
> > drivers for temperature calculations.
> >
> > +1.1.9 int thermal_zone_of_get_sensor_id(struct device_node *tz_np,
> > + struct device_node *sensor_np,
> > + u32 *id)
> > +
> > + This interface is used to get the sensor id from thermal sensor's
> > + phandle argument, it might be necessary for some platforms which
> > + have specific sensor ID rather than virtual ID from 0 - N.
>
> Does below one look better?
>
> "This interface is used to get the sensor id from the given thermal zone in DT,
> which might be useful for thermal drivers to register specific thermal zone
> device in a common way."
>
> > +
> > 1.2 thermal cooling device interface
> > 1.2.1 struct thermal_cooling_device
> > *thermal_cooling_device_register(char
> > *name,
> > void *devdata, struct thermal_cooling_device_ops *) diff --git
> > a/drivers/thermal/of-thermal.c b/drivers/thermal/of-thermal.c index
> > dc5093b..a53792b 100644
> > --- a/drivers/thermal/of-thermal.c
> > +++ b/drivers/thermal/of-thermal.c
> > @@ -449,6 +449,54 @@ thermal_zone_of_add_sensor(struct device_node
> > *zone, }
> >
> > /**
> > + * thermal_zone_of_get_sensor_id - get sensor ID from a DT thermal
> > + zone
> > + * @tz_np: a valid thermal zone device node.
> > + * @sensor_np: a sensor node of a valid sensor device.
> > + * @id: a sensor ID pointer will be passed back.
>
> the sensor ID returned if success
>
> > + *
> > + * This function will get sensor ID from a given thermal zone node,
> > + use
> > + * "thermal-sensors" as list name, and get sensor ID from first
> > + phandle's
> > + * argument.
>
> "This function will get sensor ID from a given thermal zone node and the
> sensor get must match the temperature providers @sensor_np."
>
> Regards
> Dong Aisheng
>
> > + *
> > + * Return: 0 on success, proper error code otherwise.
> > + */
> > +
> > +int thermal_zone_of_get_sensor_id(struct device_node *tz_np,
> > + struct device_node *sensor_np,
> > + u32 *id)
> > +{
> > + struct of_phandle_args sensor_specs;
> > + int ret;
> > +
> > + ret = of_parse_phandle_with_args(tz_np,
> > + "thermal-sensors",
> > + "#thermal-sensor-cells",
> > + 0,
> > + &sensor_specs);
> > + if (ret)
> > + return ret;
> > +
> > + if (sensor_specs.np != sensor_np) {
> > + of_node_put(sensor_specs.np);
> > + return -ENODEV;
> > + }
> > +
> > + if (sensor_specs.args_count >= 1) {
> > + *id = sensor_specs.args[0];
> > + WARN(sensor_specs.args_count > 1,
> > + "%pOFn: too many cells in sensor specifier %d\n",
> > + sensor_specs.np, sensor_specs.args_count);
> > + } else {
> > + *id = 0;
> > + }
> > +
> > + of_node_put(sensor_specs.np);
> > +
> > + return 0;
> > +}
> > +EXPORT_SYMBOL_GPL(thermal_zone_of_get_sensor_id);
> > +
> > +/**
> > * thermal_zone_of_sensor_register - registers a sensor to a DT thermal
> zone
> > * @dev: a valid struct device pointer of a sensor device. Must contain
> > * a valid .of_node, for the sensor node.
> > @@ -499,36 +547,22 @@ thermal_zone_of_sensor_register(struct device
> > *dev, int sensor_id, void *data,
> > sensor_np = of_node_get(dev->of_node);
> >
> > for_each_available_child_of_node(np, child) {
> > - struct of_phandle_args sensor_specs;
> > int ret, id;
> >
> > /* For now, thermal framework supports only 1 sensor per
> zone */
> > - ret = of_parse_phandle_with_args(child, "thermal-sensors",
> > - "#thermal-sensor-cells",
> > - 0, &sensor_specs);
> > + ret = thermal_zone_of_get_sensor_id(child, sensor_np, &id);
> > if (ret)
> > continue;
> >
> > - if (sensor_specs.args_count >= 1) {
> > - id = sensor_specs.args[0];
> > - WARN(sensor_specs.args_count > 1,
> > - "%pOFn: too many cells in sensor specifier %d\n",
> > - sensor_specs.np, sensor_specs.args_count);
> > - } else {
> > - id = 0;
> > - }
> > -
> > - if (sensor_specs.np == sensor_np && id == sensor_id) {
> > + if (id == sensor_id) {
> > tzd = thermal_zone_of_add_sensor(child, sensor_np,
> > data, ops);
> > if (!IS_ERR(tzd))
> > tzd->ops->set_mode(tzd,
> THERMAL_DEVICE_ENABLED);
> >
> > - of_node_put(sensor_specs.np);
> > of_node_put(child);
> > goto exit;
> > }
> > - of_node_put(sensor_specs.np);
> > }
> > exit:
> > of_node_put(sensor_np);
> > diff --git a/include/linux/thermal.h b/include/linux/thermal.h index
> > 15a4ca5..5edffe6 100644
> > --- a/include/linux/thermal.h
> > +++ b/include/linux/thermal.h
> > @@ -375,6 +375,9 @@ struct thermal_trip {
> >
> > /* Function declarations */
> > #ifdef CONFIG_THERMAL_OF
> > +int thermal_zone_of_get_sensor_id(struct device_node *tz_np,
> > + struct device_node *sensor_np,
> > + u32 *id);
> > struct thermal_zone_device *
> > thermal_zone_of_sensor_register(struct device *dev, int id, void *data,
> > const struct thermal_zone_of_device_ops
> *ops); @@
> > -386,6 +389,13 @@ struct thermal_zone_device
> > *devm_thermal_zone_of_sensor_register(
> > void devm_thermal_zone_of_sensor_unregister(struct device *dev,
> > struct thermal_zone_device *tz);
> #else
> > +
> > +static int thermal_zone_of_get_sensor_id(struct device_node *tz_np,
> > + struct device_node *sensor_np,
> > + u32 *id)
> > +{
> > + return -ENOENT;
> > +}
> > static inline struct thermal_zone_device *
> > thermal_zone_of_sensor_register(struct device *dev, int id, void *data,
> > const struct thermal_zone_of_device_ops
> *ops)
> > --
> > 2.7.4
^ permalink raw reply
* RE: [PATCH V14 2/5] thermal: of-thermal: add API for getting sensor ID from DT
From: Aisheng Dong @ 2019-06-11 11:46 UTC (permalink / raw)
To: Anson Huang, robh+dt@kernel.org, mark.rutland@arm.com,
corbet@lwn.net, shawnguo@kernel.org, s.hauer@pengutronix.de,
kernel@pengutronix.de, festevam@gmail.com,
catalin.marinas@arm.com, will.deacon@arm.com, rui.zhang@intel.com,
edubezval@gmail.com, daniel.lezcano@linaro.org,
ulf.hansson@linaro.org, Peng Fan, mchehab+samsung@kernel.org,
linux@roeck-us.net, Daniel Baluta, maxime.ripard@bootlin.com,
olof@lixom.net, jagan@amarulasolutions.com,
horms+renesas@verge.net.au, Leonard Crestez,
bjorn.andersson@linaro.org, dinguyen@kernel.org,
enric.balletbo@collabora.com, devicetree@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-doc@vger.kernel.org,
linux-arm-kernel@lists.infradead.org, linux-pm@vger.kernel.org
Cc: dl-linux-imx
In-Reply-To: <20190610025254.23940-2-Anson.Huang@nxp.com>
Hi Anson,
The implementation looks good to me.
A few minor comments on the doc:
> From: Anson.Huang@nxp.com [mailto:Anson.Huang@nxp.com]
> Sent: Monday, June 10, 2019 10:53 AM
>
> On some platforms like i.MX8QXP, the thermal driver needs a real HW sensor
> ID from DT thermal zone, the HW sensor ID is used to get temperature from
> SCU firmware, and the virtual sensor ID starting from 0 to N is NOT used at all,
This API is unware of HW ID or Virtual ID. So we probably no need to mention
It here.
> this patch adds new API thermal_zone_of_get_sensor_id() to provide the
> feature of getting sensor ID from DT thermal zone's node.
>
"It's useful for thermal driver to register the specific thermal zone devices
from DT in a common way."
> Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
> ---
> Changes since V13:
> - add new API into the thermal API doc.
> ---
> Documentation/thermal/sysfs-api.txt | 8 +++++
> drivers/thermal/of-thermal.c | 66
> ++++++++++++++++++++++++++++---------
> include/linux/thermal.h | 10 ++++++
> 3 files changed, 68 insertions(+), 16 deletions(-)
>
> diff --git a/Documentation/thermal/sysfs-api.txt
> b/Documentation/thermal/sysfs-api.txt
> index c3fa500..8d7f1b1 100644
> --- a/Documentation/thermal/sysfs-api.txt
> +++ b/Documentation/thermal/sysfs-api.txt
> @@ -159,6 +159,14 @@ temperature) and throttle appropriate devices.
> for the thermal zone device, which might be useful for platform
> drivers for temperature calculations.
>
> +1.1.9 int thermal_zone_of_get_sensor_id(struct device_node *tz_np,
> + struct device_node *sensor_np,
> + u32 *id)
> +
> + This interface is used to get the sensor id from thermal sensor's
> + phandle argument, it might be necessary for some platforms which
> + have specific sensor ID rather than virtual ID from 0 - N.
Does below one look better?
"This interface is used to get the sensor id from the given thermal zone in
DT, which might be useful for thermal drivers to register specific thermal
zone device in a common way."
> +
> 1.2 thermal cooling device interface
> 1.2.1 struct thermal_cooling_device *thermal_cooling_device_register(char
> *name,
> void *devdata, struct thermal_cooling_device_ops *) diff --git
> a/drivers/thermal/of-thermal.c b/drivers/thermal/of-thermal.c index
> dc5093b..a53792b 100644
> --- a/drivers/thermal/of-thermal.c
> +++ b/drivers/thermal/of-thermal.c
> @@ -449,6 +449,54 @@ thermal_zone_of_add_sensor(struct device_node
> *zone, }
>
> /**
> + * thermal_zone_of_get_sensor_id - get sensor ID from a DT thermal zone
> + * @tz_np: a valid thermal zone device node.
> + * @sensor_np: a sensor node of a valid sensor device.
> + * @id: a sensor ID pointer will be passed back.
the sensor ID returned if success
> + *
> + * This function will get sensor ID from a given thermal zone node, use
> + * "thermal-sensors" as list name, and get sensor ID from first
> + phandle's
> + * argument.
"This function will get sensor ID from a given thermal zone node and the sensor
get must match the temperature providers @sensor_np."
Regards
Dong Aisheng
> + *
> + * Return: 0 on success, proper error code otherwise.
> + */
> +
> +int thermal_zone_of_get_sensor_id(struct device_node *tz_np,
> + struct device_node *sensor_np,
> + u32 *id)
> +{
> + struct of_phandle_args sensor_specs;
> + int ret;
> +
> + ret = of_parse_phandle_with_args(tz_np,
> + "thermal-sensors",
> + "#thermal-sensor-cells",
> + 0,
> + &sensor_specs);
> + if (ret)
> + return ret;
> +
> + if (sensor_specs.np != sensor_np) {
> + of_node_put(sensor_specs.np);
> + return -ENODEV;
> + }
> +
> + if (sensor_specs.args_count >= 1) {
> + *id = sensor_specs.args[0];
> + WARN(sensor_specs.args_count > 1,
> + "%pOFn: too many cells in sensor specifier %d\n",
> + sensor_specs.np, sensor_specs.args_count);
> + } else {
> + *id = 0;
> + }
> +
> + of_node_put(sensor_specs.np);
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(thermal_zone_of_get_sensor_id);
> +
> +/**
> * thermal_zone_of_sensor_register - registers a sensor to a DT thermal zone
> * @dev: a valid struct device pointer of a sensor device. Must contain
> * a valid .of_node, for the sensor node.
> @@ -499,36 +547,22 @@ thermal_zone_of_sensor_register(struct device *dev,
> int sensor_id, void *data,
> sensor_np = of_node_get(dev->of_node);
>
> for_each_available_child_of_node(np, child) {
> - struct of_phandle_args sensor_specs;
> int ret, id;
>
> /* For now, thermal framework supports only 1 sensor per zone */
> - ret = of_parse_phandle_with_args(child, "thermal-sensors",
> - "#thermal-sensor-cells",
> - 0, &sensor_specs);
> + ret = thermal_zone_of_get_sensor_id(child, sensor_np, &id);
> if (ret)
> continue;
>
> - if (sensor_specs.args_count >= 1) {
> - id = sensor_specs.args[0];
> - WARN(sensor_specs.args_count > 1,
> - "%pOFn: too many cells in sensor specifier %d\n",
> - sensor_specs.np, sensor_specs.args_count);
> - } else {
> - id = 0;
> - }
> -
> - if (sensor_specs.np == sensor_np && id == sensor_id) {
> + if (id == sensor_id) {
> tzd = thermal_zone_of_add_sensor(child, sensor_np,
> data, ops);
> if (!IS_ERR(tzd))
> tzd->ops->set_mode(tzd, THERMAL_DEVICE_ENABLED);
>
> - of_node_put(sensor_specs.np);
> of_node_put(child);
> goto exit;
> }
> - of_node_put(sensor_specs.np);
> }
> exit:
> of_node_put(sensor_np);
> diff --git a/include/linux/thermal.h b/include/linux/thermal.h index
> 15a4ca5..5edffe6 100644
> --- a/include/linux/thermal.h
> +++ b/include/linux/thermal.h
> @@ -375,6 +375,9 @@ struct thermal_trip {
>
> /* Function declarations */
> #ifdef CONFIG_THERMAL_OF
> +int thermal_zone_of_get_sensor_id(struct device_node *tz_np,
> + struct device_node *sensor_np,
> + u32 *id);
> struct thermal_zone_device *
> thermal_zone_of_sensor_register(struct device *dev, int id, void *data,
> const struct thermal_zone_of_device_ops *ops); @@
> -386,6 +389,13 @@ struct thermal_zone_device
> *devm_thermal_zone_of_sensor_register(
> void devm_thermal_zone_of_sensor_unregister(struct device *dev,
> struct thermal_zone_device *tz); #else
> +
> +static int thermal_zone_of_get_sensor_id(struct device_node *tz_np,
> + struct device_node *sensor_np,
> + u32 *id)
> +{
> + return -ENOENT;
> +}
> static inline struct thermal_zone_device *
> thermal_zone_of_sensor_register(struct device *dev, int id, void *data,
> const struct thermal_zone_of_device_ops *ops)
> --
> 2.7.4
^ permalink raw reply
* Re: [PATCH v7 22/27] binfmt_elf: Extract .note.gnu.property from an ELF file
From: Dave Martin @ 2019-06-11 11:41 UTC (permalink / raw)
To: Florian Weimer
Cc: Yu-cheng Yu, 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, 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: <87lfy9cq04.fsf@oldenburg2.str.redhat.com>
On Mon, Jun 10, 2019 at 07:24:43PM +0200, Florian Weimer wrote:
> * Yu-cheng Yu:
>
> > To me, looking at PT_GNU_PROPERTY and not trying to support anything is a
> > logical choice. And it breaks only a limited set of toolchains.
> >
> > I will simplify the parser and leave this patch as-is for anyone who wants to
> > back-port. Are there any objections or concerns?
>
> Red Hat Enterprise Linux 8 does not use PT_GNU_PROPERTY and is probably
> the largest collection of CET-enabled binaries that exists today.
For clarity, RHEL is actively parsing these properties today?
> My hope was that we would backport the upstream kernel patches for CET,
> port the glibc dynamic loader to the new kernel interface, and be ready
> to run with CET enabled in principle (except that porting userspace
> libraries such as OpenSSL has not really started upstream, so many
> processes where CET is particularly desirable will still run without
> it).
>
> I'm not sure if it is a good idea to port the legacy support if it's not
> part of the mainline kernel because it comes awfully close to creating
> our own private ABI.
I guess we can aim to factor things so that PT_NOTE scanning is
available as a fallback on arches for which the absence of
PT_GNU_PROPERTY is not authoritative.
Can we argue that the lack of PT_GNU_PROPERTY is an ABI bug, fix it
for new binaries and hence limit the efforts we go to to support
theoretical binaries that lack the phdrs entry?
If we can make practical simplifications to the parsing, such as
limiting the maximum PT_NOTE size that we will search for the program
properties to 1K (say), or requiring NT_NOTE_GNU_PROPERTY_TYPE_0 to sit
by itself in a single PT_NOTE then that could help minimse the exec
overheads and the number of places for bugs to hide in the kernel.
What we can do here depends on what the tools currently do and what
binaries are out there.
Cheers
---Dave
^ permalink raw reply
* RE: [PATCH V14 1/5] dt-bindings: fsl: scu: add thermal binding
From: Aisheng Dong @ 2019-06-11 11:00 UTC (permalink / raw)
To: Anson Huang, robh+dt@kernel.org, mark.rutland@arm.com,
corbet@lwn.net, shawnguo@kernel.org, s.hauer@pengutronix.de,
kernel@pengutronix.de, festevam@gmail.com,
catalin.marinas@arm.com, will.deacon@arm.com, rui.zhang@intel.com,
edubezval@gmail.com, daniel.lezcano@linaro.org,
ulf.hansson@linaro.org, Peng Fan, mchehab+samsung@kernel.org,
linux@roeck-us.net, Daniel Baluta, maxime.ripard@bootlin.com,
olof@lixom.net, jagan@amarulasolutions.com,
horms+renesas@verge.net.au, Leonard Crestez,
bjorn.andersson@linaro.org, dinguyen@kernel.org,
enric.balletbo@collabora.com, devicetree@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-doc@vger.kernel.org,
linux-arm-kernel@lists.infradead.org, linux-pm@vger.kernel.org
Cc: dl-linux-imx
In-Reply-To: <20190610025254.23940-1-Anson.Huang@nxp.com>
> From: Anson.Huang@nxp.com [mailto:Anson.Huang@nxp.com]
> Sent: Monday, June 10, 2019 10:53 AM
>
> NXP i.MX8QXP is an ARMv8 SoC with a Cortex-M4 core inside as system
> controller, the system controller is in charge of system power, clock and
> thermal sensors etc. management, Linux kernel has to communicate with
> system controller via MU (message unit) IPC to get temperature from thermal
> sensors, this patch adds binding doc for i.MX system controller thermal driver.
>
> Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
> Reviewed-by: Rob Herring <robh@kernel.org>
Reviewed-by: Dong Aisheng <aisheng.dong@nxp.com>
Regards
Dong Aisheng
^ permalink raw reply
* Re: [PATCH v7 03/14] x86/cet/ibt: Add IBT legacy code bitmap setup function
From: Pavel Machek @ 2019-06-11 10:33 UTC (permalink / raw)
To: Yu-cheng Yu
Cc: Dave Hansen, Peter Zijlstra, 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, Randy Dunlap, Ravi V. Shankar, Vedvyas Shanbhogue,
Dave Martin
In-Reply-To: <e1543e7beb0eb55d6febcd847ccab9b219e60338.camel@intel.com>
[-- Attachment #1: Type: text/plain, Size: 1873 bytes --]
On Mon 2019-06-10 08:47:45, Yu-cheng Yu wrote:
> On Sat, 2019-06-08 at 22:52 +0200, Pavel Machek wrote:
> > Hi!
> >
> > > > 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.
> >
> > For the record, that sounds like a better interface than userspace knowing
> > about the bitmap formats...
> > Pavel
>
> Initially we implemented the bitmap that way. To manage the bitmap, every time
> the application issues a syscall for a .so it loads, and the kernel does
> copy_from_user() & copy_to_user() (or similar things). If a system has a few
> legacy .so files and every application does the same, it can take a long time to
> boot up.
Loading .so is already many syscalls, I'd not expect measurable
performance there. Are you sure?
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]
^ permalink raw reply
* Re: [PATCH v3 33/33] docs: EDID/HOWTO.txt: convert it and rename to howto.rst
From: Daniel Vetter @ 2019-06-11 9:40 UTC (permalink / raw)
To: Mauro Carvalho Chehab
Cc: Daniel Vetter, Linux Doc Mailing List, Mauro Carvalho Chehab,
linux-kernel, Jonathan Corbet, Maarten Lankhorst, Maxime Ripard,
Sean Paul, David Airlie, dri-devel
In-Reply-To: <20190611060215.232af2bb@coco.lan>
On Tue, Jun 11, 2019 at 06:02:15AM -0300, Mauro Carvalho Chehab wrote:
> Em Tue, 11 Jun 2019 10:37:31 +0200
> Daniel Vetter <daniel@ffwll.ch> escreveu:
>
> > On Sat, Jun 08, 2019 at 11:27:23PM -0300, Mauro Carvalho Chehab wrote:
> > > Sphinx need to know when a paragraph ends. So, do some adjustments
> > > at the file for it to be properly parsed.
> > >
> > > At its new index.rst, let's add a :orphan: while this is not linked to
> > > the main index.rst file, in order to avoid build warnings.
> > >
> > > that's said, I believe that this file should be moved to the
> > > GPU/DRM documentation.
> >
> > Yes, but there's a bit a twist: This is definitely end-user documentation,
> > so maybe should be in admin-guide?
> >
> > Atm all we have in Documentation/gpu/ is internals for drivers + some
> > beginnings of uapi documentation for userspace developers.
>
> On media, we have several different types of documents:
>
> - uAPI - consumed by both userspace and kernelspace developers;
> - kAPI - consumed by Kernel hackers;
> - driver-specific information. Those are usually messy, as some contain
> specific internal details, while others are pure end-user documentation.
>
> there are several cross-references between uAPI and kAPI parts.
>
> I've seem similar patterns on several other driver subsystems.
>
> I agree with Jon's principle that the best is to focus the book per
> audience. Yet, trying to rearrange the documentation means a lot of work,
> specially on those cases where a single file contain different types of
> documentation, like on media driver docs.
Yeah atm we're doing a bad job of keeping the kapi and uapi parts
separate. But the plan at least is to move all the gpu related uapi stuff
into Documentation/gpu/drm-uapi.rst. Not sure there's value in moving that
out of the gpu folder ...
> > Jon, what's your recommendation here for subsystem specific
> > end-user/adming docs?
>
> Jon, please correct me if I' wrong, bu I guess the plan is to place them
> somewhere under Documentation/admin-guide/.
>
> If so, perhaps creating a Documentation/admin-guide/drm dir there and
> place docs like EDID/HOWTO.txt, svga.txt, etc would work.
>
> Btw, that's one of the reasons[1] why I opted to keep the files where they
> are: properly organizing the converted documents call for such kind
> of discussions. On my experience, discussing names and directory locations
> can generate warm discussions and take a lot of time to reach consensus.
>
> [1] The other one is to avoid/simplify merge conflicts.
Oh definitely not asking for moving them at the same time, just wondering
how this should be solved properly.
-Daniel
>
> >
> > Thanks, Daniel
> >
> > >
> > > Signed-off-by: Mauro Carvalho Chehab <mchehab+samsung@kernel.org>
> > > ---
> > > Documentation/EDID/{HOWTO.txt => howto.rst} | 31 ++++++++++++-------
> > > .../admin-guide/kernel-parameters.txt | 2 +-
> > > drivers/gpu/drm/Kconfig | 2 +-
> > > 3 files changed, 22 insertions(+), 13 deletions(-)
> > > rename Documentation/EDID/{HOWTO.txt => howto.rst} (83%)
> > >
> > > diff --git a/Documentation/EDID/HOWTO.txt b/Documentation/EDID/howto.rst
> > > similarity index 83%
> > > rename from Documentation/EDID/HOWTO.txt
> > > rename to Documentation/EDID/howto.rst
> > > index 539871c3b785..725fd49a88ca 100644
> > > --- a/Documentation/EDID/HOWTO.txt
> > > +++ b/Documentation/EDID/howto.rst
> > > @@ -1,3 +1,9 @@
> > > +:orphan:
> > > +
> > > +====
> > > +EDID
> > > +====
> > > +
> > > In the good old days when graphics parameters were configured explicitly
> > > in a file called xorg.conf, even broken hardware could be managed.
> > >
> > > @@ -34,16 +40,19 @@ Makefile. Please note that the EDID data structure expects the timing
> > > values in a different way as compared to the standard X11 format.
> > >
> > > X11:
> > > -HTimings: hdisp hsyncstart hsyncend htotal
> > > -VTimings: vdisp vsyncstart vsyncend vtotal
> > > + HTimings:
> > > + hdisp hsyncstart hsyncend htotal
> > > + VTimings:
> > > + vdisp vsyncstart vsyncend vtotal
> > >
> > > -EDID:
> > > -#define XPIX hdisp
> > > -#define XBLANK htotal-hdisp
> > > -#define XOFFSET hsyncstart-hdisp
> > > -#define XPULSE hsyncend-hsyncstart
> > > +EDID::
> > >
> > > -#define YPIX vdisp
> > > -#define YBLANK vtotal-vdisp
> > > -#define YOFFSET vsyncstart-vdisp
> > > -#define YPULSE vsyncend-vsyncstart
> > > + #define XPIX hdisp
> > > + #define XBLANK htotal-hdisp
> > > + #define XOFFSET hsyncstart-hdisp
> > > + #define XPULSE hsyncend-hsyncstart
> > > +
> > > + #define YPIX vdisp
> > > + #define YBLANK vtotal-vdisp
> > > + #define YOFFSET vsyncstart-vdisp
> > > + #define YPULSE vsyncend-vsyncstart
> > > diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> > > index 3d072ca532bb..3faf37b8b001 100644
> > > --- a/Documentation/admin-guide/kernel-parameters.txt
> > > +++ b/Documentation/admin-guide/kernel-parameters.txt
> > > @@ -930,7 +930,7 @@
> > > edid/1680x1050.bin, or edid/1920x1080.bin is given
> > > and no file with the same name exists. Details and
> > > instructions how to build your own EDID data are
> > > - available in Documentation/EDID/HOWTO.txt. An EDID
> > > + available in Documentation/EDID/howto.rst. An EDID
> > > data set will only be used for a particular connector,
> > > if its name and a colon are prepended to the EDID
> > > name. Each connector may use a unique EDID data
> > > diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
> > > index 6b34949416b1..c3a6dd284c91 100644
> > > --- a/drivers/gpu/drm/Kconfig
> > > +++ b/drivers/gpu/drm/Kconfig
> > > @@ -141,7 +141,7 @@ config DRM_LOAD_EDID_FIRMWARE
> > > monitor are unable to provide appropriate EDID data. Since this
> > > feature is provided as a workaround for broken hardware, the
> > > default case is N. Details and instructions how to build your own
> > > - EDID data are given in Documentation/EDID/HOWTO.txt.
> > > + EDID data are given in Documentation/EDID/howto.rst.
> > >
> > > config DRM_DP_CEC
> > > bool "Enable DisplayPort CEC-Tunneling-over-AUX HDMI support"
> > > --
> > > 2.21.0
> > >
> >
>
>
>
> Thanks,
> Mauro
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
^ permalink raw reply
* Re: [PATCH v3 33/33] docs: EDID/HOWTO.txt: convert it and rename to howto.rst
From: Mauro Carvalho Chehab @ 2019-06-11 9:02 UTC (permalink / raw)
To: Daniel Vetter
Cc: Linux Doc Mailing List, Mauro Carvalho Chehab, linux-kernel,
Jonathan Corbet, Maarten Lankhorst, Maxime Ripard, Sean Paul,
David Airlie, dri-devel
In-Reply-To: <20190611083731.GS21222@phenom.ffwll.local>
Em Tue, 11 Jun 2019 10:37:31 +0200
Daniel Vetter <daniel@ffwll.ch> escreveu:
> On Sat, Jun 08, 2019 at 11:27:23PM -0300, Mauro Carvalho Chehab wrote:
> > Sphinx need to know when a paragraph ends. So, do some adjustments
> > at the file for it to be properly parsed.
> >
> > At its new index.rst, let's add a :orphan: while this is not linked to
> > the main index.rst file, in order to avoid build warnings.
> >
> > that's said, I believe that this file should be moved to the
> > GPU/DRM documentation.
>
> Yes, but there's a bit a twist: This is definitely end-user documentation,
> so maybe should be in admin-guide?
>
> Atm all we have in Documentation/gpu/ is internals for drivers + some
> beginnings of uapi documentation for userspace developers.
On media, we have several different types of documents:
- uAPI - consumed by both userspace and kernelspace developers;
- kAPI - consumed by Kernel hackers;
- driver-specific information. Those are usually messy, as some contain
specific internal details, while others are pure end-user documentation.
there are several cross-references between uAPI and kAPI parts.
I've seem similar patterns on several other driver subsystems.
I agree with Jon's principle that the best is to focus the book per
audience. Yet, trying to rearrange the documentation means a lot of work,
specially on those cases where a single file contain different types of
documentation, like on media driver docs.
> Jon, what's your recommendation here for subsystem specific
> end-user/adming docs?
Jon, please correct me if I' wrong, bu I guess the plan is to place them
somewhere under Documentation/admin-guide/.
If so, perhaps creating a Documentation/admin-guide/drm dir there and
place docs like EDID/HOWTO.txt, svga.txt, etc would work.
Btw, that's one of the reasons[1] why I opted to keep the files where they
are: properly organizing the converted documents call for such kind
of discussions. On my experience, discussing names and directory locations
can generate warm discussions and take a lot of time to reach consensus.
[1] The other one is to avoid/simplify merge conflicts.
>
> Thanks, Daniel
>
> >
> > Signed-off-by: Mauro Carvalho Chehab <mchehab+samsung@kernel.org>
> > ---
> > Documentation/EDID/{HOWTO.txt => howto.rst} | 31 ++++++++++++-------
> > .../admin-guide/kernel-parameters.txt | 2 +-
> > drivers/gpu/drm/Kconfig | 2 +-
> > 3 files changed, 22 insertions(+), 13 deletions(-)
> > rename Documentation/EDID/{HOWTO.txt => howto.rst} (83%)
> >
> > diff --git a/Documentation/EDID/HOWTO.txt b/Documentation/EDID/howto.rst
> > similarity index 83%
> > rename from Documentation/EDID/HOWTO.txt
> > rename to Documentation/EDID/howto.rst
> > index 539871c3b785..725fd49a88ca 100644
> > --- a/Documentation/EDID/HOWTO.txt
> > +++ b/Documentation/EDID/howto.rst
> > @@ -1,3 +1,9 @@
> > +:orphan:
> > +
> > +====
> > +EDID
> > +====
> > +
> > In the good old days when graphics parameters were configured explicitly
> > in a file called xorg.conf, even broken hardware could be managed.
> >
> > @@ -34,16 +40,19 @@ Makefile. Please note that the EDID data structure expects the timing
> > values in a different way as compared to the standard X11 format.
> >
> > X11:
> > -HTimings: hdisp hsyncstart hsyncend htotal
> > -VTimings: vdisp vsyncstart vsyncend vtotal
> > + HTimings:
> > + hdisp hsyncstart hsyncend htotal
> > + VTimings:
> > + vdisp vsyncstart vsyncend vtotal
> >
> > -EDID:
> > -#define XPIX hdisp
> > -#define XBLANK htotal-hdisp
> > -#define XOFFSET hsyncstart-hdisp
> > -#define XPULSE hsyncend-hsyncstart
> > +EDID::
> >
> > -#define YPIX vdisp
> > -#define YBLANK vtotal-vdisp
> > -#define YOFFSET vsyncstart-vdisp
> > -#define YPULSE vsyncend-vsyncstart
> > + #define XPIX hdisp
> > + #define XBLANK htotal-hdisp
> > + #define XOFFSET hsyncstart-hdisp
> > + #define XPULSE hsyncend-hsyncstart
> > +
> > + #define YPIX vdisp
> > + #define YBLANK vtotal-vdisp
> > + #define YOFFSET vsyncstart-vdisp
> > + #define YPULSE vsyncend-vsyncstart
> > diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> > index 3d072ca532bb..3faf37b8b001 100644
> > --- a/Documentation/admin-guide/kernel-parameters.txt
> > +++ b/Documentation/admin-guide/kernel-parameters.txt
> > @@ -930,7 +930,7 @@
> > edid/1680x1050.bin, or edid/1920x1080.bin is given
> > and no file with the same name exists. Details and
> > instructions how to build your own EDID data are
> > - available in Documentation/EDID/HOWTO.txt. An EDID
> > + available in Documentation/EDID/howto.rst. An EDID
> > data set will only be used for a particular connector,
> > if its name and a colon are prepended to the EDID
> > name. Each connector may use a unique EDID data
> > diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
> > index 6b34949416b1..c3a6dd284c91 100644
> > --- a/drivers/gpu/drm/Kconfig
> > +++ b/drivers/gpu/drm/Kconfig
> > @@ -141,7 +141,7 @@ config DRM_LOAD_EDID_FIRMWARE
> > monitor are unable to provide appropriate EDID data. Since this
> > feature is provided as a workaround for broken hardware, the
> > default case is N. Details and instructions how to build your own
> > - EDID data are given in Documentation/EDID/HOWTO.txt.
> > + EDID data are given in Documentation/EDID/howto.rst.
> >
> > config DRM_DP_CEC
> > bool "Enable DisplayPort CEC-Tunneling-over-AUX HDMI support"
> > --
> > 2.21.0
> >
>
Thanks,
Mauro
^ permalink raw reply
* Re: [PATCH v3 33/33] docs: EDID/HOWTO.txt: convert it and rename to howto.rst
From: Daniel Vetter @ 2019-06-11 8:37 UTC (permalink / raw)
To: Mauro Carvalho Chehab
Cc: Linux Doc Mailing List, Mauro Carvalho Chehab, linux-kernel,
Jonathan Corbet, Maarten Lankhorst, Maxime Ripard, Sean Paul,
David Airlie, Daniel Vetter, dri-devel
In-Reply-To: <74bec0b5b7c32c8d84adbaf9ff208803475198e5.1560045490.git.mchehab+samsung@kernel.org>
On Sat, Jun 08, 2019 at 11:27:23PM -0300, Mauro Carvalho Chehab wrote:
> Sphinx need to know when a paragraph ends. So, do some adjustments
> at the file for it to be properly parsed.
>
> At its new index.rst, let's add a :orphan: while this is not linked to
> the main index.rst file, in order to avoid build warnings.
>
> that's said, I believe that this file should be moved to the
> GPU/DRM documentation.
Yes, but there's a bit a twist: This is definitely end-user documentation,
so maybe should be in admin-guide?
Atm all we have in Documentation/gpu/ is internals for drivers + some
beginnings of uapi documentation for userspace developers.
Jon, what's your recommendation here for subsystem specific
end-user/adming docs?
Thanks, Daniel
>
> Signed-off-by: Mauro Carvalho Chehab <mchehab+samsung@kernel.org>
> ---
> Documentation/EDID/{HOWTO.txt => howto.rst} | 31 ++++++++++++-------
> .../admin-guide/kernel-parameters.txt | 2 +-
> drivers/gpu/drm/Kconfig | 2 +-
> 3 files changed, 22 insertions(+), 13 deletions(-)
> rename Documentation/EDID/{HOWTO.txt => howto.rst} (83%)
>
> diff --git a/Documentation/EDID/HOWTO.txt b/Documentation/EDID/howto.rst
> similarity index 83%
> rename from Documentation/EDID/HOWTO.txt
> rename to Documentation/EDID/howto.rst
> index 539871c3b785..725fd49a88ca 100644
> --- a/Documentation/EDID/HOWTO.txt
> +++ b/Documentation/EDID/howto.rst
> @@ -1,3 +1,9 @@
> +:orphan:
> +
> +====
> +EDID
> +====
> +
> In the good old days when graphics parameters were configured explicitly
> in a file called xorg.conf, even broken hardware could be managed.
>
> @@ -34,16 +40,19 @@ Makefile. Please note that the EDID data structure expects the timing
> values in a different way as compared to the standard X11 format.
>
> X11:
> -HTimings: hdisp hsyncstart hsyncend htotal
> -VTimings: vdisp vsyncstart vsyncend vtotal
> + HTimings:
> + hdisp hsyncstart hsyncend htotal
> + VTimings:
> + vdisp vsyncstart vsyncend vtotal
>
> -EDID:
> -#define XPIX hdisp
> -#define XBLANK htotal-hdisp
> -#define XOFFSET hsyncstart-hdisp
> -#define XPULSE hsyncend-hsyncstart
> +EDID::
>
> -#define YPIX vdisp
> -#define YBLANK vtotal-vdisp
> -#define YOFFSET vsyncstart-vdisp
> -#define YPULSE vsyncend-vsyncstart
> + #define XPIX hdisp
> + #define XBLANK htotal-hdisp
> + #define XOFFSET hsyncstart-hdisp
> + #define XPULSE hsyncend-hsyncstart
> +
> + #define YPIX vdisp
> + #define YBLANK vtotal-vdisp
> + #define YOFFSET vsyncstart-vdisp
> + #define YPULSE vsyncend-vsyncstart
> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> index 3d072ca532bb..3faf37b8b001 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -930,7 +930,7 @@
> edid/1680x1050.bin, or edid/1920x1080.bin is given
> and no file with the same name exists. Details and
> instructions how to build your own EDID data are
> - available in Documentation/EDID/HOWTO.txt. An EDID
> + available in Documentation/EDID/howto.rst. An EDID
> data set will only be used for a particular connector,
> if its name and a colon are prepended to the EDID
> name. Each connector may use a unique EDID data
> diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
> index 6b34949416b1..c3a6dd284c91 100644
> --- a/drivers/gpu/drm/Kconfig
> +++ b/drivers/gpu/drm/Kconfig
> @@ -141,7 +141,7 @@ config DRM_LOAD_EDID_FIRMWARE
> monitor are unable to provide appropriate EDID data. Since this
> feature is provided as a workaround for broken hardware, the
> default case is N. Details and instructions how to build your own
> - EDID data are given in Documentation/EDID/HOWTO.txt.
> + EDID data are given in Documentation/EDID/howto.rst.
>
> config DRM_DP_CEC
> bool "Enable DisplayPort CEC-Tunneling-over-AUX HDMI support"
> --
> 2.21.0
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
^ permalink raw reply
* Re: [PATCH v7 03/14] x86/cet/ibt: Add IBT legacy code bitmap setup function
From: Florian Weimer @ 2019-06-11 7:24 UTC (permalink / raw)
To: Dave Hansen
Cc: Yu-cheng Yu, Andy Lutomirski, 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, 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: <92e56b28-0cd4-e3f4-867b-639d9b98b86c@intel.com>
* Dave Hansen:
> My assumption has always been that these large, potentially sparse
> hardware tables *must* be mmap()'d with MAP_NORESERVE specified. That
> should keep them from being problematic with respect to overcommit.
MAP_NORESERVE pages still count towards the commit limit. The flag only
disables checks at allocation time, for this particular allocation. (At
least this was the behavior the last time I looked into this, I
believe.)
Not sure if this makes a difference here.
Thanks,
Florian
^ permalink raw reply
* [PATCH v11 11/13] ima: Define ima-modsig template
From: Thiago Jung Bauermann @ 2019-06-11 6:28 UTC (permalink / raw)
To: linux-integrity
Cc: linux-security-module, keyrings, linux-crypto, linuxppc-dev,
linux-doc, linux-kernel, Mimi Zohar, Dmitry Kasatkin,
James Morris, Serge E. Hallyn, David Howells, David Woodhouse,
Jessica Yu, Herbert Xu, David S. Miller, Jonathan Corbet,
AKASHI, Takahiro, Thiago Jung Bauermann
In-Reply-To: <20190611062817.18412-1-bauerman@linux.ibm.com>
Define new "d-modsig" template field which holds the digest that is
expected to match the one contained in the modsig, and also new "modsig"
template field which holds the appended file signature.
Add a new "ima-modsig" defined template descriptor with the new fields as
well as the ones from the "ima-sig" descriptor.
Change ima_store_measurement() to accept a struct modsig * argument so that
it can be passed along to the templates via struct ima_event_data.
Suggested-by: Mimi Zohar <zohar@linux.ibm.com>
Signed-off-by: Thiago Jung Bauermann <bauerman@linux.ibm.com>
Reviewed-by: Mimi Zohar <zohar@linux.ibm.com>
---
Documentation/security/IMA-templates.rst | 7 ++-
security/integrity/ima/ima.h | 20 +++++++-
security/integrity/ima/ima_api.c | 5 +-
security/integrity/ima/ima_main.c | 2 +-
security/integrity/ima/ima_modsig.c | 19 +++++++
security/integrity/ima/ima_policy.c | 41 ++++++++++++++++
security/integrity/ima/ima_template.c | 7 ++-
security/integrity/ima/ima_template_lib.c | 60 ++++++++++++++++++++++-
security/integrity/ima/ima_template_lib.h | 4 ++
9 files changed, 157 insertions(+), 8 deletions(-)
diff --git a/Documentation/security/IMA-templates.rst b/Documentation/security/IMA-templates.rst
index 2cd0e273cc9a..8da20b444be0 100644
--- a/Documentation/security/IMA-templates.rst
+++ b/Documentation/security/IMA-templates.rst
@@ -68,15 +68,18 @@ descriptors by adding their identifier to the format string
- 'd-ng': the digest of the event, calculated with an arbitrary hash
algorithm (field format: [<hash algo>:]digest, where the digest
prefix is shown only if the hash algorithm is not SHA1 or MD5);
+ - 'd-modsig': the digest of the event without the appended modsig;
- 'n-ng': the name of the event, without size limitations;
- - 'sig': the file signature.
+ - 'sig': the file signature;
+ - 'modsig' the appended file signature.
Below, there is the list of defined template descriptors:
- "ima": its format is ``d|n``;
- "ima-ng" (default): its format is ``d-ng|n-ng``;
- - "ima-sig": its format is ``d-ng|n-ng|sig``.
+ - "ima-sig": its format is ``d-ng|n-ng|sig``;
+ - "ima-modsig": its format is ``d-ng|n-ng|sig|d-modsig|modsig``.
diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index 0acc8e56ec73..a2b2c13ceda8 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -64,6 +64,7 @@ struct ima_event_data {
const unsigned char *filename;
struct evm_ima_xattr_data *xattr_value;
int xattr_len;
+ const struct modsig *modsig;
const char *violation;
};
@@ -207,7 +208,7 @@ int ima_collect_measurement(struct integrity_iint_cache *iint,
void ima_store_measurement(struct integrity_iint_cache *iint, struct file *file,
const unsigned char *filename,
struct evm_ima_xattr_data *xattr_value,
- int xattr_len, int pcr,
+ int xattr_len, const struct modsig *modsig, int pcr,
struct ima_template_desc *template_desc);
void ima_audit_measurement(struct integrity_iint_cache *iint,
const unsigned char *filename);
@@ -308,6 +309,10 @@ bool ima_hook_supports_modsig(enum ima_hooks func);
int ima_read_modsig(enum ima_hooks func, const void *buf, loff_t buf_len,
struct modsig **modsig);
void ima_collect_modsig(struct modsig *modsig, const void *buf, loff_t size);
+int ima_get_modsig_digest(const struct modsig *modsig, enum hash_algo *algo,
+ const u8 **digest, u32 *digest_size);
+int ima_get_raw_modsig(const struct modsig *modsig, const void **data,
+ u32 *data_len);
void ima_free_modsig(struct modsig *modsig);
#else
static inline bool ima_hook_supports_modsig(enum ima_hooks func)
@@ -326,6 +331,19 @@ static inline void ima_collect_modsig(struct modsig *modsig, const void *buf,
{
}
+static inline int ima_get_modsig_digest(const struct modsig *modsig,
+ enum hash_algo *algo, const u8 **digest,
+ u32 *digest_size)
+{
+ return -EOPNOTSUPP;
+}
+
+static inline int ima_get_raw_modsig(const struct modsig *modsig,
+ const void **data, u32 *data_len)
+{
+ return -EOPNOTSUPP;
+}
+
static inline void ima_free_modsig(struct modsig *modsig)
{
}
diff --git a/security/integrity/ima/ima_api.c b/security/integrity/ima/ima_api.c
index c351b8c37278..32297d1e6164 100644
--- a/security/integrity/ima/ima_api.c
+++ b/security/integrity/ima/ima_api.c
@@ -291,7 +291,7 @@ int ima_collect_measurement(struct integrity_iint_cache *iint,
void ima_store_measurement(struct integrity_iint_cache *iint,
struct file *file, const unsigned char *filename,
struct evm_ima_xattr_data *xattr_value,
- int xattr_len, int pcr,
+ int xattr_len, const struct modsig *modsig, int pcr,
struct ima_template_desc *template_desc)
{
static const char op[] = "add_template_measure";
@@ -303,7 +303,8 @@ void ima_store_measurement(struct integrity_iint_cache *iint,
.file = file,
.filename = filename,
.xattr_value = xattr_value,
- .xattr_len = xattr_len };
+ .xattr_len = xattr_len,
+ .modsig = modsig };
int violation = 0;
if (iint->measured_pcrs & (0x1 << pcr))
diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
index 2c9d3cf85726..85afb31fafe0 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -323,7 +323,7 @@ static int process_measurement(struct file *file, const struct cred *cred,
if (action & IMA_MEASURE)
ima_store_measurement(iint, file, pathname,
- xattr_value, xattr_len, pcr,
+ xattr_value, xattr_len, modsig, pcr,
template_desc);
if (rc == 0 && (action & IMA_APPRAISE_SUBMASK)) {
inode_lock(inode);
diff --git a/security/integrity/ima/ima_modsig.c b/security/integrity/ima/ima_modsig.c
index d438b87dba89..b01bbfeb1d98 100644
--- a/security/integrity/ima/ima_modsig.c
+++ b/security/integrity/ima/ima_modsig.c
@@ -140,6 +140,25 @@ int ima_modsig_verify(struct key *keyring, const struct modsig *modsig)
VERIFYING_MODULE_SIGNATURE, NULL, NULL);
}
+int ima_get_modsig_digest(const struct modsig *modsig, enum hash_algo *algo,
+ const u8 **digest, u32 *digest_size)
+{
+ *algo = modsig->hash_algo;
+ *digest = modsig->digest;
+ *digest_size = modsig->digest_size;
+
+ return 0;
+}
+
+int ima_get_raw_modsig(const struct modsig *modsig, const void **data,
+ u32 *data_len)
+{
+ *data = &modsig->raw_pkcs7;
+ *data_len = modsig->raw_pkcs7_len;
+
+ return 0;
+}
+
void ima_free_modsig(struct modsig *modsig)
{
if (!modsig)
diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
index f64ef84516db..6463ab8921ea 100644
--- a/security/integrity/ima/ima_policy.c
+++ b/security/integrity/ima/ima_policy.c
@@ -10,6 +10,9 @@
* - initialize default measure policy rules
*
*/
+
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
#include <linux/init.h>
#include <linux/list.h>
#include <linux/fs.h>
@@ -766,6 +769,38 @@ static void ima_log_string(struct audit_buffer *ab, char *key, char *value)
ima_log_string_op(ab, key, value, NULL);
}
+/*
+ * Validating the appended signature included in the measurement list requires
+ * the file hash calculated without the appended signature (i.e., the 'd-modsig'
+ * field). Therefore, notify the user if they have the 'modsig' field but not
+ * the 'd-modsig' field in the template.
+ */
+static void check_template_modsig(const struct ima_template_desc *template)
+{
+#define MSG "template with 'modsig' field also needs 'd-modsig' field\n"
+ bool has_modsig, has_dmodsig;
+ static bool checked;
+ int i;
+
+ /* We only need to notify the user once. */
+ if (checked)
+ return;
+
+ has_modsig = has_dmodsig = false;
+ for (i = 0; i < template->num_fields; i++) {
+ if (!strcmp(template->fields[i]->field_id, "modsig"))
+ has_modsig = true;
+ else if (!strcmp(template->fields[i]->field_id, "d-modsig"))
+ has_dmodsig = true;
+ }
+
+ if (has_modsig && !has_dmodsig)
+ pr_notice(MSG);
+
+ checked = true;
+#undef MSG
+}
+
static int ima_parse_rule(char *rule, struct ima_rule_entry *entry)
{
struct audit_buffer *ab;
@@ -1096,6 +1131,12 @@ static int ima_parse_rule(char *rule, struct ima_rule_entry *entry)
else if (entry->action == APPRAISE)
temp_ima_appraise |= ima_appraise_flag(entry->func);
+ if (!result && entry->flags & IMA_MODSIG_ALLOWED) {
+ template_desc = entry->template ? entry->template :
+ ima_template_desc_current();
+ check_template_modsig(template_desc);
+ }
+
audit_log_format(ab, "res=%d", !result);
audit_log_end(ab);
return result;
diff --git a/security/integrity/ima/ima_template.c b/security/integrity/ima/ima_template.c
index e6e892f31cbd..e25bef419c87 100644
--- a/security/integrity/ima/ima_template.c
+++ b/security/integrity/ima/ima_template.c
@@ -26,6 +26,7 @@ static struct ima_template_desc builtin_templates[] = {
{.name = IMA_TEMPLATE_IMA_NAME, .fmt = IMA_TEMPLATE_IMA_FMT},
{.name = "ima-ng", .fmt = "d-ng|n-ng"},
{.name = "ima-sig", .fmt = "d-ng|n-ng|sig"},
+ {.name = "ima-modsig", .fmt = "d-ng|n-ng|sig|d-modsig|modsig"},
{.name = "", .fmt = ""}, /* placeholder for a custom format */
};
@@ -43,8 +44,12 @@ static const struct ima_template_field supported_fields[] = {
.field_show = ima_show_template_string},
{.field_id = "sig", .field_init = ima_eventsig_init,
.field_show = ima_show_template_sig},
+ {.field_id = "d-modsig", .field_init = ima_eventdigest_modsig_init,
+ .field_show = ima_show_template_digest_ng},
+ {.field_id = "modsig", .field_init = ima_eventmodsig_init,
+ .field_show = ima_show_template_sig},
};
-#define MAX_TEMPLATE_NAME_LEN 15
+#define MAX_TEMPLATE_NAME_LEN sizeof("d|n|d-ng|n-ng|sig|d-modisg|modsig")
static struct ima_template_desc *ima_template;
static int template_desc_init_fields(const char *template_fmt,
diff --git a/security/integrity/ima/ima_template_lib.c b/security/integrity/ima/ima_template_lib.c
index 513b457ae900..dacb01fb105f 100644
--- a/security/integrity/ima/ima_template_lib.c
+++ b/security/integrity/ima/ima_template_lib.c
@@ -223,7 +223,8 @@ int ima_parse_buf(void *bufstartp, void *bufendp, void **bufcurp,
return 0;
}
-static int ima_eventdigest_init_common(u8 *digest, u32 digestsize, u8 hash_algo,
+static int ima_eventdigest_init_common(const u8 *digest, u32 digestsize,
+ u8 hash_algo,
struct ima_field_data *field_data)
{
/*
@@ -326,6 +327,41 @@ int ima_eventdigest_ng_init(struct ima_event_data *event_data,
hash_algo, field_data);
}
+/*
+ * This function writes the digest of the file which is expected to match the
+ * digest contained in the file's embedded signature.
+ */
+int ima_eventdigest_modsig_init(struct ima_event_data *event_data,
+ struct ima_field_data *field_data)
+{
+ enum hash_algo hash_algo;
+ const u8 *cur_digest;
+ u32 cur_digestsize;
+
+ if (!event_data->modsig)
+ return 0;
+
+ if (event_data->violation) {
+ /* Recording a violation. */
+ hash_algo = HASH_ALGO_SHA1;
+ cur_digest = NULL;
+ cur_digestsize = 0;
+ } else {
+ int rc;
+
+ rc = ima_get_modsig_digest(event_data->modsig, &hash_algo,
+ &cur_digest, &cur_digestsize);
+ if (rc)
+ return rc;
+ else if (hash_algo == HASH_ALGO__LAST || cur_digestsize == 0)
+ /* There was some error collecting the digest. */
+ return -EINVAL;
+ }
+
+ return ima_eventdigest_init_common(cur_digest, cur_digestsize,
+ hash_algo, field_data);
+}
+
static int ima_eventname_init_common(struct ima_event_data *event_data,
struct ima_field_data *field_data,
bool size_limit)
@@ -389,3 +425,25 @@ int ima_eventsig_init(struct ima_event_data *event_data,
return ima_write_template_field_data(xattr_value, event_data->xattr_len,
DATA_FMT_HEX, field_data);
}
+
+int ima_eventmodsig_init(struct ima_event_data *event_data,
+ struct ima_field_data *field_data)
+{
+ const void *data;
+ u32 data_len;
+ int rc;
+
+ if (!event_data->modsig)
+ return 0;
+
+ /*
+ * modsig is a runtime structure containing pointers. Get its raw data
+ * instead.
+ */
+ rc = ima_get_raw_modsig(event_data->modsig, &data, &data_len);
+ if (rc)
+ return rc;
+
+ return ima_write_template_field_data(data, data_len, DATA_FMT_HEX,
+ field_data);
+}
diff --git a/security/integrity/ima/ima_template_lib.h b/security/integrity/ima/ima_template_lib.h
index 6a3d8b831deb..1d7c690ebae5 100644
--- a/security/integrity/ima/ima_template_lib.h
+++ b/security/integrity/ima/ima_template_lib.h
@@ -38,8 +38,12 @@ int ima_eventname_init(struct ima_event_data *event_data,
struct ima_field_data *field_data);
int ima_eventdigest_ng_init(struct ima_event_data *event_data,
struct ima_field_data *field_data);
+int ima_eventdigest_modsig_init(struct ima_event_data *event_data,
+ struct ima_field_data *field_data);
int ima_eventname_ng_init(struct ima_event_data *event_data,
struct ima_field_data *field_data);
int ima_eventsig_init(struct ima_event_data *event_data,
struct ima_field_data *field_data);
+int ima_eventmodsig_init(struct ima_event_data *event_data,
+ struct ima_field_data *field_data);
#endif /* __LINUX_IMA_TEMPLATE_LIB_H */
^ permalink raw reply related
* [PATCH v11 13/13] ima: Allow template= option for appraise rules as well
From: Thiago Jung Bauermann @ 2019-06-11 6:28 UTC (permalink / raw)
To: linux-integrity
Cc: linux-security-module, keyrings, linux-crypto, linuxppc-dev,
linux-doc, linux-kernel, Mimi Zohar, Dmitry Kasatkin,
James Morris, Serge E. Hallyn, David Howells, David Woodhouse,
Jessica Yu, Herbert Xu, David S. Miller, Jonathan Corbet,
AKASHI, Takahiro, Thiago Jung Bauermann
In-Reply-To: <20190611062817.18412-1-bauerman@linux.ibm.com>
It's useful being able to specify a different IMA template on appraise
policy rules, so allow it.
Signed-off-by: Thiago Jung Bauermann <bauerman@linux.ibm.com>
Suggested-by: Mimi Zohar <zohar@linux.ibm.com>
---
security/integrity/ima/ima_policy.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
index 6463ab8921ea..1ac1ef458f2e 100644
--- a/security/integrity/ima/ima_policy.c
+++ b/security/integrity/ima/ima_policy.c
@@ -1110,7 +1110,8 @@ static int ima_parse_rule(char *rule, struct ima_rule_entry *entry)
break;
case Opt_template:
ima_log_string(ab, "template", args[0].from);
- if (entry->action != MEASURE) {
+ if (entry->action != MEASURE &&
+ entry->action != APPRAISE) {
result = -EINVAL;
break;
}
^ permalink raw reply related
* [PATCH v11 12/13] ima: Store the measurement again when appraising a modsig
From: Thiago Jung Bauermann @ 2019-06-11 6:28 UTC (permalink / raw)
To: linux-integrity
Cc: linux-security-module, keyrings, linux-crypto, linuxppc-dev,
linux-doc, linux-kernel, Mimi Zohar, Dmitry Kasatkin,
James Morris, Serge E. Hallyn, David Howells, David Woodhouse,
Jessica Yu, Herbert Xu, David S. Miller, Jonathan Corbet,
AKASHI, Takahiro, Thiago Jung Bauermann
In-Reply-To: <20190611062817.18412-1-bauerman@linux.ibm.com>
If the IMA template contains the "modsig" or "d-modsig" field, then the
modsig should be added to the measurement list when the file is appraised.
And that is what normally happens, but if a measurement rule caused a file
containing a modsig to be measured before a different rule causes it to be
appraised, the resulting measurement entry will not contain the modsig
because it is only fetched during appraisal. When the appraisal rule
triggers, it won't store a new measurement containing the modsig because
the file was already measured.
We need to detect that situation and store an additional measurement with
the modsig. This is done by adding an IMA_MEASURE action flag if we read a
modsig and the IMA template contains a modsig field.
Suggested-by: Mimi Zohar <zohar@linux.ibm.com>
Signed-off-by: Thiago Jung Bauermann <bauerman@linux.ibm.com>
---
security/integrity/ima/ima.h | 1 +
security/integrity/ima/ima_api.c | 19 +++++++++++++++----
security/integrity/ima/ima_main.c | 15 ++++++++++++---
security/integrity/ima/ima_template.c | 19 +++++++++++++++++++
4 files changed, 47 insertions(+), 7 deletions(-)
diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index a2b2c13ceda8..44f5f60424c2 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -149,6 +149,7 @@ void ima_putc(struct seq_file *m, void *data, int datalen);
void ima_print_digest(struct seq_file *m, u8 *digest, u32 size);
struct ima_template_desc *ima_template_desc_current(void);
struct ima_template_desc *lookup_template_desc(const char *name);
+bool ima_template_has_modsig(const struct ima_template_desc *ima_template);
int ima_restore_measurement_entry(struct ima_template_entry *entry);
int ima_restore_measurement_list(loff_t bufsize, void *buf);
int ima_measurements_show(struct seq_file *m, void *v);
diff --git a/security/integrity/ima/ima_api.c b/security/integrity/ima/ima_api.c
index 32297d1e6164..bb887ed3d8a7 100644
--- a/security/integrity/ima/ima_api.c
+++ b/security/integrity/ima/ima_api.c
@@ -222,6 +222,14 @@ int ima_collect_measurement(struct integrity_iint_cache *iint,
char digest[IMA_MAX_DIGEST_SIZE];
} hash;
+ /*
+ * Always collect the modsig, because IMA might have already collected
+ * the file digest without collecting the modsig in a previous
+ * measurement rule.
+ */
+ if (modsig)
+ ima_collect_modsig(modsig, buf, size);
+
if (iint->flags & IMA_COLLECTED)
goto out;
@@ -255,9 +263,6 @@ int ima_collect_measurement(struct integrity_iint_cache *iint,
memcpy(iint->ima_hash, &hash, length);
iint->version = i_version;
- if (modsig)
- ima_collect_modsig(modsig, buf, size);
-
/* Possibly temporary failure due to type of read (eg. O_DIRECT) */
if (!result)
iint->flags |= IMA_COLLECTED;
@@ -307,7 +312,13 @@ void ima_store_measurement(struct integrity_iint_cache *iint,
.modsig = modsig };
int violation = 0;
- if (iint->measured_pcrs & (0x1 << pcr))
+ /*
+ * We still need to store the measurement in the case of MODSIG because
+ * we only have its contents to put in the list at the time of
+ * appraisal, but a file measurement from earlier might already exist in
+ * the measurement list.
+ */
+ if (iint->measured_pcrs & (0x1 << pcr) && !modsig)
return;
result = ima_alloc_init_template(&event_data, &entry, template_desc);
diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
index 85afb31fafe0..e0ca39f81a59 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -307,9 +307,18 @@ static int process_measurement(struct file *file, const struct cred *cred,
/* read 'security.ima' */
xattr_len = ima_read_xattr(file_dentry(file), &xattr_value);
- /* Read the appended modsig if allowed by the policy. */
- if (iint->flags & IMA_MODSIG_ALLOWED)
- ima_read_modsig(func, buf, size, &modsig);
+ /*
+ * Read the appended modsig if allowed by the policy, and allow
+ * an additional measurement list entry, if needed, based on the
+ * template format and whether the file was already measured.
+ */
+ if (iint->flags & IMA_MODSIG_ALLOWED) {
+ rc = ima_read_modsig(func, buf, size, &modsig);
+
+ if (!rc && ima_template_has_modsig(template_desc) &&
+ iint->flags & IMA_MEASURED)
+ action |= IMA_MEASURE;
+ }
}
hash_algo = ima_get_hash_algo(xattr_value, xattr_len);
diff --git a/security/integrity/ima/ima_template.c b/security/integrity/ima/ima_template.c
index e25bef419c87..00d9a6cc8a60 100644
--- a/security/integrity/ima/ima_template.c
+++ b/security/integrity/ima/ima_template.c
@@ -56,6 +56,25 @@ static int template_desc_init_fields(const char *template_fmt,
const struct ima_template_field ***fields,
int *num_fields);
+/**
+ * ima_template_has_modsig - Check whether template has modsig-related fields.
+ * @ima_template: IMA template to check.
+ *
+ * Tells whether the given template has fields referencing a file's appended
+ * signature.
+ */
+bool ima_template_has_modsig(const struct ima_template_desc *ima_template)
+{
+ int i;
+
+ for (i = 0; i < ima_template->num_fields; i++)
+ if (!strcmp(ima_template->fields[i]->field_id, "modsig") ||
+ !strcmp(ima_template->fields[i]->field_id, "d-modsig"))
+ return true;
+
+ return false;
+}
+
static int __init ima_template_setup(char *str)
{
struct ima_template_desc *template_desc;
^ permalink raw reply related
* [PATCH v11 08/13] ima: Factor xattr_verify() out of ima_appraise_measurement()
From: Thiago Jung Bauermann @ 2019-06-11 6:28 UTC (permalink / raw)
To: linux-integrity
Cc: linux-security-module, keyrings, linux-crypto, linuxppc-dev,
linux-doc, linux-kernel, Mimi Zohar, Dmitry Kasatkin,
James Morris, Serge E. Hallyn, David Howells, David Woodhouse,
Jessica Yu, Herbert Xu, David S. Miller, Jonathan Corbet,
AKASHI, Takahiro, Thiago Jung Bauermann
In-Reply-To: <20190611062817.18412-1-bauerman@linux.ibm.com>
Verify xattr signature in a separate function so that the logic in
ima_appraise_measurement() remains clear when it gains the ability to also
verify an appended module signature.
The code in the switch statement is unchanged except for having to
dereference the status and cause variables (since they're now pointers),
and fixing the style of a block comment to appease checkpatch.
Suggested-by: Mimi Zohar <zohar@linux.ibm.com>
Signed-off-by: Thiago Jung Bauermann <bauerman@linux.ibm.com>
Reviewed-by: Mimi Zohar <zohar@linux.ibm.com>
---
security/integrity/ima/ima_appraise.c | 141 +++++++++++++++-----------
1 file changed, 81 insertions(+), 60 deletions(-)
diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c
index 18bbe753421a..5d4772f39757 100644
--- a/security/integrity/ima/ima_appraise.c
+++ b/security/integrity/ima/ima_appraise.c
@@ -202,6 +202,83 @@ int ima_read_xattr(struct dentry *dentry,
return ret;
}
+/*
+ * xattr_verify - verify xattr digest or signature
+ *
+ * Verify whether the hash or signature matches the file contents.
+ *
+ * Return 0 on success, error code otherwise.
+ */
+static int xattr_verify(enum ima_hooks func, struct integrity_iint_cache *iint,
+ struct evm_ima_xattr_data *xattr_value, int xattr_len,
+ enum integrity_status *status, const char **cause)
+{
+ int rc = -EINVAL, hash_start = 0;
+
+ switch (xattr_value->type) {
+ case IMA_XATTR_DIGEST_NG:
+ /* first byte contains algorithm id */
+ hash_start = 1;
+ /* fall through */
+ case IMA_XATTR_DIGEST:
+ if (iint->flags & IMA_DIGSIG_REQUIRED) {
+ *cause = "IMA-signature-required";
+ *status = INTEGRITY_FAIL;
+ break;
+ }
+ clear_bit(IMA_DIGSIG, &iint->atomic_flags);
+ if (xattr_len - sizeof(xattr_value->type) - hash_start >=
+ iint->ima_hash->length)
+ /*
+ * xattr length may be longer. md5 hash in previous
+ * version occupied 20 bytes in xattr, instead of 16
+ */
+ rc = memcmp(&xattr_value->data[hash_start],
+ iint->ima_hash->digest,
+ iint->ima_hash->length);
+ else
+ rc = -EINVAL;
+ if (rc) {
+ *cause = "invalid-hash";
+ *status = INTEGRITY_FAIL;
+ break;
+ }
+ *status = INTEGRITY_PASS;
+ break;
+ case EVM_IMA_XATTR_DIGSIG:
+ set_bit(IMA_DIGSIG, &iint->atomic_flags);
+ rc = integrity_digsig_verify(INTEGRITY_KEYRING_IMA,
+ (const char *)xattr_value,
+ xattr_len,
+ iint->ima_hash->digest,
+ iint->ima_hash->length);
+ if (rc == -EOPNOTSUPP) {
+ *status = INTEGRITY_UNKNOWN;
+ break;
+ }
+ if (IS_ENABLED(CONFIG_INTEGRITY_PLATFORM_KEYRING) && rc &&
+ func == KEXEC_KERNEL_CHECK)
+ rc = integrity_digsig_verify(INTEGRITY_KEYRING_PLATFORM,
+ (const char *)xattr_value,
+ xattr_len,
+ iint->ima_hash->digest,
+ iint->ima_hash->length);
+ if (rc) {
+ *cause = "invalid-signature";
+ *status = INTEGRITY_FAIL;
+ } else {
+ *status = INTEGRITY_PASS;
+ }
+ break;
+ default:
+ *status = INTEGRITY_UNKNOWN;
+ *cause = "unknown-ima-data";
+ break;
+ }
+
+ return rc;
+}
+
/*
* ima_appraise_measurement - appraise file measurement
*
@@ -221,7 +298,7 @@ int ima_appraise_measurement(enum ima_hooks func,
struct dentry *dentry = file_dentry(file);
struct inode *inode = d_backing_inode(dentry);
enum integrity_status status = INTEGRITY_UNKNOWN;
- int rc = xattr_len, hash_start = 0;
+ int rc = xattr_len;
if (!(inode->i_opflags & IOP_XATTR))
return INTEGRITY_UNKNOWN;
@@ -259,65 +336,9 @@ int ima_appraise_measurement(enum ima_hooks func,
WARN_ONCE(true, "Unexpected integrity status %d\n", status);
}
- switch (xattr_value->type) {
- case IMA_XATTR_DIGEST_NG:
- /* first byte contains algorithm id */
- hash_start = 1;
- /* fall through */
- case IMA_XATTR_DIGEST:
- if (iint->flags & IMA_DIGSIG_REQUIRED) {
- cause = "IMA-signature-required";
- status = INTEGRITY_FAIL;
- break;
- }
- clear_bit(IMA_DIGSIG, &iint->atomic_flags);
- if (xattr_len - sizeof(xattr_value->type) - hash_start >=
- iint->ima_hash->length)
- /* xattr length may be longer. md5 hash in previous
- version occupied 20 bytes in xattr, instead of 16
- */
- rc = memcmp(&xattr_value->data[hash_start],
- iint->ima_hash->digest,
- iint->ima_hash->length);
- else
- rc = -EINVAL;
- if (rc) {
- cause = "invalid-hash";
- status = INTEGRITY_FAIL;
- break;
- }
- status = INTEGRITY_PASS;
- break;
- case EVM_IMA_XATTR_DIGSIG:
- set_bit(IMA_DIGSIG, &iint->atomic_flags);
- rc = integrity_digsig_verify(INTEGRITY_KEYRING_IMA,
- (const char *)xattr_value,
- xattr_len,
- iint->ima_hash->digest,
- iint->ima_hash->length);
- if (rc == -EOPNOTSUPP) {
- status = INTEGRITY_UNKNOWN;
- break;
- }
- if (IS_ENABLED(CONFIG_INTEGRITY_PLATFORM_KEYRING) && rc &&
- func == KEXEC_KERNEL_CHECK)
- rc = integrity_digsig_verify(INTEGRITY_KEYRING_PLATFORM,
- (const char *)xattr_value,
- xattr_len,
- iint->ima_hash->digest,
- iint->ima_hash->length);
- if (rc) {
- cause = "invalid-signature";
- status = INTEGRITY_FAIL;
- } else {
- status = INTEGRITY_PASS;
- }
- break;
- default:
- status = INTEGRITY_UNKNOWN;
- cause = "unknown-ima-data";
- break;
- }
+ if (xattr_value)
+ rc = xattr_verify(func, iint, xattr_value, xattr_len, &status,
+ &cause);
out:
/*
^ permalink raw reply related
* [PATCH v11 05/13] integrity: Select CONFIG_KEYS instead of depending on it
From: Thiago Jung Bauermann @ 2019-06-11 6:28 UTC (permalink / raw)
To: linux-integrity
Cc: linux-security-module, keyrings, linux-crypto, linuxppc-dev,
linux-doc, linux-kernel, Mimi Zohar, Dmitry Kasatkin,
James Morris, Serge E. Hallyn, David Howells, David Woodhouse,
Jessica Yu, Herbert Xu, David S. Miller, Jonathan Corbet,
AKASHI, Takahiro, Thiago Jung Bauermann
In-Reply-To: <20190611062817.18412-1-bauerman@linux.ibm.com>
This avoids a dependency cycle in soon-to-be-introduced
CONFIG_IMA_APPRAISE_MODSIG: it will select CONFIG_MODULE_SIG_FORMAT
which in turn selects CONFIG_KEYS. Kconfig then complains that
CONFIG_INTEGRITY_SIGNATURE depends on CONFIG_KEYS.
Signed-off-by: Thiago Jung Bauermann <bauerman@linux.ibm.com>
Signed-off-by: Mimi Zohar <zohar@linux.ibm.com>
---
security/integrity/Kconfig | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/security/integrity/Kconfig b/security/integrity/Kconfig
index 3ba1168b1756..93d73902c571 100644
--- a/security/integrity/Kconfig
+++ b/security/integrity/Kconfig
@@ -17,8 +17,8 @@ if INTEGRITY
config INTEGRITY_SIGNATURE
bool "Digital signature verification using multiple keyrings"
- depends on KEYS
default n
+ select KEYS
select SIGNATURE
help
This option enables digital signature verification support
^ permalink raw reply related
* [PATCH v11 10/13] ima: Collect modsig
From: Thiago Jung Bauermann @ 2019-06-11 6:28 UTC (permalink / raw)
To: linux-integrity
Cc: linux-security-module, keyrings, linux-crypto, linuxppc-dev,
linux-doc, linux-kernel, Mimi Zohar, Dmitry Kasatkin,
James Morris, Serge E. Hallyn, David Howells, David Woodhouse,
Jessica Yu, Herbert Xu, David S. Miller, Jonathan Corbet,
AKASHI, Takahiro, Thiago Jung Bauermann
In-Reply-To: <20190611062817.18412-1-bauerman@linux.ibm.com>
Obtain the modsig and calculate its corresponding hash in
ima_collect_measurement().
Signed-off-by: Thiago Jung Bauermann <bauerman@linux.ibm.com>
---
security/integrity/ima/ima.h | 8 ++++-
security/integrity/ima/ima_api.c | 5 ++-
security/integrity/ima/ima_appraise.c | 2 +-
security/integrity/ima/ima_main.c | 2 +-
security/integrity/ima/ima_modsig.c | 50 ++++++++++++++++++++++++++-
5 files changed, 62 insertions(+), 5 deletions(-)
diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index ebbfae10f174..0acc8e56ec73 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -203,7 +203,7 @@ int ima_get_action(struct inode *inode, const struct cred *cred, u32 secid,
int ima_must_measure(struct inode *inode, int mask, enum ima_hooks func);
int ima_collect_measurement(struct integrity_iint_cache *iint,
struct file *file, void *buf, loff_t size,
- enum hash_algo algo);
+ enum hash_algo algo, struct modsig *modsig);
void ima_store_measurement(struct integrity_iint_cache *iint, struct file *file,
const unsigned char *filename,
struct evm_ima_xattr_data *xattr_value,
@@ -307,6 +307,7 @@ static inline int ima_read_xattr(struct dentry *dentry,
bool ima_hook_supports_modsig(enum ima_hooks func);
int ima_read_modsig(enum ima_hooks func, const void *buf, loff_t buf_len,
struct modsig **modsig);
+void ima_collect_modsig(struct modsig *modsig, const void *buf, loff_t size);
void ima_free_modsig(struct modsig *modsig);
#else
static inline bool ima_hook_supports_modsig(enum ima_hooks func)
@@ -320,6 +321,11 @@ static inline int ima_read_modsig(enum ima_hooks func, const void *buf,
return -EOPNOTSUPP;
}
+static inline void ima_collect_modsig(struct modsig *modsig, const void *buf,
+ loff_t size)
+{
+}
+
static inline void ima_free_modsig(struct modsig *modsig)
{
}
diff --git a/security/integrity/ima/ima_api.c b/security/integrity/ima/ima_api.c
index c0cf4bcfc82f..c351b8c37278 100644
--- a/security/integrity/ima/ima_api.c
+++ b/security/integrity/ima/ima_api.c
@@ -208,7 +208,7 @@ int ima_get_action(struct inode *inode, const struct cred *cred, u32 secid,
*/
int ima_collect_measurement(struct integrity_iint_cache *iint,
struct file *file, void *buf, loff_t size,
- enum hash_algo algo)
+ enum hash_algo algo, struct modsig *modsig)
{
const char *audit_cause = "failed";
struct inode *inode = file_inode(file);
@@ -255,6 +255,9 @@ int ima_collect_measurement(struct integrity_iint_cache *iint,
memcpy(iint->ima_hash, &hash, length);
iint->version = i_version;
+ if (modsig)
+ ima_collect_modsig(modsig, buf, size);
+
/* Possibly temporary failure due to type of read (eg. O_DIRECT) */
if (!result)
iint->flags |= IMA_COLLECTED;
diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c
index 70252ac3321d..aa14e3fe25d5 100644
--- a/security/integrity/ima/ima_appraise.c
+++ b/security/integrity/ima/ima_appraise.c
@@ -438,7 +438,7 @@ void ima_update_xattr(struct integrity_iint_cache *iint, struct file *file)
!(iint->flags & IMA_HASH))
return;
- rc = ima_collect_measurement(iint, file, NULL, 0, ima_hash_algo);
+ rc = ima_collect_measurement(iint, file, NULL, 0, ima_hash_algo, NULL);
if (rc < 0)
return;
diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
index 8ddf9faa8d02..2c9d3cf85726 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -314,7 +314,7 @@ static int process_measurement(struct file *file, const struct cred *cred,
hash_algo = ima_get_hash_algo(xattr_value, xattr_len);
- rc = ima_collect_measurement(iint, file, buf, size, hash_algo);
+ rc = ima_collect_measurement(iint, file, buf, size, hash_algo, modsig);
if (rc != 0 && rc != -EBADF && rc != -EINVAL)
goto out_locked;
diff --git a/security/integrity/ima/ima_modsig.c b/security/integrity/ima/ima_modsig.c
index f41ebe370fa0..d438b87dba89 100644
--- a/security/integrity/ima/ima_modsig.c
+++ b/security/integrity/ima/ima_modsig.c
@@ -17,6 +17,19 @@
struct modsig {
struct pkcs7_message *pkcs7_msg;
+
+ enum hash_algo hash_algo;
+
+ /* This digest will go in the 'd-modsig' field of the IMA template. */
+ const u8 *digest;
+ u32 digest_size;
+
+ /*
+ * This is what will go to the measurement list if the template requires
+ * storing the signature.
+ */
+ int raw_pkcs7_len;
+ u8 raw_pkcs7[];
};
/**
@@ -71,7 +84,8 @@ int ima_read_modsig(enum ima_hooks func, const void *buf, loff_t buf_len,
sig_len = be32_to_cpu(sig->sig_len);
buf_len -= sig_len + sizeof(*sig);
- hdr = kmalloc(sizeof(*hdr), GFP_KERNEL);
+ /* Allocate sig_len additional bytes to hold the raw PKCS#7 data. */
+ hdr = kzalloc(sizeof(*hdr) + sig_len, GFP_KERNEL);
if (!hdr)
return -ENOMEM;
@@ -81,11 +95,45 @@ int ima_read_modsig(enum ima_hooks func, const void *buf, loff_t buf_len,
return PTR_ERR(hdr->pkcs7_msg);
}
+ memcpy(hdr->raw_pkcs7, buf + buf_len, sig_len);
+ hdr->raw_pkcs7_len = sig_len;
+
+ /* We don't know the hash algorithm yet. */
+ hdr->hash_algo = HASH_ALGO__LAST;
+
*modsig = hdr;
return 0;
}
+/**
+ * ima_collect_modsig - Calculate the file hash without the appended signature.
+ *
+ * Since the modsig is part of the file contents, the hash used in its signature
+ * isn't the same one ordinarily calculated by IMA. Therefore PKCS7 code
+ * calculates a separate one for signature verification.
+ */
+void ima_collect_modsig(struct modsig *modsig, const void *buf, loff_t size)
+{
+ int rc;
+
+ /*
+ * Provide the file contents (minus the appended sig) so that the PKCS7
+ * code can calculate the file hash.
+ */
+ size -= modsig->raw_pkcs7_len + strlen(MODULE_SIG_STRING) +
+ sizeof(struct module_signature);
+ rc = pkcs7_supply_detached_data(modsig->pkcs7_msg, buf, size);
+ if (rc)
+ return;
+
+ /* Ask the PKCS7 code to calculate the file hash. */
+ rc = pkcs7_get_digest(modsig->pkcs7_msg, &modsig->digest,
+ &modsig->digest_size, &modsig->hash_algo);
+ if (rc)
+ return;
+}
+
int ima_modsig_verify(struct key *keyring, const struct modsig *modsig)
{
return verify_pkcs7_message_sig(NULL, 0, modsig->pkcs7_msg, keyring,
^ permalink raw reply related
* [PATCH v11 09/13] ima: Implement support for module-style appended signatures
From: Thiago Jung Bauermann @ 2019-06-11 6:28 UTC (permalink / raw)
To: linux-integrity
Cc: linux-security-module, keyrings, linux-crypto, linuxppc-dev,
linux-doc, linux-kernel, Mimi Zohar, Dmitry Kasatkin,
James Morris, Serge E. Hallyn, David Howells, David Woodhouse,
Jessica Yu, Herbert Xu, David S. Miller, Jonathan Corbet,
AKASHI, Takahiro, Thiago Jung Bauermann
In-Reply-To: <20190611062817.18412-1-bauerman@linux.ibm.com>
Implement the appraise_type=imasig|modsig option, allowing IMA to read and
verify modsig signatures.
In case a file has both an xattr signature and an appended modsig, IMA will
only use the appended signature if the key used by the xattr signature
isn't present in the IMA or platform keyring.
Because modsig verification needs to convert from an integrity keyring id
to the keyring itself, add an integrity_keyring_from_id() function in
digsig.c so that integrity_modsig_verify() can use it.
Signed-off-by: Thiago Jung Bauermann <bauerman@linux.ibm.com>
Signed-off-by: Mimi Zohar <zohar@linux.ibm.com>
---
security/integrity/digsig.c | 43 ++++++++++++----
security/integrity/ima/Kconfig | 3 ++
security/integrity/ima/ima.h | 22 ++++++++-
security/integrity/ima/ima_appraise.c | 51 +++++++++++++++++--
security/integrity/ima/ima_main.c | 11 ++++-
security/integrity/ima/ima_modsig.c | 71 +++++++++++++++++++++++++++
security/integrity/ima/ima_policy.c | 12 ++---
security/integrity/integrity.h | 19 +++++++
8 files changed, 209 insertions(+), 23 deletions(-)
diff --git a/security/integrity/digsig.c b/security/integrity/digsig.c
index e19c2eb72c51..3399a7e32830 100644
--- a/security/integrity/digsig.c
+++ b/security/integrity/digsig.c
@@ -43,11 +43,10 @@ static const char * const keyring_name[INTEGRITY_KEYRING_MAX] = {
#define restrict_link_to_ima restrict_link_by_builtin_trusted
#endif
-int integrity_digsig_verify(const unsigned int id, const char *sig, int siglen,
- const char *digest, int digestlen)
+static struct key *integrity_keyring_from_id(const unsigned int id)
{
- if (id >= INTEGRITY_KEYRING_MAX || siglen < 2)
- return -EINVAL;
+ if (id >= INTEGRITY_KEYRING_MAX)
+ return ERR_PTR(-EINVAL);
if (!keyring[id]) {
keyring[id] =
@@ -56,23 +55,49 @@ int integrity_digsig_verify(const unsigned int id, const char *sig, int siglen,
int err = PTR_ERR(keyring[id]);
pr_err("no %s keyring: %d\n", keyring_name[id], err);
keyring[id] = NULL;
- return err;
+ return ERR_PTR(err);
}
}
+ return keyring[id];
+}
+
+int integrity_digsig_verify(const unsigned int id, const char *sig, int siglen,
+ const char *digest, int digestlen)
+{
+ struct key *keyring;
+
+ if (siglen < 2)
+ return -EINVAL;
+
+ keyring = integrity_keyring_from_id(id);
+ if (IS_ERR(keyring))
+ return PTR_ERR(keyring);
+
switch (sig[1]) {
case 1:
/* v1 API expect signature without xattr type */
- return digsig_verify(keyring[id], sig + 1, siglen - 1,
- digest, digestlen);
+ return digsig_verify(keyring, sig + 1, siglen - 1, digest,
+ digestlen);
case 2:
- return asymmetric_verify(keyring[id], sig, siglen,
- digest, digestlen);
+ return asymmetric_verify(keyring, sig, siglen, digest,
+ digestlen);
}
return -EOPNOTSUPP;
}
+int integrity_modsig_verify(const unsigned int id, const struct modsig *modsig)
+{
+ struct key *keyring;
+
+ keyring = integrity_keyring_from_id(id);
+ if (IS_ERR(keyring))
+ return PTR_ERR(keyring);
+
+ return ima_modsig_verify(keyring, modsig);
+}
+
static int __integrity_init_keyring(const unsigned int id, key_perm_t perm,
struct key_restriction *restriction)
{
diff --git a/security/integrity/ima/Kconfig b/security/integrity/ima/Kconfig
index bba19f9ea184..0fb542455698 100644
--- a/security/integrity/ima/Kconfig
+++ b/security/integrity/ima/Kconfig
@@ -234,6 +234,9 @@ config IMA_APPRAISE_BOOTPARAM
config IMA_APPRAISE_MODSIG
bool "Support module-style signatures for appraisal"
depends on IMA_APPRAISE
+ depends on INTEGRITY_ASYMMETRIC_KEYS
+ select PKCS7_MESSAGE_PARSER
+ select MODULE_SIG_FORMAT
default n
help
Adds support for signatures appended to files. The format of the
diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index 9e2580164e97..ebbfae10f174 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -192,6 +192,10 @@ enum ima_hooks {
__ima_hooks(__ima_hook_enumify)
};
+extern const char *const func_tokens[];
+
+struct modsig;
+
/* LIM API function definitions */
int ima_get_action(struct inode *inode, const struct cred *cred, u32 secid,
int mask, enum ima_hooks func, int *pcr,
@@ -245,7 +249,7 @@ int ima_appraise_measurement(enum ima_hooks func,
struct integrity_iint_cache *iint,
struct file *file, const unsigned char *filename,
struct evm_ima_xattr_data *xattr_value,
- int xattr_len);
+ int xattr_len, const struct modsig *modsig);
int ima_must_appraise(struct inode *inode, int mask, enum ima_hooks func);
void ima_update_xattr(struct integrity_iint_cache *iint, struct file *file);
enum integrity_status ima_get_cache_status(struct integrity_iint_cache *iint,
@@ -261,7 +265,8 @@ static inline int ima_appraise_measurement(enum ima_hooks func,
struct file *file,
const unsigned char *filename,
struct evm_ima_xattr_data *xattr_value,
- int xattr_len)
+ int xattr_len,
+ const struct modsig *modsig)
{
return INTEGRITY_UNKNOWN;
}
@@ -300,11 +305,24 @@ static inline int ima_read_xattr(struct dentry *dentry,
#ifdef CONFIG_IMA_APPRAISE_MODSIG
bool ima_hook_supports_modsig(enum ima_hooks func);
+int ima_read_modsig(enum ima_hooks func, const void *buf, loff_t buf_len,
+ struct modsig **modsig);
+void ima_free_modsig(struct modsig *modsig);
#else
static inline bool ima_hook_supports_modsig(enum ima_hooks func)
{
return false;
}
+
+static inline int ima_read_modsig(enum ima_hooks func, const void *buf,
+ loff_t buf_len, struct modsig **modsig)
+{
+ return -EOPNOTSUPP;
+}
+
+static inline void ima_free_modsig(struct modsig *modsig)
+{
+}
#endif /* CONFIG_IMA_APPRAISE_MODSIG */
/* LSM based policy rules require audit */
diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c
index 5d4772f39757..70252ac3321d 100644
--- a/security/integrity/ima/ima_appraise.c
+++ b/security/integrity/ima/ima_appraise.c
@@ -279,6 +279,33 @@ static int xattr_verify(enum ima_hooks func, struct integrity_iint_cache *iint,
return rc;
}
+/*
+ * modsig_verify - verify modsig signature
+ *
+ * Verify whether the signature matches the file contents.
+ *
+ * Return 0 on success, error code otherwise.
+ */
+static int modsig_verify(enum ima_hooks func, const struct modsig *modsig,
+ enum integrity_status *status, const char **cause)
+{
+ int rc;
+
+ rc = integrity_modsig_verify(INTEGRITY_KEYRING_IMA, modsig);
+ if (IS_ENABLED(CONFIG_INTEGRITY_PLATFORM_KEYRING) && rc &&
+ func == KEXEC_KERNEL_CHECK)
+ rc = integrity_modsig_verify(INTEGRITY_KEYRING_PLATFORM,
+ modsig);
+ if (rc) {
+ *cause = "invalid-signature";
+ *status = INTEGRITY_FAIL;
+ } else {
+ *status = INTEGRITY_PASS;
+ }
+
+ return rc;
+}
+
/*
* ima_appraise_measurement - appraise file measurement
*
@@ -291,7 +318,7 @@ int ima_appraise_measurement(enum ima_hooks func,
struct integrity_iint_cache *iint,
struct file *file, const unsigned char *filename,
struct evm_ima_xattr_data *xattr_value,
- int xattr_len)
+ int xattr_len, const struct modsig *modsig)
{
static const char op[] = "appraise_data";
const char *cause = "unknown";
@@ -299,11 +326,14 @@ int ima_appraise_measurement(enum ima_hooks func,
struct inode *inode = d_backing_inode(dentry);
enum integrity_status status = INTEGRITY_UNKNOWN;
int rc = xattr_len;
+ bool try_modsig = iint->flags & IMA_MODSIG_ALLOWED && modsig;
- if (!(inode->i_opflags & IOP_XATTR))
+ /* If not appraising a modsig, we need an xattr. */
+ if (!(inode->i_opflags & IOP_XATTR) && !try_modsig)
return INTEGRITY_UNKNOWN;
- if (rc <= 0) {
+ /* If reading the xattr failed and there's no modsig, error out. */
+ if (rc <= 0 && !try_modsig) {
if (rc && rc != -ENODATA)
goto out;
@@ -326,6 +356,10 @@ int ima_appraise_measurement(enum ima_hooks func,
case INTEGRITY_UNKNOWN:
break;
case INTEGRITY_NOXATTRS: /* No EVM protected xattrs. */
+ /* It's fine not to have xattrs when using a modsig. */
+ if (try_modsig)
+ break;
+ /* fall through */
case INTEGRITY_NOLABEL: /* No security.evm xattr. */
cause = "missing-HMAC";
goto out;
@@ -340,6 +374,15 @@ int ima_appraise_measurement(enum ima_hooks func,
rc = xattr_verify(func, iint, xattr_value, xattr_len, &status,
&cause);
+ /*
+ * If we have a modsig and either no imasig or the imasig's key isn't
+ * known, then try verifying the modsig.
+ */
+ if (try_modsig &&
+ (!xattr_value || xattr_value->type == IMA_XATTR_DIGEST_NG ||
+ rc == -ENOKEY))
+ rc = modsig_verify(func, modsig, &status, &cause);
+
out:
/*
* File signatures on some filesystems can not be properly verified.
@@ -356,7 +399,7 @@ int ima_appraise_measurement(enum ima_hooks func,
op, cause, rc, 0);
} else if (status != INTEGRITY_PASS) {
/* Fix mode, but don't replace file signatures. */
- if ((ima_appraise & IMA_APPRAISE_FIX) &&
+ if ((ima_appraise & IMA_APPRAISE_FIX) && !try_modsig &&
(!xattr_value ||
xattr_value->type != EVM_IMA_XATTR_DIGSIG)) {
if (!ima_fix_xattr(dentry, iint))
diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
index af341a80118f..8ddf9faa8d02 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -202,6 +202,7 @@ static int process_measurement(struct file *file, const struct cred *cred,
int rc = 0, action, must_appraise = 0;
int pcr = CONFIG_IMA_MEASURE_PCR_IDX;
struct evm_ima_xattr_data *xattr_value = NULL;
+ struct modsig *modsig = NULL;
int xattr_len = 0;
bool violation_check;
enum hash_algo hash_algo;
@@ -302,10 +303,15 @@ static int process_measurement(struct file *file, const struct cred *cred,
}
if ((action & IMA_APPRAISE_SUBMASK) ||
- strcmp(template_desc->name, IMA_TEMPLATE_IMA_NAME) != 0)
+ strcmp(template_desc->name, IMA_TEMPLATE_IMA_NAME) != 0) {
/* read 'security.ima' */
xattr_len = ima_read_xattr(file_dentry(file), &xattr_value);
+ /* Read the appended modsig if allowed by the policy. */
+ if (iint->flags & IMA_MODSIG_ALLOWED)
+ ima_read_modsig(func, buf, size, &modsig);
+ }
+
hash_algo = ima_get_hash_algo(xattr_value, xattr_len);
rc = ima_collect_measurement(iint, file, buf, size, hash_algo);
@@ -322,7 +328,7 @@ static int process_measurement(struct file *file, const struct cred *cred,
if (rc == 0 && (action & IMA_APPRAISE_SUBMASK)) {
inode_lock(inode);
rc = ima_appraise_measurement(func, iint, file, pathname,
- xattr_value, xattr_len);
+ xattr_value, xattr_len, modsig);
inode_unlock(inode);
if (!rc)
rc = mmap_violation_check(func, file, &pathbuf,
@@ -339,6 +345,7 @@ static int process_measurement(struct file *file, const struct cred *cred,
rc = -EACCES;
mutex_unlock(&iint->mutex);
kfree(xattr_value);
+ ima_free_modsig(modsig);
out:
if (pathbuf)
__putname(pathbuf);
diff --git a/security/integrity/ima/ima_modsig.c b/security/integrity/ima/ima_modsig.c
index 87503bfe8c8b..f41ebe370fa0 100644
--- a/security/integrity/ima/ima_modsig.c
+++ b/security/integrity/ima/ima_modsig.c
@@ -8,8 +8,17 @@
* Thiago Jung Bauermann <bauerman@linux.ibm.com>
*/
+#include <linux/types.h>
+#include <linux/module_signature.h>
+#include <keys/asymmetric-type.h>
+#include <crypto/pkcs7.h>
+
#include "ima.h"
+struct modsig {
+ struct pkcs7_message *pkcs7_msg;
+};
+
/**
* ima_hook_supports_modsig - can the policy allow modsig for this hook?
*
@@ -29,3 +38,65 @@ bool ima_hook_supports_modsig(enum ima_hooks func)
return false;
}
}
+
+/*
+ * ima_read_modsig - Read modsig from buf.
+ *
+ * Return: 0 on success, error code otherwise.
+ */
+int ima_read_modsig(enum ima_hooks func, const void *buf, loff_t buf_len,
+ struct modsig **modsig)
+{
+ const size_t marker_len = strlen(MODULE_SIG_STRING);
+ const struct module_signature *sig;
+ struct modsig *hdr;
+ size_t sig_len;
+ const void *p;
+ int rc;
+
+ if (buf_len <= marker_len + sizeof(*sig))
+ return -ENOENT;
+
+ p = buf + buf_len - marker_len;
+ if (memcmp(p, MODULE_SIG_STRING, marker_len))
+ return -ENOENT;
+
+ buf_len -= marker_len;
+ sig = (const struct module_signature *)(p - sizeof(*sig));
+
+ rc = mod_check_sig(sig, buf_len, func_tokens[func]);
+ if (rc)
+ return rc;
+
+ sig_len = be32_to_cpu(sig->sig_len);
+ buf_len -= sig_len + sizeof(*sig);
+
+ hdr = kmalloc(sizeof(*hdr), GFP_KERNEL);
+ if (!hdr)
+ return -ENOMEM;
+
+ hdr->pkcs7_msg = pkcs7_parse_message(buf + buf_len, sig_len);
+ if (IS_ERR(hdr->pkcs7_msg)) {
+ kfree(hdr);
+ return PTR_ERR(hdr->pkcs7_msg);
+ }
+
+ *modsig = hdr;
+
+ return 0;
+}
+
+int ima_modsig_verify(struct key *keyring, const struct modsig *modsig)
+{
+ return verify_pkcs7_message_sig(NULL, 0, modsig->pkcs7_msg, keyring,
+ VERIFYING_MODULE_SIGNATURE, NULL, NULL);
+}
+
+void ima_free_modsig(struct modsig *modsig)
+{
+ if (!modsig)
+ return;
+
+ pkcs7_free_message(modsig->pkcs7_msg);
+ kfree(modsig);
+}
diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
index 06ae4b7b3676..f64ef84516db 100644
--- a/security/integrity/ima/ima_policy.c
+++ b/security/integrity/ima/ima_policy.c
@@ -1167,6 +1167,12 @@ void ima_delete_rules(void)
}
}
+#define __ima_hook_stringify(str) (#str),
+
+const char *const func_tokens[] = {
+ __ima_hooks(__ima_hook_stringify)
+};
+
#ifdef CONFIG_IMA_READ_POLICY
enum {
mask_exec = 0, mask_write, mask_read, mask_append
@@ -1179,12 +1185,6 @@ static const char *const mask_tokens[] = {
"^MAY_APPEND"
};
-#define __ima_hook_stringify(str) (#str),
-
-static const char *const func_tokens[] = {
- __ima_hooks(__ima_hook_stringify)
-};
-
void *ima_policy_start(struct seq_file *m, loff_t *pos)
{
loff_t l = *pos;
diff --git a/security/integrity/integrity.h b/security/integrity/integrity.h
index 0e7330a36a9d..c6e7f41db470 100644
--- a/security/integrity/integrity.h
+++ b/security/integrity/integrity.h
@@ -153,10 +153,13 @@ int integrity_kernel_read(struct file *file, loff_t offset,
extern struct dentry *integrity_dir;
+struct modsig;
+
#ifdef CONFIG_INTEGRITY_SIGNATURE
int integrity_digsig_verify(const unsigned int id, const char *sig, int siglen,
const char *digest, int digestlen);
+int integrity_modsig_verify(unsigned int id, const struct modsig *modsig);
int __init integrity_init_keyring(const unsigned int id);
int __init integrity_load_x509(const unsigned int id, const char *path);
@@ -171,6 +174,12 @@ static inline int integrity_digsig_verify(const unsigned int id,
return -EOPNOTSUPP;
}
+static inline int integrity_modsig_verify(unsigned int id,
+ const struct modsig *modsig)
+{
+ return -EOPNOTSUPP;
+}
+
static inline int integrity_init_keyring(const unsigned int id)
{
return 0;
@@ -196,6 +205,16 @@ static inline int asymmetric_verify(struct key *keyring, const char *sig,
}
#endif
+#ifdef CONFIG_IMA_APPRAISE_MODSIG
+int ima_modsig_verify(struct key *keyring, const struct modsig *modsig);
+#else
+static inline int ima_modsig_verify(struct key *keyring,
+ const struct modsig *modsig)
+{
+ return -EOPNOTSUPP;
+}
+#endif
+
#ifdef CONFIG_IMA_LOAD_X509
void __init ima_load_x509(void);
#else
^ permalink raw reply related
* [PATCH v11 07/13] ima: Add modsig appraise_type option for module-style appended signatures
From: Thiago Jung Bauermann @ 2019-06-11 6:28 UTC (permalink / raw)
To: linux-integrity
Cc: linux-security-module, keyrings, linux-crypto, linuxppc-dev,
linux-doc, linux-kernel, Mimi Zohar, Dmitry Kasatkin,
James Morris, Serge E. Hallyn, David Howells, David Woodhouse,
Jessica Yu, Herbert Xu, David S. Miller, Jonathan Corbet,
AKASHI, Takahiro, Thiago Jung Bauermann
In-Reply-To: <20190611062817.18412-1-bauerman@linux.ibm.com>
Introduce the modsig keyword to the IMA policy syntax to specify that
a given hook should expect the file to have the IMA signature appended
to it. Here is how it can be used in a rule:
appraise func=KEXEC_KERNEL_CHECK appraise_type=imasig|modsig
With this rule, IMA will accept either a signature stored in the extended
attribute or an appended signature.
For now, the rule above will behave exactly the same as if
appraise_type=imasig was specified. The actual modsig implementation
will be introduced separately.
Suggested-by: Mimi Zohar <zohar@linux.ibm.com>
Signed-off-by: Thiago Jung Bauermann <bauerman@linux.ibm.com>
---
Documentation/ABI/testing/ima_policy | 6 +++++-
security/integrity/ima/Kconfig | 10 +++++++++
security/integrity/ima/Makefile | 1 +
security/integrity/ima/ima.h | 9 ++++++++
security/integrity/ima/ima_modsig.c | 31 ++++++++++++++++++++++++++++
security/integrity/ima/ima_policy.c | 12 +++++++++--
security/integrity/integrity.h | 1 +
7 files changed, 67 insertions(+), 3 deletions(-)
diff --git a/Documentation/ABI/testing/ima_policy b/Documentation/ABI/testing/ima_policy
index b383c1763610..e622cdafe0af 100644
--- a/Documentation/ABI/testing/ima_policy
+++ b/Documentation/ABI/testing/ima_policy
@@ -36,7 +36,7 @@ Description:
euid:= decimal value
fowner:= decimal value
lsm: are LSM specific
- option: appraise_type:= [imasig]
+ option: appraise_type:= [imasig] [imasig|modsig]
template:= name of a defined IMA template type
(eg, ima-ng). Only valid when action is "measure".
pcr:= decimal value
@@ -104,3 +104,7 @@ Description:
measure func=KEXEC_KERNEL_CHECK pcr=4
measure func=KEXEC_INITRAMFS_CHECK pcr=5
+
+ Example of appraise rule allowing modsig appended signatures:
+
+ appraise func=KEXEC_KERNEL_CHECK appraise_type=imasig|modsig
diff --git a/security/integrity/ima/Kconfig b/security/integrity/ima/Kconfig
index a18f8c6d13b5..bba19f9ea184 100644
--- a/security/integrity/ima/Kconfig
+++ b/security/integrity/ima/Kconfig
@@ -231,6 +231,16 @@ config IMA_APPRAISE_BOOTPARAM
This option enables the different "ima_appraise=" modes
(eg. fix, log) from the boot command line.
+config IMA_APPRAISE_MODSIG
+ bool "Support module-style signatures for appraisal"
+ depends on IMA_APPRAISE
+ default n
+ help
+ Adds support for signatures appended to files. The format of the
+ appended signature is the same used for signed kernel modules.
+ The modsig keyword can be used in the IMA policy to allow a hook
+ to accept such signatures.
+
config IMA_TRUSTED_KEYRING
bool "Require all keys on the .ima keyring be signed (deprecated)"
depends on IMA_APPRAISE && SYSTEM_TRUSTED_KEYRING
diff --git a/security/integrity/ima/Makefile b/security/integrity/ima/Makefile
index d921dc4f9eb0..31d57cdf2421 100644
--- a/security/integrity/ima/Makefile
+++ b/security/integrity/ima/Makefile
@@ -9,5 +9,6 @@ obj-$(CONFIG_IMA) += ima.o
ima-y := ima_fs.o ima_queue.o ima_init.o ima_main.o ima_crypto.o ima_api.o \
ima_policy.o ima_template.o ima_template_lib.o
ima-$(CONFIG_IMA_APPRAISE) += ima_appraise.o
+ima-$(CONFIG_IMA_APPRAISE_MODSIG) += ima_modsig.o
ima-$(CONFIG_HAVE_IMA_KEXEC) += ima_kexec.o
obj-$(CONFIG_IMA_BLACKLIST_KEYRING) += ima_mok.o
diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index 18b48a6d0b80..9e2580164e97 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -298,6 +298,15 @@ static inline int ima_read_xattr(struct dentry *dentry,
#endif /* CONFIG_IMA_APPRAISE */
+#ifdef CONFIG_IMA_APPRAISE_MODSIG
+bool ima_hook_supports_modsig(enum ima_hooks func);
+#else
+static inline bool ima_hook_supports_modsig(enum ima_hooks func)
+{
+ return false;
+}
+#endif /* CONFIG_IMA_APPRAISE_MODSIG */
+
/* LSM based policy rules require audit */
#ifdef CONFIG_IMA_LSM_RULES
diff --git a/security/integrity/ima/ima_modsig.c b/security/integrity/ima/ima_modsig.c
new file mode 100644
index 000000000000..87503bfe8c8b
--- /dev/null
+++ b/security/integrity/ima/ima_modsig.c
@@ -0,0 +1,31 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * IMA support for appraising module-style appended signatures.
+ *
+ * Copyright (C) 2019 IBM Corporation
+ *
+ * Author:
+ * Thiago Jung Bauermann <bauerman@linux.ibm.com>
+ */
+
+#include "ima.h"
+
+/**
+ * ima_hook_supports_modsig - can the policy allow modsig for this hook?
+ *
+ * modsig is only supported by hooks using ima_post_read_file(), because only
+ * they preload the contents of the file in a buffer. FILE_CHECK does that in
+ * some cases, but not when reached from vfs_open(). POLICY_CHECK can support
+ * it, but it's not useful in practice because it's a text file so deny.
+ */
+bool ima_hook_supports_modsig(enum ima_hooks func)
+{
+ switch (func) {
+ case KEXEC_KERNEL_CHECK:
+ case KEXEC_INITRAMFS_CHECK:
+ case MODULE_CHECK:
+ return true;
+ default:
+ return false;
+ }
+}
diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
index fd9b01881d17..06ae4b7b3676 100644
--- a/security/integrity/ima/ima_policy.c
+++ b/security/integrity/ima/ima_policy.c
@@ -1049,6 +1049,10 @@ static int ima_parse_rule(char *rule, struct ima_rule_entry *entry)
ima_log_string(ab, "appraise_type", args[0].from);
if ((strcmp(args[0].from, "imasig")) == 0)
entry->flags |= IMA_DIGSIG_REQUIRED;
+ else if (ima_hook_supports_modsig(entry->func) &&
+ strcmp(args[0].from, "imasig|modsig") == 0)
+ entry->flags |= IMA_DIGSIG_REQUIRED |
+ IMA_MODSIG_ALLOWED;
else
result = -EINVAL;
break;
@@ -1358,8 +1362,12 @@ int ima_policy_show(struct seq_file *m, void *v)
}
if (entry->template)
seq_printf(m, "template=%s ", entry->template->name);
- if (entry->flags & IMA_DIGSIG_REQUIRED)
- seq_puts(m, "appraise_type=imasig ");
+ if (entry->flags & IMA_DIGSIG_REQUIRED) {
+ if (entry->flags & IMA_MODSIG_ALLOWED)
+ seq_puts(m, "appraise_type=imasig|modsig ");
+ else
+ seq_puts(m, "appraise_type=imasig ");
+ }
if (entry->flags & IMA_PERMIT_DIRECTIO)
seq_puts(m, "permit_directio ");
rcu_read_unlock();
diff --git a/security/integrity/integrity.h b/security/integrity/integrity.h
index 88a29f72a74f..0e7330a36a9d 100644
--- a/security/integrity/integrity.h
+++ b/security/integrity/integrity.h
@@ -36,6 +36,7 @@
#define IMA_NEW_FILE 0x04000000
#define EVM_IMMUTABLE_DIGSIG 0x08000000
#define IMA_FAIL_UNVERIFIABLE_SIGS 0x10000000
+#define IMA_MODSIG_ALLOWED 0x20000000
#define IMA_DO_MASK (IMA_MEASURE | IMA_APPRAISE | IMA_AUDIT | \
IMA_HASH | IMA_APPRAISE_SUBMASK)
^ permalink raw reply related
* [PATCH v11 06/13] ima: Use designated initializers for struct ima_event_data
From: Thiago Jung Bauermann @ 2019-06-11 6:28 UTC (permalink / raw)
To: linux-integrity
Cc: linux-security-module, keyrings, linux-crypto, linuxppc-dev,
linux-doc, linux-kernel, Mimi Zohar, Dmitry Kasatkin,
James Morris, Serge E. Hallyn, David Howells, David Woodhouse,
Jessica Yu, Herbert Xu, David S. Miller, Jonathan Corbet,
AKASHI, Takahiro, Thiago Jung Bauermann
In-Reply-To: <20190611062817.18412-1-bauerman@linux.ibm.com>
Designated initializers allow specifying only the members of the struct
that need initialization. Non-mentioned members are initialized to zero.
This makes the code a bit clearer (particularly in ima_add_boot_aggregate)
and also allows adding a new member to the struct without having to update
all struct initializations.
Signed-off-by: Thiago Jung Bauermann <bauerman@linux.ibm.com>
Reviewed-by: Mimi Zohar <zohar@linux.ibm.com>
---
security/integrity/ima/ima_api.c | 13 +++++++++----
security/integrity/ima/ima_init.c | 4 ++--
2 files changed, 11 insertions(+), 6 deletions(-)
diff --git a/security/integrity/ima/ima_api.c b/security/integrity/ima/ima_api.c
index 78eb11c7ac07..c0cf4bcfc82f 100644
--- a/security/integrity/ima/ima_api.c
+++ b/security/integrity/ima/ima_api.c
@@ -139,8 +139,10 @@ void ima_add_violation(struct file *file, const unsigned char *filename,
{
struct ima_template_entry *entry;
struct inode *inode = file_inode(file);
- struct ima_event_data event_data = {iint, file, filename, NULL, 0,
- cause};
+ struct ima_event_data event_data = { .iint = iint,
+ .file = file,
+ .filename = filename,
+ .violation = cause };
int violation = 1;
int result;
@@ -294,8 +296,11 @@ void ima_store_measurement(struct integrity_iint_cache *iint,
int result = -ENOMEM;
struct inode *inode = file_inode(file);
struct ima_template_entry *entry;
- struct ima_event_data event_data = {iint, file, filename, xattr_value,
- xattr_len, NULL};
+ struct ima_event_data event_data = { .iint = iint,
+ .file = file,
+ .filename = filename,
+ .xattr_value = xattr_value,
+ .xattr_len = xattr_len };
int violation = 0;
if (iint->measured_pcrs & (0x1 << pcr))
diff --git a/security/integrity/ima/ima_init.c b/security/integrity/ima/ima_init.c
index 993d0f1915ff..368ef658a1cd 100644
--- a/security/integrity/ima/ima_init.c
+++ b/security/integrity/ima/ima_init.c
@@ -49,8 +49,8 @@ static int __init ima_add_boot_aggregate(void)
const char *audit_cause = "ENOMEM";
struct ima_template_entry *entry;
struct integrity_iint_cache tmp_iint, *iint = &tmp_iint;
- struct ima_event_data event_data = {iint, NULL, boot_aggregate_name,
- NULL, 0, NULL};
+ struct ima_event_data event_data = { .iint = iint,
+ .filename = boot_aggregate_name };
int result = -ENOMEM;
int violation = 0;
struct {
^ permalink raw reply related
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