Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Medium] `virtio_dmb_hiwater_set` contains a TOCTOU race with `virtio_dmb_inc_used` that can cause the high-water mark to be permanently set lower than the current total usage. --
--- Patch [11]: [RFC PATCH 11/12] virtio: expose device memory buffer occupancy over debugfs --- commit 821ee4cbeac917d62e62a95c88f58236d2f6db9c Author: Alexander Graf <[email protected]> virtio: expose device memory buffer occupancy over debugfs This commit adds debugfs files to expose the memory occupancy of virtio device memory buffers when CONFIG_VIRTIO_DEBUG is enabled. It introduces counters for total allocated pages, allocation failures, and a high-water mark to track peak usage. > diff --git a/drivers/virtio/virtio_dmb.c b/drivers/virtio/virtio_dmb.c > index 42126f928bc06..fe556d585f244 100644 > --- a/drivers/virtio/virtio_dmb.c > +++ b/drivers/virtio/virtio_dmb.c [ ... ] > +static int virtio_dmb_hiwater_set(void *data, u64 val) > +{ > + struct virtio_dmb *dmb = data; > + > + /* Restarting the measurement is the only meaningful write. */ > + if (val) > + return -EINVAL; > + > + /* > + * Restart from what is allocated now rather than from zero, so that > + * the file never reports a peak below the occupancy it is read > + * alongside. > + */ > + atomic_long_set(&dmb->used_hiwater, > + atomic_long_read(&dmb->total_used)); [Severity: Medium] Can this sequence result in the high-water mark being set lower than the current total usage due to a race condition? If a write to the debugfs used_pages_hiwater file occurs concurrently with new allocations, could we see the following sequence: Thread A reads total_used (e.g., 10) in virtio_dmb_hiwater_set(). Thread B allocates memory in virtio_dmb_inc_used(), increasing both total_used and used_hiwater (e.g., to 110). Thread A then blindly overwrites used_hiwater with the stale value (10) using atomic_long_set(). This seems to contradict the comment above it, leaving used_hiwater substantially lower than total_used. Is there a way to safely read and update this to avoid a time-of-check to time-of-use race? > + > + return 0; > +} > +DEFINE_DEBUGFS_ATTRIBUTE(virtio_dmb_hiwater_fops, virtio_dmb_hiwater_get, > + virtio_dmb_hiwater_set, "%llu\n"); [ ... ] -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=11
