From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <linux-kernel-owner@vger.kernel.org>
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
        id S1752347AbdKFXRB (ORCPT <rfc822;w@1wt.eu>);
        Mon, 6 Nov 2017 18:17:01 -0500
Received: from mga03.intel.com ([134.134.136.65]:19987 "EHLO mga03.intel.com"
        rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP
        id S1751362AbdKFXRA (ORCPT <rfc822;linux-kernel@vger.kernel.org>);
        Mon, 6 Nov 2017 18:17:00 -0500
X-ExtLoop1: 1
X-IronPort-AV: E=Sophos;i="5.44,354,1505804400"; 
   d="scan'208";a="1240541831"
Date: Mon, 6 Nov 2017 15:16:58 -0800
From: Andi Kleen <ak@linux.intel.com>
To: Milind Chabbi <chabbi.milind@gmail.com>
Cc: jolsa@redhat.com, Peter Zijlstra <peterz@infradead.org>,
        Ingo Molnar <mingo@redhat.com>,
        Arnaldo Carvalho de Melo <acme@kernel.org>,
        Alexander Shishkin <alexander.shishkin@linux.intel.com>,
        Namhyung Kim <namhyung@kernel.org>,
        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>,
        linux-kernel@vger.kernel.org
Subject: Re: [PATCH] perf/core: fast breakpoint modification via
 _IOC_MODIFY_BREAKPOINT.
Message-ID: <20171106231658.GE5320@tassilo.jf.intel.com>
References: <1510006156-18988-1-git-send-email-chabbi.milind@gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <1510006156-18988-1-git-send-email-chabbi.milind@gmail.com>
User-Agent: Mutt/1.9.1 (2017-09-22)
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

> +static int _perf_event_modify_breakpoint(struct perf_event *bp,
> +					 struct perf_event_attr *attr)
> +{
> +	u64 old_addr = bp->attr.bp_addr;
> +	u64 old_len = bp->attr.bp_len;
> +	int old_type = bp->attr.bp_type;
> +	int err = 0;
> +
> +	_perf_event_disable(bp);
> +
> +	bp->attr.bp_addr = attr->bp_addr;
> +	bp->attr.bp_type = attr->bp_type;
> +	bp->attr.bp_len = attr->bp_len;

You don't check any of the other fields, so user space is free
to fill in junk. That means they can never be used for anything.
It would be better to check at least some of them for being
zero, and also that the type matches the break point.

-Andi