[PATCH] Makefile: add USE_PIE

[PATCH] Makefile: add USE_PIE

[Fabrice Fontaine]

Do not hardcode -pie and allow the user to drop it (e.g. PIE could be
enabled or disabled by the buildsystem such as buildroot)

Signed-off-by: Fabrice Fontaine <fontaine.fabrice@gmail.com>
---
 Makefile | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/Makefile b/Makefile
index 7c221a89..a5269687 100644
--- a/Makefile
+++ b/Makefile
@@ -137,7 +137,11 @@ LDFLAGS = -Wl,-z,now,-z,noexecstack
 # If you want a static binary, you might uncomment these
 # LDFLAGS += -static
 # STRIP = -s
-LDLIBS = -ldl -pie
+LDLIBS = -ldl
+USE_PIE = 1
+ifdef USE_PIE
+LDLIBS += -pie
+endif
 
 # To explicitly disable libudev, set -DNO_LIBUDEV in CXFLAGS
 ifeq (, $(findstring -DNO_LIBUDEV,  $(CXFLAGS)))
-- 
2.43.0

Re: [PATCH] Makefile: add USE_PIE

[Paul Menzel]

Dear Fabrice,


Thank you for your patch.

Am 05.05.24 um 15:39 schrieb Fabrice Fontaine:
> Do not hardcode -pie and allow the user to drop it (e.g. PIE could be
> enabled or disabled by the buildsystem such as buildroot)

This sounds reasonable, but it changes the current default behavior, 
doesn’t it? Could you please elaborate, when this was added, and if the 
new default would break systems?

A formal nit pick for the commit messages would be to please add a 
dot/period at the end of sentences.)

> Signed-off-by: Fabrice Fontaine <fontaine.fabrice@gmail.com>
> ---
>   Makefile | 6 +++++-
>   1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/Makefile b/Makefile
> index 7c221a89..a5269687 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -137,7 +137,11 @@ LDFLAGS = -Wl,-z,now,-z,noexecstack
>   # If you want a static binary, you might uncomment these
>   # LDFLAGS += -static
>   # STRIP = -s
> -LDLIBS = -ldl -pie
> +LDLIBS = -ldl
> +USE_PIE = 1
> +ifdef USE_PIE
> +LDLIBS += -pie
> +endif
>   
>   # To explicitly disable libudev, set -DNO_LIBUDEV in CXFLAGS
>   ifeq (, $(findstring -DNO_LIBUDEV,  $(CXFLAGS)))


Kind regards,

Paul

Re: [PATCH] Makefile: add USE_PIE

[Fabrice Fontaine]

Dear Paul,

Le dim. 5 mai 2024 à 19:22, Paul Menzel <pmenzel@molgen.mpg.de> a écrit :
>
> Dear Fabrice,
>
>
> Thank you for your patch.
>
> Am 05.05.24 um 15:39 schrieb Fabrice Fontaine:
> > Do not hardcode -pie and allow the user to drop it (e.g. PIE could be
> > enabled or disabled by the buildsystem such as buildroot)
>
> This sounds reasonable, but it changes the current default behavior,
> doesn’t it? Could you please elaborate, when this was added, and if the
> new default would break systems?

Why are you saying that it changes the current default behavior?
USE_PIE is set to 1 by default but perhaps I missed something.

>
> A formal nit pick for the commit messages would be to please add a
> dot/period at the end of sentences.)
>
> > Signed-off-by: Fabrice Fontaine <fontaine.fabrice@gmail.com>
> > ---
> >   Makefile | 6 +++++-
> >   1 file changed, 5 insertions(+), 1 deletion(-)
> >
> > diff --git a/Makefile b/Makefile
> > index 7c221a89..a5269687 100644
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -137,7 +137,11 @@ LDFLAGS = -Wl,-z,now,-z,noexecstack
> >   # If you want a static binary, you might uncomment these
> >   # LDFLAGS += -static
> >   # STRIP = -s
> > -LDLIBS = -ldl -pie
> > +LDLIBS = -ldl
> > +USE_PIE = 1
> > +ifdef USE_PIE
> > +LDLIBS += -pie
> > +endif
> >
> >   # To explicitly disable libudev, set -DNO_LIBUDEV in CXFLAGS
> >   ifeq (, $(findstring -DNO_LIBUDEV,  $(CXFLAGS)))
>
>
> Kind regards,
>
> Paul

