[libcamera-devel] meson: fix build when sys/auxv.h and getauxval() are not present

1097 diff mbox series

On some libc sys/auxv.h could not be present and getauxval() too.
This way build will fail.

Check in meson if they are present and add HAVE_SYS_AUXV_H and
HAVE_GETAUXVAL defines to cxx arguments.
Add #ifdef HAVE_ statements around #include <sys/auxv.h> and getauxval()
in utils.cpp.

Signed-off-by: Giulio Benetti <giulio.benetti@micronovasrl.com>
---
 meson.build             | 12 ++++++++++++
 src/libcamera/utils.cpp |  4 ++++
 2 files changed, 16 insertions(+)

Hi Giulio,

Thank you for looking at this issue.

Following the discussion at [0], I intend to try and implement a
secure_getenv() which does not use getauxval() instead.

The use of getauxval() is already a workaround for not having
secure_getenv() available, so we should instead determine if
secure_getenv() is provided, and if not use a fallback which is
implemented with issetugid().

[0] https://marc.info/?l=buildroot&m=155510281716087&w=2


On 23/04/2019 12:09, Giulio Benetti wrote:
> On some libc sys/auxv.h could not be present and getauxval() too.
> This way build will fail.
> 
> Check in meson if they are present and add HAVE_SYS_AUXV_H and
> HAVE_GETAUXVAL defines to cxx arguments.
> Add #ifdef HAVE_ statements around #include <sys/auxv.h> and getauxval()
> in utils.cpp.
> 
> Signed-off-by: Giulio Benetti <giulio.benetti@micronovasrl.com>
> ---
>  meson.build             | 12 ++++++++++++
>  src/libcamera/utils.cpp |  4 ++++
>  2 files changed, 16 insertions(+)
> 
> diff --git a/meson.build b/meson.build
> index 6e68c3e..72a3652 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -20,6 +20,18 @@ common_arguments = [
>  c_arguments = common_arguments
>  cpp_arguments = common_arguments
>  
> +cxx = meson.get_compiler('cpp')
> +
> +# check for header sys/auxv.h
> +if cxx.has_header('sys/auxv.h')
> +  cpp_arguments += ['-DHAVE_SYS_AUXV_H']
> +endif
> +
> +# check for function getauxval()
> +if cxx.has_function('getauxval')
> +  cpp_arguments += ['-DHAVE_GETAUXVAL']

based on [1] I believe Meson discourages adding defines to the
cpp_arguments where possible, and instead recommends generating a
configuration header ... something like my old work-in-progress patch
for this topic:

+cc = meson.get_compiler('c')
+config_h = configuration_data()
+
+if cc.has_header_symbol('stdlib.h', 'secure_getenv', prefix: '#define
_GNU_SOURCE')
+    config_h.set('HAVE_SECURE_GETENV', 1)
+else
+    message('C library does not support secure_getenv, using getenv
instead')
+endif
+configure_file(output: 'config.h', configuration: config_h)
+add_project_arguments('-include', 'config.h', language: 'c')


[1] https://github.com/mesonbuild/meson/issues/2247

> +endif
> +
>  add_project_arguments(c_arguments, language: 'c')
>  add_project_arguments(cpp_arguments, language: 'cpp')
>  
> diff --git a/src/libcamera/utils.cpp b/src/libcamera/utils.cpp
> index 66123b1..ae574ab 100644
> --- a/src/libcamera/utils.cpp
> +++ b/src/libcamera/utils.cpp
> @@ -8,7 +8,9 @@
>  #include "utils.h"
>  
>  #include <string.h>
> +#ifdef HAVE_SYS_AUXV_H
>  #include <sys/auxv.h>
> +#endif
>  
>  /**
>   * \file utils.h
> @@ -57,8 +59,10 @@ const char *basename(const char *path)
>   */
>  char *secure_getenv(const char *name)
>  {
> +#ifdef HAVE_GETAUXVAL
>  	if (getauxval(AT_SECURE))
>  		return NULL;
> +#endif
>  

This fall through leaves systems without getauxval() vulnerable to
attacks through set-uid programs...

Would you like to create a new patch based on the above? If not I'll
keep this on my todo list for the coming week.


>  	return getenv(name);
>  }
>

Hi Kieran,

Il 23/04/2019 18:08, Kieran Bingham ha scritto:
> Hi Giulio,
> 
> Thank you for looking at this issue.
> 
> Following the discussion at [0], I intend to try and implement a
> secure_getenv() which does not use getauxval() instead.
> 
> The use of getauxval() is already a workaround for not having
> secure_getenv() available, so we should instead determine if
> secure_getenv() is provided, and if not use a fallback which is
> implemented with issetugid().
> 
> [0] https://marc.info/?l=buildroot&m=155510281716087&w=2

I've missed that at all on Buildroot ML but it's good explained.
Thanks for pointing me.

> 
> On 23/04/2019 12:09, Giulio Benetti wrote:
>> On some libc sys/auxv.h could not be present and getauxval() too.
>> This way build will fail.
>>
>> Check in meson if they are present and add HAVE_SYS_AUXV_H and
>> HAVE_GETAUXVAL defines to cxx arguments.
>> Add #ifdef HAVE_ statements around #include <sys/auxv.h> and getauxval()
>> in utils.cpp.
>>
>> Signed-off-by: Giulio Benetti <giulio.benetti@micronovasrl.com>
>> ---
>>   meson.build             | 12 ++++++++++++
>>   src/libcamera/utils.cpp |  4 ++++
>>   2 files changed, 16 insertions(+)
>>
>> diff --git a/meson.build b/meson.build
>> index 6e68c3e..72a3652 100644
>> --- a/meson.build
>> +++ b/meson.build
>> @@ -20,6 +20,18 @@ common_arguments = [
>>   c_arguments = common_arguments
>>   cpp_arguments = common_arguments
>>   
>> +cxx = meson.get_compiler('cpp')
>> +
>> +# check for header sys/auxv.h
>> +if cxx.has_header('sys/auxv.h')
>> +  cpp_arguments += ['-DHAVE_SYS_AUXV_H']
>> +endif
>> +
>> +# check for function getauxval()
>> +if cxx.has_function('getauxval')
>> +  cpp_arguments += ['-DHAVE_GETAUXVAL']
> 
> based on [1] I believe Meson discourages adding defines to the
> cpp_arguments where possible, and instead recommends generating a
> configuration header ... something like my old work-in-progress patch
> for this topic:
> 
> +cc = meson.get_compiler('c')
> +config_h = configuration_data()
> +
> +if cc.has_header_symbol('stdlib.h', 'secure_getenv', prefix: '#define
> _GNU_SOURCE')
> +    config_h.set('HAVE_SECURE_GETENV', 1)
> +else
> +    message('C library does not support secure_getenv, using getenv
> instead')
> +endif
> +configure_file(output: 'config.h', configuration: config_h)
> +add_project_arguments('-include', 'config.h', language: 'c')

I didn't know about it, indeed this ^^^^^^^^^^^^^^ was my first choice, 
but I thought it was too much for only 2 HAVE_* but [1] clarifies it 
very well.

> 
> [1] https://github.com/mesonbuild/meson/issues/2247
> 
>> +endif
>> +
>>   add_project_arguments(c_arguments, language: 'c')
>>   add_project_arguments(cpp_arguments, language: 'cpp')
>>   
>> diff --git a/src/libcamera/utils.cpp b/src/libcamera/utils.cpp
>> index 66123b1..ae574ab 100644
>> --- a/src/libcamera/utils.cpp
>> +++ b/src/libcamera/utils.cpp
>> @@ -8,7 +8,9 @@
>>   #include "utils.h"
>>   
>>   #include <string.h>
>> +#ifdef HAVE_SYS_AUXV_H
>>   #include <sys/auxv.h>
>> +#endif
>>   
>>   /**
>>    * \file utils.h
>> @@ -57,8 +59,10 @@ const char *basename(const char *path)
>>    */
>>   char *secure_getenv(const char *name)
>>   {
>> +#ifdef HAVE_GETAUXVAL
>>   	if (getauxval(AT_SECURE))
>>   		return NULL;
>> +#endif
>>   
> 
> This fall through leaves systems without getauxval() vulnerable to
> attacks through set-uid programs...

Yes, right.

> Would you like to create a new patch based on the above? If not I'll
> keep this on my todo list for the coming week.

I can do it. I don't know how quickly but I hope before next week.

Best regards
Giulio

> 
>>   	return getenv(name);
>>   }
>>
>

Local secure_getenv() at the moment uses getauxval(AT_SECURE), but it's
not always present in libc and secure_getenv() instead could be.

Check if secure_getenv() is available from libc and use it, otherwise
workaround it using issetugid().

Giulio Benetti (2):
  meson: check if secure_getenv() is present
  libcamera: utils: call secure_getenv() if it exists or workaround with
    issetugid()

 meson.build             | 12 ++++++++++++
 src/libcamera/utils.cpp |  8 ++++++--
 2 files changed, 18 insertions(+), 2 deletions(-)