public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Jiri Olsa <jolsa@kernel.org>
To: Ingo Molnar <mingo@kernel.org>,
	Peter Zijlstra <a.p.zijlstra@chello.nl>,
	Arnaldo Carvalho de Melo <acme@kernel.org>
Cc: lkml <linux-kernel@vger.kernel.org>,
	Namhyung Kim <namhyung@kernel.org>,
	David Ahern <dsahern@gmail.com>, Andi Kleen <ak@linux.intel.com>,
	Milind Chabbi <chabbi.milind@gmail.com>,
	Alexander Shishkin <alexander.shishkin@linux.intel.com>,
	Michael Ellerman <mpe@ellerman.id.au>,
	Hari Bathini <hbathini@linux.vnet.ibm.com>,
	Jin Yao <yao.jin@linux.intel.com>,
	Kan Liang <kan.liang@intel.com>,
	Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>,
	Oleg Nesterov <onestero@redhat.com>,
	Will Deacon <will.deacon@arm.com>
Subject: [PATCH 5/8] hw_breakpoint: Add perf_event_attr fields check in __modify_user_hw_breakpoint
Date: Mon, 12 Mar 2018 14:45:45 +0100	[thread overview]
Message-ID: <20180312134548.31532-6-jolsa@kernel.org> (raw)
In-Reply-To: <20180312134548.31532-1-jolsa@kernel.org>

And rename it to modify_user_hw_breakpoint_check.

We are about to use modify_user_hw_breakpoint_check for user space
breakpoints modification, we must be very strict to check only the
fields we can change have changed. As Peter explained:

  Suppose someone does:
        attr = malloc(sizeof(*attr)); // uninitialized memory
        attr->type = BP;
        attr->bp_addr = new_addr;
        attr->bp_type = bp_type;
        attr->bp_len = bp_len;
        ioctl(fd, PERF_IOC_MOD_ATTR, &attr);

  And feeds absolute shite for the rest of the fields.
  Then we later want to extend IOC_MOD_ATTR to allow changing
  attr::sample_type but we can't, because that would break the
  above application.

I'm making this check optional because we already export
modify_user_hw_breakpoint and with this check we could
break existing users.

Suggested-by: Peter Zijlstra <peterz@infradead.org>
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 kernel/events/hw_breakpoint.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/kernel/events/hw_breakpoint.c b/kernel/events/hw_breakpoint.c
index a556aba223da..0c82663395f7 100644
--- a/kernel/events/hw_breakpoint.c
+++ b/kernel/events/hw_breakpoint.c
@@ -456,7 +456,9 @@ register_user_hw_breakpoint(struct perf_event_attr *attr,
 }
 EXPORT_SYMBOL_GPL(register_user_hw_breakpoint);
 
