[PATCH v3 0/3] lib: sbi: fix shared memory double-fetch in DBTR and SSE

[Himanshu Chauhan]

Hi Liutong,

On Thu, Sep 10, 2026 at 05:29:55AM +0000, liutong wrote:
> M-mode validates a request held in S-mode shared memory and then reads
> that same memory again in order to act on it. S-mode may rewrite the
> memory in between, so the values programmed into M-mode state are not
> necessarily the values that were validated.

After a thought, I think this is an S-mode problem we are trying to solve in
M-mode without much gain. First of all, this should be fixed in S-mode.
Secondly, even if there is a CPU running rogue code, cannot bring down the
system. The triggers programmed may be bad. But again, its a S-mode bug.

As said in one of the previos patch, a new series which optimizes a scenario
defined there would make sense.

I would NAK this patch series at this time.

Regards
Himanshu

> 
>   - sbi_dbtr_install_trig() re-reads each entry in three passes
>   - sbi_dbtr_update_trig() re-reads each entry four times
>   - sse_write_attrs() re-reads the attributes in a check pass and again
>     in an apply pass
> 
> Each is fixed by copying the request once and making every later
> decision against that copy.
> 
> Himanshu pointed out on v2 that the install_trig patch was written
> against a shape of the function that no longer exists upstream: the
> per-slot capability matching and the dry-run slot allocation added
> since then re-read shared memory independently, so the race was still
> open. That patch is reworked here against current master, with the
> copy threaded through those paths.
> 
> The other half of the previous six patch series, three integer overflow
> fixes and one unmap fix, is sent separately as "[PATCH v3 0/4] lib:
> sbi: fix integer overflows in shared memory bounds checks". Both halves
> apply to master in either order.
> 
> Changes in v3:
> - Rebased onto current master
> - Split out of the previous six patch series
> - Subject prefixes changed to "lib: sbi: dbtr:" and "lib: sbi: sse:" to
>   match the rest of those files' history
> - 1/3: reworked, see the patch for details
> - 2/3: new in v3
> - 3/3: added a bound check inside sse_write_attrs()
> - Dropped the DBTR shared memory range check that was v2 3/6. Pengpeng
>   Hou sent the same fix in "lib: sbi: dbtr: validate complete shared
>   memory range", which already carries a Reviewed-by. Note that
>   Himanshu's remark on v2 3/6, that the "dom &&" guard skips the check
>   entirely when dom is NULL, applies to that patch as well and is left
>   to its thread.
> - Link to v2: https://lore.kernel.org/opensbi/20260731103405.1535818-1-liutong@iscas.ac.cn/
> 
> liutong (3):
>   lib: sbi: dbtr: fix shared memory double-fetch in install_trig
>   lib: sbi: dbtr: fix shared memory double-fetch in update_trig
>   lib: sbi: sse: fix shared memory double-fetch in sse_write_attrs
> 
>  lib/sbi/sbi_dbtr.c | 226 +++++++++++++++++++++++++++------------------
>  lib/sbi/sbi_sse.c  |  26 ++++--
>  2 files changed, 158 insertions(+), 94 deletions(-)
>
> 
> base-commit: 3593a5facc4c6938b90429a6973ba9ee21fc5899
> -- 
> 2.34.1
>