Re: [PATCH] virtio_mem: fix hardcoded 'vm' variable in bbm iteration macros

[David Hildenbrand (Arm)]

On 6/29/26 05:37, lirongqing wrote:
> From: Li RongQing <lirongqing@xxxxxxxxx>
> 
> virtio_mem_bbm_for_each_bb() and virtio_mem_bbm_for_each_bb_rev()
> accept a '_vm' parameter to allow callers to pass any variable name
> referring to the virtio_mem instance. However, the 'for' loop
> initializer and part of the loop condition use the bare name 'vm'
> instead of the macro parameter '_vm':

Right.

> 
> All current call sites happen to pass a variable named 'vm', so the
> bug is latent and does not cause a regression today.

Unlikely to happen, because no code that iterates memory blocks deals with two
virtio-mem devices at the same time.

> Nevertheless,
> the macros are formally broken: any future caller using a different
> variable name would silently capture the outer 'vm' from the
> enclosing scope, leading to incorrect iteration bounds.

Again, unlikely, but we should clean this up.

> 
> Fix by replacing all bare 'vm->' references inside the macros with
> the '_vm' parameter, and wrap in parentheses following kernel macro
> hygiene conventions.
> 
> Fixes: 4ba50cd3355d ("virtio-mem: Big Block Mode (BBM) memory hotplug")

There is no user-visible bug, so drop the Fixes: tag.

> Signed-off-by: Li RongQing <lirongqing@xxxxxxxxx>
> ---
>  drivers/virtio/virtio_mem.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/virtio/virtio_mem.c b/drivers/virtio/virtio_mem.c
> index 11c4415..82a285c 100644
> --- a/drivers/virtio/virtio_mem.c
> +++ b/drivers/virtio/virtio_mem.c
> @@ -423,14 +423,14 @@ static int virtio_mem_bbm_bb_states_prepare_next_bb(struct virtio_mem *vm)
>  }
>  
>  #define virtio_mem_bbm_for_each_bb(_vm, _bb_id, _state) \
> -	for (_bb_id = vm->bbm.first_bb_id; \
> -	     _bb_id < vm->bbm.next_bb_id && _vm->bbm.bb_count[_state]; \
> +	for (_bb_id = (_vm)->bbm.first_bb_id; \
> +	     _bb_id < (_vm)->bbm.next_bb_id && (_vm)->bbm.bb_count[_state]; \
>  	     _bb_id++) \
>  		if (virtio_mem_bbm_get_bb_state(_vm, _bb_id) == _state)
>  
>  #define virtio_mem_bbm_for_each_bb_rev(_vm, _bb_id, _state) \
> -	for (_bb_id = vm->bbm.next_bb_id - 1; \
> -	     _bb_id >= vm->bbm.first_bb_id && _vm->bbm.bb_count[_state]; \
> +	for (_bb_id = (_vm)->bbm.next_bb_id - 1; \
> +	     _bb_id >= (_vm)->bbm.first_bb_id && (_vm)->bbm.bb_count[_state]; \
>  	     _bb_id--) \
>  		if (virtio_mem_bbm_get_bb_state(_vm, _bb_id) == _state)
>  

Without the Fixes: tag

Acked-by: David Hildenbrand (Arm) <david@xxxxxxxxxx>

Thanks!


-- 
Cheers,

David