[libcamera-devel,2/2] libcamera: utils: call secure_getenv() if it exists or workaround with issetugid()

Message ID 20190424110044.12608-3-giulio.benetti@micronovasrl.com
State Superseded
Headers show
Series
  • improve secure_getenv()
Related show

Commit Message

Giulio Benetti April 24, 2019, 11 a.m. UTC
When secure_getenv() is not available, need to have a workaround.

Check if secure_getenv() is present, otherwise call issetugid() on its
place.

Signed-off-by: Giulio Benetti <giulio.benetti@micronovasrl.com>
---
 src/libcamera/utils.cpp | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

Comments

Kieran Bingham April 26, 2019, 7:02 a.m. UTC | #1
Hi Giulio

On 24/04/2019 13:00, Giulio Benetti wrote:
> When secure_getenv() is not available, need to have a workaround.
> 
> Check if secure_getenv() is present, otherwise call issetugid() on its
> place.
> 
> Signed-off-by: Giulio Benetti <giulio.benetti@micronovasrl.com>
> ---
>  src/libcamera/utils.cpp | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/src/libcamera/utils.cpp b/src/libcamera/utils.cpp
> index 66123b1..529dd4a 100644
> --- a/src/libcamera/utils.cpp
> +++ b/src/libcamera/utils.cpp
> @@ -8,7 +8,7 @@
>  #include "utils.h"
>  
>  #include <string.h>
> -#include <sys/auxv.h>
> +#include <unistd.h>
>  
>  /**
>   * \file utils.h
> @@ -57,10 +57,14 @@ const char *basename(const char *path)
>   */
>  char *secure_getenv(const char *name)
>  {
> -	if (getauxval(AT_SECURE))
> +#if HAVE_SECURE_GETENV
> +	return secure_getenv(name);

This causes segfaults in the tests, which is odd as reading the code I
expected an infinite loop.

(you can run the tests with ninja test)

I believe the correct expression here is to add '::' to choose the top
level name-space explicitly:

+	return ::secure_getenv(name);


> +#else
> +	if (issetugid())
>  		return NULL;
>  
>  	return getenv(name);
> +#endif
>  }
>  
>  /**
>
Giulio Benetti April 26, 2019, 7:43 a.m. UTC | #2
Hi Kieran, All,

Il 26/04/2019 09:02, Kieran Bingham ha scritto:
> Hi Giulio
> 
> On 24/04/2019 13:00, Giulio Benetti wrote:
>> When secure_getenv() is not available, need to have a workaround.
>>
>> Check if secure_getenv() is present, otherwise call issetugid() on its
>> place.
>>
>> Signed-off-by: Giulio Benetti <giulio.benetti@micronovasrl.com>
>> ---
>>   src/libcamera/utils.cpp | 8 ++++++--
>>   1 file changed, 6 insertions(+), 2 deletions(-)
>>
>> diff --git a/src/libcamera/utils.cpp b/src/libcamera/utils.cpp
>> index 66123b1..529dd4a 100644
>> --- a/src/libcamera/utils.cpp
>> +++ b/src/libcamera/utils.cpp
>> @@ -8,7 +8,7 @@
>>   #include "utils.h"
>>   
>>   #include <string.h>
>> -#include <sys/auxv.h>
>> +#include <unistd.h>
>>   
>>   /**
>>    * \file utils.h
>> @@ -57,10 +57,14 @@ const char *basename(const char *path)
>>    */
>>   char *secure_getenv(const char *name)
>>   {
>> -	if (getauxval(AT_SECURE))
>> +#if HAVE_SECURE_GETENV
>> +	return secure_getenv(name);
> 
> This causes segfaults in the tests, which is odd as reading the code I
> expected an infinite loop.
> 
> (you can run the tests with ninja test)

This is good to know. Thanks.

> I believe the correct expression here is to add '::' to choose the top
> level name-space explicitly:
> 
> +	return ::secure_getenv(name);

Gosh, I've done a c++ recursive call instead of calling a c function.

I'm going to send v2 patchset modifying both patches.
Both reviews make a lot of sense!

Best regards
Giulio

> 
>> +#else
>> +	if (issetugid())
>>   		return NULL;
>>   
>>   	return getenv(name);
>> +#endif
>>   }
>>   
>>   /**
>>
>

Patch

diff --git a/src/libcamera/utils.cpp b/src/libcamera/utils.cpp
index 66123b1..529dd4a 100644
--- a/src/libcamera/utils.cpp
+++ b/src/libcamera/utils.cpp
@@ -8,7 +8,7 @@ 
 #include "utils.h"
 
 #include <string.h>
-#include <sys/auxv.h>
+#include <unistd.h>
 
 /**
  * \file utils.h
@@ -57,10 +57,14 @@  const char *basename(const char *path)
  */
 char *secure_getenv(const char *name)
 {
-	if (getauxval(AT_SECURE))
+#if HAVE_SECURE_GETENV
+	return secure_getenv(name);
+#else
+	if (issetugid())
 		return NULL;
 
 	return getenv(name);
+#endif
 }
 
 /**