| Message ID | 20211031144214.237879-3-leo.yan@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | coresight: etm: Correct (virtual) contextID tracing for namespace | expand |
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
On Thu, Nov 04, 2021 at 03:07:45PM +0000, Suzuki Kuruppassery Poulose wrote: [...] > > @@ -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) ? Yeah, I will use a separate patch set to add this helper function and polish relevant codes in the kernel. Thanks for suggestion. Leo
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) + return -EINVAL; + /* * only implemented when vmid tracing is enabled, i.e. at least one * vmid comparator is implemented and at least 8 bit vmid size @@ -2148,6 +2162,13 @@ static ssize_t vmid_masks_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); val1 = config->vmid_mask0; val2 = config->vmid_mask1; @@ -2165,6 +2186,13 @@ static ssize_t vmid_masks_store(struct device *dev, struct etmv4_config *config = &drvdata->config; int nr_inputs; + /* + * 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; + /* * only implemented when vmid tracing is enabled, i.e. at least one * vmid comparator is implemented and at least 8 bit vmid size
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> --- .../coresight/coresight-etm4x-sysfs.c | 28 +++++++++++++++++++ 1 file changed, 28 insertions(+)