Best Regards,

Fabrice

Re: [PATCH] Makefile: add USE_PIE

[Paul Menzel]

Dear Fabrice,


Am 05.05.24 um 19:38 schrieb Fabrice Fontaine:

> Le dim. 5 mai 2024 à 19:22, Paul Menzel <pmenzel@molgen.mpg.de> a écrit :

>> Am 05.05.24 um 15:39 schrieb Fabrice Fontaine:
>>> Do not hardcode -pie and allow the user to drop it (e.g. PIE could be
>>> enabled or disabled by the buildsystem such as buildroot)
>>
>> This sounds reasonable, but it changes the current default behavior,
>> doesn’t it? Could you please elaborate, when this was added, and if the
>> new default would break systems?
> 
> Why are you saying that it changes the current default behavior?
> USE_PIE is set to 1 by default but perhaps I missed something.

Sorry, I overlooked `USE_PIE = 1` in the diff. :/

>> A formal nit pick for the commit messages would be to please add a
>> dot/period at the end of sentences.)
>>
>>> Signed-off-by: Fabrice Fontaine <fontaine.fabrice@gmail.com>
>>> ---
>>>    Makefile | 6 +++++-
>>>    1 file changed, 5 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/Makefile b/Makefile
>>> index 7c221a89..a5269687 100644
>>> --- a/Makefile
>>> +++ b/Makefile
>>> @@ -137,7 +137,11 @@ LDFLAGS = -Wl,-z,now,-z,noexecstack
>>>    # If you want a static binary, you might uncomment these
>>>    # LDFLAGS += -static
>>>    # STRIP = -s
>>> -LDLIBS = -ldl -pie
>>> +LDLIBS = -ldl
>>> +USE_PIE = 1
>>> +ifdef USE_PIE
>>> +LDLIBS += -pie
>>> +endif
>>>
>>>    # To explicitly disable libudev, set -DNO_LIBUDEV in CXFLAGS
>>>    ifeq (, $(findstring -DNO_LIBUDEV,  $(CXFLAGS)))


Kind regards,

Paul

Re: [PATCH] Makefile: add USE_PIE

[Mariusz Tkaczyk]

On Sun,  5 May 2024 15:39:23 +0200
Fabrice Fontaine <fontaine.fabrice@gmail.com> wrote:

> Do not hardcode -pie and allow the user to drop it (e.g. PIE could be
> enabled or disabled by the buildsystem such as buildroot)

What about -fPIE? It is in CWFLAGS but it is configurable.
Do you specify you own set of CWFLAGS?

> 
> Signed-off-by: Fabrice Fontaine <fontaine.fabrice@gmail.com>
> ---
>  Makefile | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/Makefile b/Makefile
> index 7c221a89..a5269687 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -137,7 +137,11 @@ LDFLAGS = -Wl,-z,now,-z,noexecstack
>  # If you want a static binary, you might uncomment these
>  # LDFLAGS += -static
>  # STRIP = -s
> -LDLIBS = -ldl -pie
> +LDLIBS = -ldl
> +USE_PIE = 1
> +ifdef USE_PIE
> +LDLIBS += -pie
> +endif
>  
>  # To explicitly disable libudev, set -DNO_LIBUDEV in CXFLAGS
>  ifeq (, $(findstring -DNO_LIBUDEV,  $(CXFLAGS)))

AFAIK -pie is not library specifier, it is a a gcc linking setting so having it
in LDLIBS seems weird to me. What about making LDFLAGS configurable?

diff --git a/Makefile b/Makefile
index 7c221a891181..adac7905ab57 100644
--- a/Makefile
+++ b/Makefile
@@ -132,12 +132,12 @@ CFLAGS += -DUSE_PTHREADS
 MON_LDFLAGS += -pthread
 endif

