Re: [PATCH 2/2] habanalabs: improve security in Debug IOCTL
On Tue, Aug 6, 2019 at 10:38 AM Omer Shpigelman wrote:
>
> This patch improves the security in the Debug IOCTL.
> It adds checks that:
> - The register index value is in the allowed range for all opcodes.
> - The event types number is in the allowed range in SPMU enable.
> - The events number is in the allowed range in SPMU disable.
>
> Signed-off-by: Omer Shpigelman
> ---
> drivers/misc/habanalabs/goya/goya_coresight.c | 72 ---
> 1 file changed, 63 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/misc/habanalabs/goya/goya_coresight.c
> b/drivers/misc/habanalabs/goya/goya_coresight.c
> index d7ec7ad84cc6..4f7ffc137ab7 100644
> --- a/drivers/misc/habanalabs/goya/goya_coresight.c
> +++ b/drivers/misc/habanalabs/goya/goya_coresight.c
> @@ -15,6 +15,12 @@
>
> #define GOYA_PLDM_CORESIGHT_TIMEOUT_USEC (CORESIGHT_TIMEOUT_USEC * 100)
>
> +#define SPMU_SECTION_SIZE DMA_CH_0_CS_SPMU_MAX_OFFSET
> +#define SPMU_EVENT_TYPES_OFFSET0x400
> +#define SPMU_MAX_EVENT_TYPES ((SPMU_SECTION_SIZE - \
> + SPMU_EVENT_TYPES_OFFSET) / 4)
> +#define SPMU_MAX_EVENTS(SPMU_SECTION_SIZE / 4)
> +
> static u64 debug_stm_regs[GOYA_STM_LAST + 1] = {
> [GOYA_STM_CPU] = mmCPU_STM_BASE,
> [GOYA_STM_DMA_CH_0_CS] = mmDMA_CH_0_CS_STM_BASE,
> @@ -226,9 +232,16 @@ static int goya_config_stm(struct hl_device *hdev,
> struct hl_debug_params *params)
> {
> struct hl_debug_params_stm *input;
> - u64 base_reg = debug_stm_regs[params->reg_idx] - CFG_BASE;
> + u64 base_reg;
> int rc;
>
> + if (params->reg_idx >= ARRAY_SIZE(debug_stm_regs)) {
> + dev_err(hdev->dev, "Invalid register index in STM\n");
> + return -EINVAL;
> + }
> +
> + base_reg = debug_stm_regs[params->reg_idx] - CFG_BASE;
> +
> WREG32(base_reg + 0xFB0, CORESIGHT_UNLOCK);
>
> if (params->enable) {
> @@ -288,10 +301,17 @@ static int goya_config_etf(struct hl_device *hdev,
> struct hl_debug_params *params)
> {
> struct hl_debug_params_etf *input;
> - u64 base_reg = debug_etf_regs[params->reg_idx] - CFG_BASE;
> + u64 base_reg;
> u32 val;
> int rc;
>
> + if (params->reg_idx >= ARRAY_SIZE(debug_etf_regs)) {
> + dev_err(hdev->dev, "Invalid register index in ETF\n");
> + return -EINVAL;
> + }
> +
> + base_reg = debug_etf_regs[params->reg_idx] - CFG_BASE;
> +
> WREG32(base_reg + 0xFB0, CORESIGHT_UNLOCK);
>
> val = RREG32(base_reg + 0x304);
> @@ -445,11 +465,18 @@ static int goya_config_etr(struct hl_device *hdev,
> static int goya_config_funnel(struct hl_device *hdev,
> struct hl_debug_params *params)
> {
> - WREG32(debug_funnel_regs[params->reg_idx] - CFG_BASE + 0xFB0,
> - CORESIGHT_UNLOCK);
> + u64 base_reg;
> +
> + if (params->reg_idx >= ARRAY_SIZE(debug_funnel_regs)) {
> + dev_err(hdev->dev, "Invalid register index in FUNNEL\n");
> + return -EINVAL;
> + }
>
> - WREG32(debug_funnel_regs[params->reg_idx] - CFG_BASE,
> - params->enable ? 0x33F : 0);
> + base_reg = debug_funnel_regs[params->reg_idx] - CFG_BASE;
> +
> + WREG32(base_reg + 0xFB0, CORESIGHT_UNLOCK);
> +
> + WREG32(base_reg, params->enable ? 0x33F : 0);
>
> return 0;
> }
> @@ -458,9 +485,16 @@ static int goya_config_bmon(struct hl_device *hdev,
> struct hl_debug_params *params)
> {
> struct hl_debug_params_bmon *input;
> - u64 base_reg = debug_bmon_regs[params->reg_idx] - CFG_BASE;
> + u64 base_reg;
> u32 pcie_base = 0;
>
> + if (params->reg_idx >= ARRAY_SIZE(debug_bmon_regs)) {
> + dev_err(hdev->dev, "Invalid register index in BMON\n");
> + return -EINVAL;
> + }
> +
> + base_reg = debug_bmon_regs[params->reg_idx] - CFG_BASE;
> +
> WREG32(base_reg + 0x104, 1);
>
> if (params->enable) {
> @@ -522,7 +556,7 @@ static int goya_config_bmon(struct hl_device *hdev,
> static int goya_config_spmu(struct hl_device *hdev,
> struct hl_debug_params *params)
> {
> - u64 base_reg = debug_spmu_regs[params->reg_idx] - CFG_BASE;
> + u64 base_reg;
> struct hl_debug_params_spmu *input = params->input;
> u64 *output;
> u32 output_arr_len;
> @@ -531,6 +565,13 @@ static int goya_config_spmu(struct hl_device *hdev,
> u32 cycle_cnt_idx;
> int i;
>
> + if (params->reg_idx >= ARRAY_SIZE(debug_spmu_regs)) {
> + dev_err(hdev->dev, "Invalid register index in SPMU\n");
> + return -EINVAL;
> + }
> +
> + base_reg = debug_spmu_regs[params->reg_idx] - CFG_BASE;
> +
> if (params->enable) {
> input =
[PATCH 2/2] habanalabs: improve security in Debug IOCTL
This patch improves the security in the Debug IOCTL.
It adds checks that:
- The register index value is in the allowed range for all opcodes.
- The event types number is in the allowed range in SPMU enable.
- The events number is in the allowed range in SPMU disable.
Signed-off-by: Omer Shpigelman
---
drivers/misc/habanalabs/goya/goya_coresight.c | 72 ---
1 file changed, 63 insertions(+), 9 deletions(-)
diff --git a/drivers/misc/habanalabs/goya/goya_coresight.c
b/drivers/misc/habanalabs/goya/goya_coresight.c
index d7ec7ad84cc6..4f7ffc137ab7 100644
--- a/drivers/misc/habanalabs/goya/goya_coresight.c
+++ b/drivers/misc/habanalabs/goya/goya_coresight.c
@@ -15,6 +15,12 @@
#define GOYA_PLDM_CORESIGHT_TIMEOUT_USEC (CORESIGHT_TIMEOUT_USEC * 100)
+#define SPMU_SECTION_SIZE DMA_CH_0_CS_SPMU_MAX_OFFSET
+#define SPMU_EVENT_TYPES_OFFSET0x400
+#define SPMU_MAX_EVENT_TYPES ((SPMU_SECTION_SIZE - \
+ SPMU_EVENT_TYPES_OFFSET) / 4)
+#define SPMU_MAX_EVENTS(SPMU_SECTION_SIZE / 4)
+
static u64 debug_stm_regs[GOYA_STM_LAST + 1] = {
[GOYA_STM_CPU] = mmCPU_STM_BASE,
[GOYA_STM_DMA_CH_0_CS] = mmDMA_CH_0_CS_STM_BASE,
@@ -226,9 +232,16 @@ static int goya_config_stm(struct hl_device *hdev,
struct hl_debug_params *params)
{
struct hl_debug_params_stm *input;
- u64 base_reg = debug_stm_regs[params->reg_idx] - CFG_BASE;
+ u64 base_reg;
int rc;
+ if (params->reg_idx >= ARRAY_SIZE(debug_stm_regs)) {
+ dev_err(hdev->dev, "Invalid register index in STM\n");
+ return -EINVAL;
+ }
+
+ base_reg = debug_stm_regs[params->reg_idx] - CFG_BASE;
+
WREG32(base_reg + 0xFB0, CORESIGHT_UNLOCK);
if (params->enable) {
@@ -288,10 +301,17 @@ static int goya_config_etf(struct hl_device *hdev,
struct hl_debug_params *params)
{
struct hl_debug_params_etf *input;
- u64 base_reg = debug_etf_regs[params->reg_idx] - CFG_BASE;
+ u64 base_reg;
u32 val;
int rc;
+ if (params->reg_idx >= ARRAY_SIZE(debug_etf_regs)) {
+ dev_err(hdev->dev, "Invalid register index in ETF\n");
+ return -EINVAL;
+ }
+
+ base_reg = debug_etf_regs[params->reg_idx] - CFG_BASE;
+
WREG32(base_reg + 0xFB0, CORESIGHT_UNLOCK);
val = RREG32(base_reg + 0x304);
@@ -445,11 +465,18 @@ static int goya_config_etr(struct hl_device *hdev,
static int goya_config_funnel(struct hl_device *hdev,
struct hl_debug_params *params)
{
- WREG32(debug_funnel_regs[params->reg_idx] - CFG_BASE + 0xFB0,
- CORESIGHT_UNLOCK);
+ u64 base_reg;
+
+ if (params->reg_idx >= ARRAY_SIZE(debug_funnel_regs)) {
+ dev_err(hdev->dev, "Invalid register index in FUNNEL\n");
+ return -EINVAL;
+ }
- WREG32(debug_funnel_regs[params->reg_idx] - CFG_BASE,
- params->enable ? 0x33F : 0);
+ base_reg = debug_funnel_regs[params->reg_idx] - CFG_BASE;
+
+ WREG32(base_reg + 0xFB0, CORESIGHT_UNLOCK);
+
+ WREG32(base_reg, params->enable ? 0x33F : 0);
return 0;
}
@@ -458,9 +485,16 @@ static int goya_config_bmon(struct hl_device *hdev,
struct hl_debug_params *params)
{
struct hl_debug_params_bmon *input;
- u64 base_reg = debug_bmon_regs[params->reg_idx] - CFG_BASE;
+ u64 base_reg;
u32 pcie_base = 0;
+ if (params->reg_idx >= ARRAY_SIZE(debug_bmon_regs)) {
+ dev_err(hdev->dev, "Invalid register index in BMON\n");
+ return -EINVAL;
+ }
+
+ base_reg = debug_bmon_regs[params->reg_idx] - CFG_BASE;
+
WREG32(base_reg + 0x104, 1);
if (params->enable) {
@@ -522,7 +556,7 @@ static int goya_config_bmon(struct hl_device *hdev,
static int goya_config_spmu(struct hl_device *hdev,
struct hl_debug_params *params)
{
- u64 base_reg = debug_spmu_regs[params->reg_idx] - CFG_BASE;
+ u64 base_reg;
struct hl_debug_params_spmu *input = params->input;
u64 *output;
u32 output_arr_len;
@@ -531,6 +565,13 @@ static int goya_config_spmu(struct hl_device *hdev,
u32 cycle_cnt_idx;
int i;
+ if (params->reg_idx >= ARRAY_SIZE(debug_spmu_regs)) {
+ dev_err(hdev->dev, "Invalid register index in SPMU\n");
+ return -EINVAL;
+ }
+
+ base_reg = debug_spmu_regs[params->reg_idx] - CFG_BASE;
+
if (params->enable) {
input = params->input;
@@ -543,11 +584,18 @@ static int goya_config_spmu(struct hl_device *hdev,
return -EINVAL;
}
+ if (input->event_types_num > SPMU_MAX_EVENT_TYPES) {
+ dev_err(hdev->dev,
+
