lkml.org 
[lkml]   [2021]   [Nov]   [17]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    From
    SubjectRe: [PATCH v1 3/4] coresight: etm4x: Don't trace contextID for non-root namespace in perf mode
    Hi Suzuki,

    On Tue, Nov 16, 2021 at 09:46:20AM +0000, Suzuki Kuruppassery Poulose wrote:
    > Hi Leo,
    >
    > On 31/10/2021 14:42, Leo Yan wrote:
    > > When runs in perf mode, the driver always enables the contextID tracing.
    > > This can lead to confusion if the program runs in non-root PID namespace
    > > and potentially leak kernel information.
    > >
    > > When programs running in perf mode, this patch changes to only enable
    > > contextID tracing for root PID namespace.
    > >
    >
    > The only concern with the patch here is we silently ignore the CTXTID
    > flag and the perf assumes the CTXTID is traced, when traced from a non-root
    > namespace. Does the decoder handle this case gracefully ? We are
    > fine if that is the case.

    Good point. As far as I know, if CoreSight trace data doesn't contain
    context packets, tidq->tid is initialized as '0' and tidq->pid is
    '-1'. In this case, the decoder will fail to find thread context and
    the user space samples will not output anymore, see [1],
    cs_etm__mem_access() returns 0 when the thread pointer is NULL and
    the user space samples will be skipped.

    On the other hand, I observed an unexpected behaviour is the decoder
    also fails to output any kernel samples. From my understanding, the
    kernel samples should always be output, I will check furthermore for
    this (I can think one possibility is perf tool fails to find a
    'correct' vmlinux when parsing symbols).

    [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/tools/perf/util/cs-etm.c#n720

    > Either way, we don't want to enforce the policy in the perf tool, if we
    > can transparently handle the missing CTXTID and allow the trace session
    > and decode complete. That said, your approach is the safe bet here.

    Do you agree below assumption for tracing in non-root PID namespace?

    For non-root namespace we doesn't tracing PID, CoreSight trace data
    doesn't contain context packet, so the perf decoder cannot find the
    corresponding thread context and perf tool will not generate any
    samples for user mode. But the decoder should generate kernel
    samples.

    If you agree with this, in theory I think we should not change anything
    in perf tool (but let me confirm the decoder kernel samples can output
    properly).

    > > Signed-off-by: Leo Yan <leo.yan@linaro.org>
    > > ---
    > > drivers/hwtracing/coresight/coresight-etm4x-core.c | 10 ++++++++--
    > > 1 file changed, 8 insertions(+), 2 deletions(-)
    > >
    > > diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c
    > > index e24252eaf8e4..6e614bfb38c6 100644
    > > --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c
    > > +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c
    > > @@ -615,7 +615,9 @@ static int etm4_parse_event_config(struct coresight_device *csdev,
    > > config->cfg |= BIT(11);
    > > }
    > > - if (attr->config & BIT(ETM_OPT_CTXTID))
    > > + /* Only trace contextID when runs in root PID namespace */
    > > + if ((attr->config & BIT(ETM_OPT_CTXTID)) &&
    > > + (task_active_pid_ns(current) == &init_pid_ns))
    > > /* bit[6], Context ID tracing bit */
    > > config->cfg |= BIT(ETM4_CFG_BIT_CTXTID);
    >
    > As mentioned in the previous comment, please add a helper here, than open
    > coding the check.

    Agreed, will add new helper for checking root namespace.

    Thanks for reviewing.

    Leo

    \
     
     \ /
      Last update: 2021-11-17 14:54    [W:3.308 / U:0.008 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site