| Message ID | 20230503122035.32026-3-naush@raspberrypi.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series |
|
| Related | show |
Hi Naush On Wed, May 03, 2023 at 01:20:24PM +0100, Naushir Patuck via libcamera-devel wrote: > Remove the restriction on using the '/' character in the IPA name > string. This allows more flexibility in IPA directory structures where > different IPA modules might live in subdirectories under the usual > src/ipa/<platform>/ top level directory. > > Signed-off-by: Naushir Patuck <naush@raspberrypi.com> Looks good! Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com> > --- > src/libcamera/ipa_module.cpp | 11 +++++------ > 1 file changed, 5 insertions(+), 6 deletions(-) > > diff --git a/src/libcamera/ipa_module.cpp b/src/libcamera/ipa_module.cpp > index c152153c180f..3ca7074b3b58 100644 > --- a/src/libcamera/ipa_module.cpp > +++ b/src/libcamera/ipa_module.cpp > @@ -225,9 +225,9 @@ Span<const uint8_t> elfLoadSymbol(Span<const uint8_t> elf, const char *symbol) > * \brief The name of the IPA module > * > * The name may be used to build file system paths to IPA-specific resources. > - * It shall only contain printable characters, and may not contain '/', '*', > - * '?' or '\'. For IPA modules included in libcamera, it shall match the > - * directory of the IPA module in the source tree. > + * It shall only contain printable characters, and may not contain '*', '?' or > + * '\'. For IPA modules included in libcamera, it shall match the directory of > + * the IPA module in the source tree. > * > * \todo Allow user to choose to isolate open source IPAs > */ > @@ -304,9 +304,8 @@ int IPAModule::loadIPAModuleInfo() > std::string ipaName = info_.name; > auto iter = std::find_if_not(ipaName.begin(), ipaName.end(), > [](unsigned char c) -> bool { > - return isprint(c) && c != '/' && > - c != '?' && c != '*' && > - c != '\\'; > + return isprint(c) && c != '?' && > + c != '*' && c != '\\'; > }); > if (iter != ipaName.end()) { > LOG(IPAModule, Error) > -- > 2.34.1 >
Hi Naush, Thank you for the patch. On Wed, May 03, 2023 at 01:20:24PM +0100, Naushir Patuck via libcamera-devel wrote: > Remove the restriction on using the '/' character in the IPA name > string. This allows more flexibility in IPA directory structures where > different IPA modules might live in subdirectories under the usual > src/ipa/<platform>/ top level directory. > > Signed-off-by: Naushir Patuck <naush@raspberrypi.com> > --- > src/libcamera/ipa_module.cpp | 11 +++++------ > 1 file changed, 5 insertions(+), 6 deletions(-) > > diff --git a/src/libcamera/ipa_module.cpp b/src/libcamera/ipa_module.cpp > index c152153c180f..3ca7074b3b58 100644 > --- a/src/libcamera/ipa_module.cpp > +++ b/src/libcamera/ipa_module.cpp > @@ -225,9 +225,9 @@ Span<const uint8_t> elfLoadSymbol(Span<const uint8_t> elf, const char *symbol) > * \brief The name of the IPA module > * > * The name may be used to build file system paths to IPA-specific resources. > - * It shall only contain printable characters, and may not contain '/', '*', > - * '?' or '\'. For IPA modules included in libcamera, it shall match the > - * directory of the IPA module in the source tree. > + * It shall only contain printable characters, and may not contain '*', '?' or > + * '\'. For IPA modules included in libcamera, it shall match the directory of > + * the IPA module in the source tree. > * > * \todo Allow user to choose to isolate open source IPAs > */ > @@ -304,9 +304,8 @@ int IPAModule::loadIPAModuleInfo() > std::string ipaName = info_.name; > auto iter = std::find_if_not(ipaName.begin(), ipaName.end(), > [](unsigned char c) -> bool { > - return isprint(c) && c != '/' && > - c != '?' && c != '*' && > - c != '\\'; > + return isprint(c) && c != '?' && > + c != '*' && c != '\\'; I think I mentioned in the review of the previous version that we should disallow ".." to avoid escaping into the file system. I however have a bit of a hard time at this point to decide if it would be best implemented here or elsewhere, or what the exact naming rules should be. I'll thus add a * \todo Consider module naming restrictions to avoid escaping from a base * directory. Forbidding ".." may be enough, but this may be best implemented * in a different layer. comment. Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > }); > if (iter != ipaName.end()) { > LOG(IPAModule, Error)
diff --git a/src/libcamera/ipa_module.cpp b/src/libcamera/ipa_module.cpp index c152153c180f..3ca7074b3b58 100644 --- a/src/libcamera/ipa_module.cpp +++ b/src/libcamera/ipa_module.cpp @@ -225,9 +225,9 @@ Span<const uint8_t> elfLoadSymbol(Span<const uint8_t> elf, const char *symbol) * \brief The name of the IPA module * * The name may be used to build file system paths to IPA-specific resources. - * It shall only contain printable characters, and may not contain '/', '*', - * '?' or '\'. For IPA modules included in libcamera, it shall match the - * directory of the IPA module in the source tree. + * It shall only contain printable characters, and may not contain '*', '?' or + * '\'. For IPA modules included in libcamera, it shall match the directory of + * the IPA module in the source tree. * * \todo Allow user to choose to isolate open source IPAs */ @@ -304,9 +304,8 @@ int IPAModule::loadIPAModuleInfo() std::string ipaName = info_.name; auto iter = std::find_if_not(ipaName.begin(), ipaName.end(), [](unsigned char c) -> bool { - return isprint(c) && c != '/' && - c != '?' && c != '*' && - c != '\\'; + return isprint(c) && c != '?' && + c != '*' && c != '\\'; }); if (iter != ipaName.end()) { LOG(IPAModule, Error)
Remove the restriction on using the '/' character in the IPA name string. This allows more flexibility in IPA directory structures where different IPA modules might live in subdirectories under the usual src/ipa/<platform>/ top level directory. Signed-off-by: Naushir Patuck <naush@raspberrypi.com> --- src/libcamera/ipa_module.cpp | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-)