Messages in this thread | | | From | Oded Gabbay <> | Date | Tue, 6 Aug 2019 15:22:35 +0300 | Subject | Re: [PATCH 2/2] habanalabs: improve security in Debug IOCTL |
| |
On Tue, Aug 6, 2019 at 10:38 AM Omer Shpigelman <oshpigelman@habana.ai> 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 <oshpigelman@habana.ai> > --- > 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_OFFSET 0x400 > +#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, > + "too many event types values for SPMU enable\n"); > + return -EINVAL; > + } > + > WREG32(base_reg + 0xE04, 0x41013046); > WREG32(base_reg + 0xE04, 0x41013040); > > for (i = 0 ; i < input->event_types_num ; i++) > - WREG32(base_reg + 0x400 + i * 4, input->event_types[i]); > + WREG32(base_reg + SPMU_EVENT_TYPES_OFFSET + i * 4, > + input->event_types[i]); > > WREG32(base_reg + 0xE04, 0x41013041); > WREG32(base_reg + 0xC00, 0x8000003F); > @@ -567,6 +615,12 @@ static int goya_config_spmu(struct hl_device *hdev, > return -EINVAL; > } > > + if (events_num > SPMU_MAX_EVENTS) { > + dev_err(hdev->dev, > + "too many events values for SPMU disable\n"); > + return -EINVAL; > + } > + > WREG32(base_reg + 0xE04, 0x41013040); > > for (i = 0 ; i < events_num ; i++) > -- > 2.17.1 >
Both patches are: Reviewed-by: Oded Gabbay <oded.gabbay@gmail.com>
| |