[libcamera-devel] [PATCH v2 1/2] libcamera: ipa_manager: Only parse IPA modules

[Kieran Bingham]

On 04/05/2023 16:10, Laurent Pinchart wrote:
> Hi Kieran,
> 
> Thank you for the patch.
> 
> On Thu, May 04, 2023 at 03:48:00PM +0100, Kieran Bingham via libcamera-devel wrote:
>> Ensure that when we iterate the libcamera libdir we only select shared
>> objects which are expected to be IPA modules, with an ipa_ prefix.
>>
>> Any shared object not matching this filter is ignored and not processed
>> by the IPA Manager.
>>
>> Reviewed-by: Javier Martinez Canillas <javierm at redhat.com>
>> Signed-off-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> 
> I wonder what this protects against, as there shouldn't be other files
> in this directory, especially if we move IPA modules to an ipa/
> subdirectory. I don't mind much, as this isn't a hot path, so I have no
> objection if you want to merge this, but is it useful ?

Only that it prevents us trying to load any files that might be there 
otherwise.



[76:32:31.897778214] [2776180] ERROR IPAModule ipa_module.cpp:286 
fake.so: IPA module is not an ELF file
[76:32:31.899112311] [2776180] ERROR IPAModule ipa_module.cpp:172 Symbol 
ipaModuleInfo not found
[76:32:31.899119975] [2776180] ERROR IPAModule ipa_module.cpp:292 
v4l2-compat.so: IPA module has no valid info


We already filter on .so objects, and we do other checks so it's not 
crucial, and it's not going to prevent any attempt to get 
'non-libcamera' code parsed as anyone dropping a file here could equally 
call it 'ipa_fake.so' ... so no not really.

I'll drop this and collect 2/2

--
Kieran



> 
>> ---
>>   src/libcamera/ipa_manager.cpp | 4 ++++
>>   1 file changed, 4 insertions(+)
>>
>> diff --git a/src/libcamera/ipa_manager.cpp b/src/libcamera/ipa_manager.cpp
>> index 030ef43fb994..c0c2f027e902 100644
>> --- a/src/libcamera/ipa_manager.cpp
>> +++ b/src/libcamera/ipa_manager.cpp
>> @@ -206,6 +206,10 @@ void IPAManager::parseDir(const char *libDir, unsigned int maxDepth,
>>   		if (strcmp(&ent->d_name[offset], ".so"))
>>   			continue;
>>   
>> +		/* Ignore any modules which are not IPAs. */
>> +		if (strncmp(ent->d_name, "ipa_", 4) != 0)
>> +			continue;
>> +
>>   		files.push_back(std::string(libDir) + "/" + ent->d_name);
>>   	}
>>   
>