linux-kernel - Re: [PATCH 2/3] iova: Remove magazine BUG

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]

Date:   Wed, 17 Aug 2022 12:12:03 +0100
From:   Robin Murphy <robin.murphy@....com>
To:     John Garry <john.garry@...wei.com>, joro@...tes.org,
        will@...nel.org
Cc:     iommu@...ts.linux.dev, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/3] iova: Remove magazine BUG_ON() checks

On 2022-08-17 11:09, John Garry wrote:
> Two of the magazine helpers have BUG_ON() checks, as follows:
> - iova_magazine_pop() - here we ensure that the mag is not empty. However we
>    already ensure that in the only caller, __iova_rcache_get().
> - iova_magazine_push() - here we ensure that the mag is not full. However
>    we already ensure that in the only caller, __iova_rcache_insert().
> 
> As described, the two bug checks are pointless so drop them.

In some ways it's logical to have assertions to make sure that callers 
*are* already checking so as not to call incorrectly. However I'm 
inclined to agree that this particular case is very niche, with little 
chance of any new callers being added, so they're not offering much 
value, especially on performance-critical paths.

Acked-by: Robin Murphy <robin.murphy@....com>

> Signed-off-by: John Garry <john.garry@...wei.com>
> ---
>   drivers/iommu/iova.c | 4 ----
>   1 file changed, 4 deletions(-)
> 
> diff --git a/drivers/iommu/iova.c b/drivers/iommu/iova.c
> index 580fdf669922..8aece052ce72 100644
> --- a/drivers/iommu/iova.c
> +++ b/drivers/iommu/iova.c
> @@ -694,8 +694,6 @@ static unsigned long iova_magazine_pop(struct iova_magazine *mag,
>   	int i;
>   	unsigned long pfn;
>   
> -	BUG_ON(iova_magazine_empty(mag));
> -
>   	/* Only fall back to the rbtree if we have no suitable pfns at all */
>   	for (i = mag->size - 1; mag->pfns[i] > limit_pfn; i--)
>   		if (i == 0)
> @@ -710,8 +708,6 @@ static unsigned long iova_magazine_pop(struct iova_magazine *mag,
>   
>   static void iova_magazine_push(struct iova_magazine *mag, unsigned long pfn)
>   {
> -	BUG_ON(iova_magazine_full(mag));
> -
>   	mag->pfns[mag->size++] = pfn;
>   }
>