linux-kernel - Re: [PATCH v3 03/15] iommu: Always register bus notifiers

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: <3d613192-f673-852e-9c52-b8a913d25616@arm.com>
Date:   Wed, 6 Jul 2022 14:43:40 +0100
From:   Robin Murphy <robin.murphy@....com>
To:     Baolu Lu <baolu.lu@...ux.intel.com>, joro@...tes.org
Cc:     will@...nel.org, iommu@...ts.linux.dev,
        linux-arm-kernel@...ts.infradead.org,
        suravee.suthikulpanit@....com, vasant.hegde@....com,
        mjrosato@...ux.ibm.com, gerald.schaefer@...ux.ibm.com,
        schnelle@...ux.ibm.com, linux-s390@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3 03/15] iommu: Always register bus notifiers

On 2022-07-06 02:53, Baolu Lu wrote:
> On 2022/7/6 01:08, Robin Murphy wrote:
>>   /*
>>    * Use a function instead of an array here because the domain-type is a
>>    * bit-field, so an array would waste memory.
>> @@ -152,6 +172,10 @@ static int __init iommu_subsys_init(void)
>>               (iommu_cmd_line & IOMMU_CMD_LINE_STRICT) ?
>>                   "(set via kernel command line)" : "");
>> +    /* If the system is so broken that this fails, it will WARN 
>> anyway */
> 
> Can you please elaborate a bit on this? iommu_bus_init() still return
> errors.

Indeed, it's commenting on the fact that we don't try to clean up or 
propagate an error value further even if it did ever manage to return 
one. I feared that if I strip the error handling out of iommu_bus_init() 
itself on the same reasoning, we'll just get constant patches from the 
static checker brigade trying to add it back, so it seemed like the 
neatest compromise to keep that decision where it's obviously in an 
early initcall, rather than in the helper function which can be viewed 
out of context. However, I'm happy to either expand this comment or go 
the whole way and make iommu_bus_init() return void if you think it's 
worthwhile.

Cheers,
Robin.

> 
>> +    for (int i = 0; i < ARRAY_SIZE(iommu_buses); i++)
>> +        iommu_bus_init(iommu_buses[i]);
>> +
>>       return 0;
> 
> Best regards,
> baolu