linux-kernel - Re: [PATCH v2] iommu/iova: Reset max32_alloc

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:   Thu, 3 Mar 2022 07:52:38 +0800
From:   Miles Chen <miles.chen@...iatek.com>
To:     <robin.murphy@....com>
CC:     <Libo.Kang@...iatek.com>, <Ning.Li@...iatek.com>,
        <iommu@...ts.linux-foundation.org>,
        <linux-arm-kernel@...ts.infradead.org>,
        <linux-kernel@...r.kernel.org>,
        <linux-mediatek@...ts.infradead.org>, <matthias.bgg@...il.com>,
        <miles.chen@...iatek.com>, <stable@...r.kernel.org>,
        <will@...nel.org>, <wsd_upstream@...iatek.com>,
        <yf.wang@...iatek.com>
Subject: Re: [PATCH v2] iommu/iova: Reset max32_alloc_size after cleaning

>> If no cached iova is freed, resetting max32_alloc_size before
>> the retry allocation only give us a retry. Is it possible that
>> other users free their iovas during the additional retry?
>
> No, it's not possible, since everyone's serialised by iova_rbtree_lock. 
> If the caches were already empty and the retry gets the lock first, it 
> will still fail again - forcing a reset of max32_alloc_size only means 
> it has to take the slow path to that failure. If another caller *did* 
> manage to get in and free something between free_global_cached_iovas() 
> dropping the lock and alloc_iova() re-taking it, then that would have 
> legitimately reset max32_alloc_size anyway.

Thanks for your explanation.

YF showed me some numbers yesterday and maybe we can have a further
discussion in that test case. (It looks like that some iovas are freed but
their pfn_lo(s) are less than cached_iova->pfn_lo, so max32_alloc_size is not
reset. So there are enought free iovas but the allocation still failed)

__cached_rbnode_delete_update(struct iova_domain *iovad, struct iova *free)
{
	struct iova *cached_iova;

	cached_iova = to_iova(iovad->cached32_node);
	if (free == cached_iova ||
		(free->pfn_hi < iovad->dma_32bit_pfn &&
		 free->pfn_lo >= cached_iova->pfn_lo)) {
		iovad->cached32_node = rb_next(&free->node);
		iovad->max32_alloc_size = iovad->dma_32bit_pfn;
	}
	...
}

Hi YF,
Could your share your observation of the iova allocation failure you hit?

Thanks,
Miles