linux-kernel - Re: [PATCH v4 07/10] KVM: arm64: Convert pkvm

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <aCs-jYUrEraCnLaX@google.com>
Date: Mon, 19 May 2025 14:22:05 +0000
From: Quentin Perret <qperret@...gle.com>
To: Marc Zyngier <maz@...nel.org>
Cc: Vincent Donnefort <vdonnefort@...gle.com>, oliver.upton@...ux.dev,
	joey.gouly@....com, suzuki.poulose@....com, yuzenghui@...wei.com,
	catalin.marinas@....com, will@...nel.org,
	linux-arm-kernel@...ts.infradead.org, kvmarm@...ts.linux.dev,
	linux-kernel@...r.kernel.org, kernel-team@...roid.com
Subject: Re: [PATCH v4 07/10] KVM: arm64: Convert pkvm_mappings to interval
 tree

On Friday 16 May 2025 at 14:15:45 (+0100), Marc Zyngier wrote:
> > -/*
> > - * __tmp is updated to rb_next(__tmp) *before* entering the body of the loop to allow freeing
> > - * of __map inline.
> > - */
> > +INTERVAL_TREE_DEFINE(struct pkvm_mapping, node, u64, __subtree_last,
> > +		     __pkvm_mapping_start, __pkvm_mapping_end, static,
> > +		     pkvm_mapping);
> > +
> >  #define for_each_mapping_in_range_safe(__pgt, __start, __end, __map)				\
> > -	for (struct rb_node *__tmp = find_first_mapping_node(&(__pgt)->pkvm_mappings,		\
> > -							     ((__start) >> PAGE_SHIFT));	\
> > +	for (struct pkvm_mapping *__tmp = pkvm_mapping_iter_first(&(__pgt)->pkvm_mappings,	\
> > +								  __start, __end - 1);		\
> >  	     __tmp && ({									\
> > -				__map = rb_entry(__tmp, struct pkvm_mapping, node);		\
> > -				__tmp = rb_next(__tmp);						\
> > +				__map = __tmp;							\
> > +				__tmp = pkvm_mapping_iter_next(__map, __start, __end - 1);	\
> >  				true;								\
> >  		       });									\
> > -	    )											\
> > -		if (__map->gfn < ((__start) >> PAGE_SHIFT))					\
> > -			continue;								\
> > -		else if (__map->gfn >= ((__end) >> PAGE_SHIFT))					\
> > -			break;									\
> > -		else
> > +	    )
> 
> The removal of the comment worries me a bit. Is this iterator still
> safe wrt freeing of the iterator in the loop?

Yep it is still safe (we're still caching the next value in __tmp before
entering the body of the loop). But we shouldn't remove the comment
altogether, it just needs an update.

Cheers,
Quentin