-LDFLAGS = -Wl,-z,now,-z,noexecstack
+LDFLAGS ?= -pie -Wl,-z,now,-z,noexecstack

 # If you want a static binary, you might uncomment these
 # LDFLAGS += -static
 # STRIP = -s
-LDLIBS = -ldl -pie
+LDLIBS = -ldl

It works on my setup however I'm not deeply sure if it is correct.
Let me know if it resolves your issue. I would prefer to give possibility to
customize LDFLAGS rather than add ifdef to Makefile.

Thanks,
Mariusz

Re: [PATCH] Makefile: add USE_PIE

[Fabrice Fontaine]

Le lun. 6 mai 2024 à 16:56, Mariusz Tkaczyk
<mariusz.tkaczyk@linux.intel.com> a écrit :
>
> On Sun,  5 May 2024 15:39:23 +0200
> Fabrice Fontaine <fontaine.fabrice@gmail.com> wrote:
>
> > Do not hardcode -pie and allow the user to drop it (e.g. PIE could be
> > enabled or disabled by the buildsystem such as buildroot)
>
> What about -fPIE? It is in CWFLAGS but it is configurable.
> Do you specify you own set of CWFLAGS?

Yes, CWFLAGS is set to an empty value.

>
> >
> > Signed-off-by: Fabrice Fontaine <fontaine.fabrice@gmail.com>
> > ---
> >  Makefile | 6 +++++-
> >  1 file changed, 5 insertions(+), 1 deletion(-)
> >
> > diff --git a/Makefile b/Makefile
> > index 7c221a89..a5269687 100644
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -137,7 +137,11 @@ LDFLAGS = -Wl,-z,now,-z,noexecstack
> >  # If you want a static binary, you might uncomment these
> >  # LDFLAGS += -static
> >  # STRIP = -s
> > -LDLIBS = -ldl -pie
> > +LDLIBS = -ldl
> > +USE_PIE = 1
> > +ifdef USE_PIE
> > +LDLIBS += -pie
> > +endif
> >
> >  # To explicitly disable libudev, set -DNO_LIBUDEV in CXFLAGS
> >  ifeq (, $(findstring -DNO_LIBUDEV,  $(CXFLAGS)))
>
> AFAIK -pie is not library specifier, it is a a gcc linking setting so having it
> in LDLIBS seems weird to me. What about making LDFLAGS configurable?

Sure, moving -pie to LDFLAGS and allowing the user to override it will
also work.

>
> diff --git a/Makefile b/Makefile
> index 7c221a891181..adac7905ab57 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -132,12 +132,12 @@ CFLAGS += -DUSE_PTHREADS
>  MON_LDFLAGS += -pthread
>  endif
>
> -LDFLAGS = -Wl,-z,now,-z,noexecstack
> +LDFLAGS ?= -pie -Wl,-z,now,-z,noexecstack
>
>  # If you want a static binary, you might uncomment these
>  # LDFLAGS += -static
>  # STRIP = -s
> -LDLIBS = -ldl -pie
> +LDLIBS = -ldl
>
> It works on my setup however I'm not deeply sure if it is correct.
> Let me know if it resolves your issue. I would prefer to give possibility to
> customize LDFLAGS rather than add ifdef to Makefile.
>
> Thanks,
> Mariusz

Best Regards,

Fabrice

Re: [PATCH] Makefile: add USE_PIE

[Mariusz Tkaczyk]

On Mon, 6 May 2024 17:54:22 +0200
Fabrice Fontaine <fontaine.fabrice@gmail.com> wrote:

> > AFAIK -pie is not library specifier, it is a a gcc linking setting so
> > having it in LDLIBS seems weird to me. What about making LDFLAGS
> > configurable?  
> 
> Sure, moving -pie to LDFLAGS and allowing the user to override it will
> also work.

Could you please send v2 with possibility to customize LDFLAGS? You can add me
in Suggested-by.

Thanks,
Mariusz