-static int __modify_user_hw_breakpoint(struct perf_event *bp, struct perf_event_attr *attr)
+static int
+modify_user_hw_breakpoint_check(struct perf_event *bp, struct perf_event_attr *attr,
+			        bool check)
 {
 	u64 old_addr = bp->attr.bp_addr;
 	u64 old_len  = bp->attr.bp_len;
@@ -468,6 +470,9 @@ static int __modify_user_hw_breakpoint(struct perf_event *bp, struct perf_event_
 	bp->attr.bp_type = attr->bp_type;
 	bp->attr.bp_len  = attr->bp_len;
 
+	if (check && memcmp(&bp->attr, attr, sizeof(*attr)))
+		return -EINVAL;
+
 	err = validate_hw_breakpoint(bp);
 	if (!err && modify)
 		err = modify_bp_slot(bp, old_type);
@@ -505,7 +510,7 @@ int modify_user_hw_breakpoint(struct perf_event *bp, struct perf_event_attr *att
 	else
 		perf_event_disable(bp);
 
-	err = __modify_user_hw_breakpoint(bp, attr);
+	err = modify_user_hw_breakpoint_check(bp, attr, false);
 
 	if (err) {
 		if (!bp->attr.disabled)
-- 
2.13.6

  parent reply	other threads:[~2018-03-12 13:46 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-03-12 13:45 [PATCHv3 0/8] hw_breakpoint: Breakpoint modification fixes and new modify ioctl Jiri Olsa
2018-03-12 13:45 ` [PATCH 1/8] hw_breakpoint: Pass bp_type directly as find_slot_idx argument Jiri Olsa
2018-03-13  6:18   ` [tip:perf/core] hw_breakpoint: Pass bp_type directly as find_slot_idx() argument tip-bot for Jiri Olsa
2018-03-12 13:45 ` [PATCH 2/8] hw_breakpoint: Pass bp_type argument to __reserve_bp_slot|__release_bp_slot Jiri Olsa
2018-03-13  6:19   ` [tip:perf/core] hw_breakpoint: Pass bp_type argument to __reserve_bp_slot|__release_bp_slot() tip-bot for Jiri Olsa
2018-03-12 13:45 ` [PATCH 3/8] hw_breakpoint: Add modify_bp_slot function Jiri Olsa
2018-03-13  6:19   ` [tip:perf/core] hw_breakpoint: Add modify_bp_slot() function tip-bot for Jiri Olsa
2018-03-12 13:45 ` [PATCH 4/8] hw_breakpoint: Factor out __modify_user_hw_breakpoint function Jiri Olsa
2018-03-13  6:20   ` [tip:perf/core] hw_breakpoint: Factor out __modify_user_hw_breakpoint() function tip-bot for Jiri Olsa
2018-03-12 13:45 ` Jiri Olsa [this message]
2018-03-13  6:20   ` [tip:perf/core] hw_breakpoint: Add perf_event_attr fields check in __modify_user_hw_breakpoint() tip-bot for Jiri Olsa
2018-03-12 13:45 ` [PATCH 6/8] perf/core: Move perf_event_attr::sample_max_stack into perf_copy_attr Jiri Olsa
2018-03-13  6:21   ` [tip:perf/core] perf/core: Move perf_event_attr::sample_max_stack into perf_copy_attr() tip-bot for Jiri Olsa
2018-03-12 13:45 ` [PATCH 7/8] perf/core: fast breakpoint modification via _IOC_MODIFY_ATTRIBUTES Jiri Olsa
2018-03-13  6:21   ` [tip:perf/core] perf/core: Implement " tip-bot for Milind Chabbi
2018-03-13 15:14   ` tip-bot for Milind Chabbi
2018-03-12 13:45 ` [PATCH 8/8] perf tests: Add breakpoint accounting/modify test Jiri Olsa
2018-03-13  6:22   ` [tip:perf/core] " tip-bot for Jiri Olsa
2018-03-13 15:13   ` tip-bot for Jiri Olsa
2018-03-13  6:37 ` [PATCHv3 0/8] hw_breakpoint: Breakpoint modification fixes and new modify ioctl Ingo Molnar
2018-03-13  9:28   ` Jiri Olsa
2018-03-13  9:50     ` Jiri Olsa
2018-03-14  8:45       ` [PATCH] hw_breakpoint: Fix build for disabled CONFIG_HAVE_HW_BREAKPOINT Jiri Olsa
2018-03-14 12:28         ` Ingo Molnar
  -- strict thread matches above, loose matches on Subject: below --
2017-11-29  8:38 [PATCHv2 0/8] hw_breakpoint: Breakpoint modification fixes and new modify ioctl Jiri Olsa
2017-11-29  8:38 ` [PATCH 5/8] hw_breakpoint: Add perf_event_attr fields check in __modify_user_hw_breakpoint Jiri Olsa

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180312134548.31532-6-jolsa@kernel.org \
    --to=jolsa@kernel.org \
    --cc=a.p.zijlstra@chello.nl \
    --cc=acme@kernel.org \
    --cc=ak@linux.intel.com \
    --cc=alexander.shishkin@linux.intel.com \
    --cc=chabbi.milind@gmail.com \
    --cc=dsahern@gmail.com \
    --cc=hbathini@linux.vnet.ibm.com \
    --cc=kan.liang@intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@kernel.org \
    --cc=mpe@ellerman.id.au \
    --cc=namhyung@kernel.org \
    --cc=onestero@redhat.com \
    --cc=sukadev@linux.vnet.ibm.com \
    --cc=will.deacon@arm.com \
    --cc=yao.jin@linux.intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox