Hi Robin, On 2020/12/7 20:46, Robin Murphy wrote: > On 2020-12-07 12:15, zhukeqian wrote: >> Hi, >> >> On 2020/12/7 20:05, Will Deacon wrote: >>> On Mon, Dec 07, 2020 at 12:01:09PM +0000, Robin Murphy wrote: >>>> On 2020-12-05 08:29, Keqian Zhu wrote: >>>>> ... then we have more chance to detect wrong code logic. >>>> >>>> I don't follow that justification - it's still the same check with the same >>>> outcome, so how does moving it have any effect on the chance to detect >>>> errors? >> >>>> >>>> AFAICS the only difference it would make is to make some errors *less* >>>> obvious - if a sufficiently broken caller passes an empty prot value >>>> alongside an invalid size or already-mapped address, this will now quietly >>>> hide the warnings from the more serious condition(s). >>>> >>>> Yes, it will bail out a bit faster in the specific case where the prot >>>> value >>>> is the only thing wrong, but since when do we optimise for fundamentally >>>> incorrect API usage? >>> >>> I thought it was the other way round -- doesn't this patch move the "empty >>> prot" check later, so we have a chance to check the size and addresses >>> first? >> >> Yes, this is my original idea. >> For that we treat iommu_prot with no permission as success at early start, >> defer >> this early return can expose hidden errors. > > ...oh dear, my apologies. I've just had a week off and apparently in that > time I lost the ability to read :( > > I was somehow convinced that the existing check happened at the point where > we go to install the PTE, and this patch was moving it earlier. Looking at > the whole code in context now I see I got it completely backwards. Sorry for > being an idiot. > I see :-) I should make a more descriptive commit message.
> I guess that only goes to show that a more descriptive commit message would > definitely be a good thing :) > I have adapted Will's suggestion and sent v2, please check whether it is OK to you? Cheers, Keqian [...] >> _______________________________________________ >> iommu mailing list >> io...@lists.linux-foundation.org >> https://lists.linuxfoundation.org/mailman/listinfo/iommu >> > . >
