lkml.org 
[lkml]   [2021]   [Nov]   [4]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRe: [PATCH v1 2/4] coresight: etm4x: Don't use virtual contextID for non-root PID namespace
    From
    Date
    Hi Leo

    On 31/10/2021 14:42, Leo Yan wrote:
    > As commented in the function ctxid_pid_store(), it can cause the PID
    > values mismatching between context ID tracing and PID allocated in a
    > non-root namespace, and it can leak kernel information.
    >
    > For this reason, when a process runs in non-root PID namespace, the
    > driver doesn't allow contextID tracing and returns failure when access
    > contextID related sysfs nodes.
    >
    > VMID works for virtual contextID when the kernel runs in EL2 mode with
    > VHE; on the other hand, the driver doesn't prevent users from accessing
    > it when programs run in the non-root namespace. Thus this can lead
    > to same issues with contextID described above.
    >
    > This patch imposes the checking on VMID related sysfs knobs, it returns
    > failure if current process runs in non-root PID namespace.
    >
    > Signed-off-by: Leo Yan <leo.yan@linaro.org>

    Patch looks good to me. Please see minor comment below.


    > ---
    > .../coresight/coresight-etm4x-sysfs.c | 28 +++++++++++++++++++
    > 1 file changed, 28 insertions(+)
    >
    > diff --git a/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c b/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c
    > index e4c8c44d04ef..e218281703b0 100644
    > --- a/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c
    > +++ b/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c
    > @@ -2111,6 +2111,13 @@ static ssize_t vmid_val_show(struct device *dev,
    > struct etmv4_drvdata *drvdata = dev_get_drvdata(dev->parent);
    > struct etmv4_config *config = &drvdata->config;
    >
    > + /*
    > + * Don't use virtual contextID tracing if coming from a PID namespace.
    > + * See comment in ctxid_pid_store().
    > + */
    > + if (task_active_pid_ns(current) != &init_pid_ns)
    > + return -EINVAL;
    > +
    > spin_lock(&drvdata->spinlock);
    > val = (unsigned long)config->vmid_val[config->vmid_idx];
    > spin_unlock(&drvdata->spinlock);
    > @@ -2125,6 +2132,13 @@ static ssize_t vmid_val_store(struct device *dev,
    > struct etmv4_drvdata *drvdata = dev_get_drvdata(dev->parent);
    > struct etmv4_config *config = &drvdata->config;
    >
    > + /*
    > + * Don't use virtual contextID tracing if coming from a PID namespace.
    > + * See comment in ctxid_pid_store().
    > + */
    > + if (task_active_pid_ns(current) != &init_pid_ns)

    Please could we add a helper function to make this obvious ?

    e.g: task_is_in_root_ns(task) ?

    Suzuki

    \
     
     \ /
      Last update: 2021-11-04 16:08    [W:7.500 / U:0.028 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site