Re: [PATCH 01/18] dbtr: Add consistent range checks to trigger ecalls

Himanshu Chauhan
2026-04-07T05:23:08Z

On Fri, Mar 13, 2026 at 10:50 AM Nicholas Piggin <npiggin@gmail.com> wrote:
>
> Be more consistent with the sequence of trigger range checks in dbtr
> ecall handlers. This includes adding a missing range check in install
> triggers that can result in SBI past the end of the shm region.
>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
> lib/sbi/sbi_dbtr.c | 27 ++++++++++++++++++++++++---
> 1 file changed, 24 insertions(+), 3 deletions(-)
>
> diff --git a/lib/sbi/sbi_dbtr.c b/lib/sbi/sbi_dbtr.c
> index 8bcb4312..b0160163 100644
> --- a/lib/sbi/sbi_dbtr.c
> +++ b/lib/sbi/sbi_dbtr.c
> @@ -550,6 +550,9 @@ int sbi_dbtr_read_trig(unsigned long smode,
> if (!hs)
> return SBI_ERR_FAILED;
>
> + if (trig_count >= hs->total_trigs)
> + return SBI_ERR_BAD_RANGE;
> +
> if (trig_idx_base >= hs->total_trigs ||
> trig_idx_base + trig_count >= hs->total_trigs)
> return SBI_ERR_INVALID_PARAM;
> @@ -594,6 +597,9 @@ int sbi_dbtr_install_trig(unsigned long smode,
> if (!hs)
> return SBI_ERR_FAILED;
>
> + if (trig_count >= hs->total_trigs)
> + return SBI_ERR_BAD_RANGE;
> +
> if (sbi_dbtr_shmem_disabled(hs))
> return SBI_ERR_NO_SHMEM;
>
> @@ -663,6 +669,11 @@ int sbi_dbtr_uninstall_trig(unsigned long trig_idx_base,
> if (!hs)
> return SBI_ERR_FAILED;
>
> + if (trig_idx_base >= hs->total_trigs ||
> + find_last_bit(&trig_idx_mask, hs->total_trigs) +
> + trig_idx_base >= hs->total_trigs)
> + return SBI_ERR_INVALID_PARAM;
> +
> for_each_set_bit_from(idx, &trig_mask, hs->total_trigs) {
> trig = INDEX_TO_TRIGGER(idx);
> if (!(trig->state & RV_DBTR_BIT_MASK(TS, MAPPED)))
> @@ -688,6 +699,11 @@ int sbi_dbtr_enable_trig(unsigned long trig_idx_base,
> if (!hs)
> return SBI_ERR_FAILED;
>
> + if (trig_idx_base >= hs->total_trigs ||
> + find_last_bit(&trig_idx_mask, hs->total_trigs) +
> + trig_idx_base >= hs->total_trigs)
> + return SBI_ERR_INVALID_PARAM;
> +
> for_each_set_bit_from(idx, &trig_mask, hs->total_trigs) {
> trig = INDEX_TO_TRIGGER(idx);
> sbi_dprintf("%s: enable trigger %lu\n", __func__, idx);
> @@ -710,14 +726,14 @@ int sbi_dbtr_update_trig(unsigned long smode,
> if (!hs)
> return SBI_ERR_FAILED;
>
> + if (trig_count >= hs->total_trigs)
> + return SBI_ERR_BAD_RANGE;
> +
> if (sbi_dbtr_shmem_disabled(hs))
> return SBI_ERR_NO_SHMEM;
>
> shmem_base = hart_shmem_base(hs);
>
> - if (trig_count >= hs->total_trigs)
> - return SBI_ERR_BAD_RANGE;
> -
> for_each_trig_entry(shmem_base, trig_count, typeof(*entry), entry) {
> sbi_hart_protection_map_range((unsigned long)entry, sizeof(*entry));
> trig_idx = entry->id.idx;
> @@ -754,6 +770,11 @@ int sbi_dbtr_disable_trig(unsigned long trig_idx_base,
> if (!hs)
> return SBI_ERR_FAILED;
>
> + if (trig_idx_base >= hs->total_trigs ||
> + find_last_bit(&trig_idx_mask, hs->total_trigs) +
> + trig_idx_base >= hs->total_trigs)
> + return SBI_ERR_INVALID_PARAM;
> +
> for_each_set_bit_from(idx, &trig_mask, hs->total_trigs) {
> trig = INDEX_TO_TRIGGER(idx);
> dbtr_trigger_disable(trig);

Looks good to me.

Reviewed-by: Himanshu Chauhan <himanshu.chauhan@oss.qualcomm.com>

> --
> 2.51.0
>
>
> --
> opensbi mailing list
> opensbi@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/opensbi