[PATCH bpf-next] libbpf: set close-on-exec flag on gzopen

[PATCH bpf-next] libbpf: set close-on-exec flag on gzopen

[Martin Kelly]

From: Marco Vedovati <marco.vedovati@crowdstrike.com>

Enable the close-on-exec flag when using gzopen

This is especially important for multithreaded programs making use of
libbpf, where a fork + exec could race with libbpf library calls,
potentially resulting in a file descriptor leaked to the new process.

Signed-off-by: Marco Vedovati <marco.vedovati@crowdstrike.com>
---
 tools/lib/bpf/libbpf.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 17883f5a44b9..b14a4376a86e 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -1978,9 +1978,9 @@ static int bpf_object__read_kconfig_file(struct bpf_object *obj, void *data)
 		return -ENAMETOOLONG;
 
 	/* gzopen also accepts uncompressed files. */
-	file = gzopen(buf, "r");
+	file = gzopen(buf, "re");
 	if (!file)
-		file = gzopen("/proc/config.gz", "r");
+		file = gzopen("/proc/config.gz", "re");
 
 	if (!file) {
 		pr_warn("failed to open system Kconfig\n");
-- 
2.34.1

Re: [PATCH bpf-next] libbpf: set close-on-exec flag on gzopen

[Martin Kelly]

On 8/10/23 14:43, Martin Kelly wrote:
> From: Marco Vedovati <marco.vedovati@crowdstrike.com>
>
> Enable the close-on-exec flag when using gzopen
>
> This is especially important for multithreaded programs making use of
> libbpf, where a fork + exec could race with libbpf library calls,
> potentially resulting in a file descriptor leaked to the new process.
>
> Signed-off-by: Marco Vedovati <marco.vedovati@crowdstrike.com>
> ---
>   tools/lib/bpf/libbpf.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index 17883f5a44b9..b14a4376a86e 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -1978,9 +1978,9 @@ static int bpf_object__read_kconfig_file(struct bpf_object *obj, void *data)
>   		return -ENAMETOOLONG;
>   
>   	/* gzopen also accepts uncompressed files. */
> -	file = gzopen(buf, "r");
> +	file = gzopen(buf, "re");
>   	if (!file)
> -		file = gzopen("/proc/config.gz", "r");
> +		file = gzopen("/proc/config.gz", "re");
>   
>   	if (!file) {
>   		pr_warn("failed to open system Kconfig\n");

Sorry for double-sending the patch; the first was missing the bpf-next 
prefix and I wasn't sure if that would be an issue. Feel free to ignore 
this patch, as the other already got a reply.

Re: [PATCH bpf-next] libbpf: set close-on-exec flag on gzopen

[patchwork-bot+netdevbpf]

Hello:

This patch was applied to bpf/bpf-next.git (master)
by Daniel Borkmann <daniel@iogearbox.net>:

On Thu, 10 Aug 2023 14:43:53 -0700 you wrote:
> From: Marco Vedovati <marco.vedovati@crowdstrike.com>
> 
> Enable the close-on-exec flag when using gzopen
> 
> This is especially important for multithreaded programs making use of
> libbpf, where a fork + exec could race with libbpf library calls,
> potentially resulting in a file descriptor leaked to the new process.
> 
> [...]

Here is the summary with links:
  - [bpf-next] libbpf: set close-on-exec flag on gzopen
    https://git.kernel.org/bpf/bpf-next/c/8e50750f122e

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html

Re: [PATCH bpf-next] libbpf: set close-on-exec flag on gzopen

[Daniel Borkmann]

On 8/10/23 11:43 PM, Martin Kelly wrote:
> From: Marco Vedovati <marco.vedovati@crowdstrike.com>
> 
> Enable the close-on-exec flag when using gzopen
> 
> This is especially important for multithreaded programs making use of
> libbpf, where a fork + exec could race with libbpf library calls,
> potentially resulting in a file descriptor leaked to the new process.
> 
> Signed-off-by: Marco Vedovati <marco.vedovati@crowdstrike.com>

Looks good, thanks for the fix, applied! Do we also need to convert the
fmemopen in bpf_object__read_kconfig_mem - if yes, could you also send a
patch?

Thanks,
Daniel

Re: [PATCH bpf-next] libbpf: set close-on-exec flag on gzopen

[Stanislav Fomichev]

On 08/10, Martin Kelly wrote:
> On 8/10/23 14:43, Martin Kelly wrote:
> > From: Marco Vedovati <marco.vedovati@crowdstrike.com>
> > 
> > Enable the close-on-exec flag when using gzopen
> > 
> > This is especially important for multithreaded programs making use of
> > libbpf, where a fork + exec could race with libbpf library calls,
> > potentially resulting in a file descriptor leaked to the new process.
> > 
> > Signed-off-by: Marco Vedovati <marco.vedovati@crowdstrike.com>
> > ---
> >   tools/lib/bpf/libbpf.c | 4 ++--
> >   1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> > index 17883f5a44b9..b14a4376a86e 100644
> > --- a/tools/lib/bpf/libbpf.c
> > +++ b/tools/lib/bpf/libbpf.c
> > @@ -1978,9 +1978,9 @@ static int bpf_object__read_kconfig_file(struct bpf_object *obj, void *data)
> >   		return -ENAMETOOLONG;
> >   	/* gzopen also accepts uncompressed files. */
> > -	file = gzopen(buf, "r");
> > +	file = gzopen(buf, "re");
> >   	if (!file)
> > -		file = gzopen("/proc/config.gz", "r");
> > +		file = gzopen("/proc/config.gz", "re");
> >   	if (!file) {
> >   		pr_warn("failed to open system Kconfig\n");
> 
> Sorry for double-sending the patch; the first was missing the bpf-next
> prefix and I wasn't sure if that would be an issue. Feel free to ignore this
> patch, as the other already got a reply.

Oops, I missed your resend:

https://lore.kernel.org/bpf/ZNVf6kHamI9awatB@google.com/

Next time pls at least reply to the first 'wrong' one :-)

Re: [External] Re: [PATCH bpf-next] libbpf: set close-on-exec flag on gzopen

[Martin Kelly]

On 8/14/23 10:03, Stanislav Fomichev wrote:
> On 08/10, Martin Kelly wrote:
>> On 8/10/23 14:43, Martin Kelly wrote:
>>> From: Marco Vedovati <marco.vedovati@crowdstrike.com>
>>>
>>> Enable the close-on-exec flag when using gzopen
>>>
>>> This is especially important for multithreaded programs making use of
>>> libbpf, where a fork + exec could race with libbpf library calls,
>>> potentially resulting in a file descriptor leaked to the new process.
>>>
>>> Signed-off-by: Marco Vedovati <marco.vedovati@crowdstrike.com>
>>> ---
>>>    tools/lib/bpf/libbpf.c | 4 ++--
>>>    1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
>>> index 17883f5a44b9..b14a4376a86e 100644
>>> --- a/tools/lib/bpf/libbpf.c
>>> +++ b/tools/lib/bpf/libbpf.c
>>> @@ -1978,9 +1978,9 @@ static int bpf_object__read_kconfig_file(struct bpf_object *obj, void *data)
>>>    		return -ENAMETOOLONG;
>>>    	/* gzopen also accepts uncompressed files. */
>>> -	file = gzopen(buf, "r");
>>> +	file = gzopen(buf, "re");
>>>    	if (!file)
>>> -		file = gzopen("/proc/config.gz", "r");
>>> +		file = gzopen("/proc/config.gz", "re");
>>>    	if (!file) {
>>>    		pr_warn("failed to open system Kconfig\n");
>> Sorry for double-sending the patch; the first was missing the bpf-next
>> prefix and I wasn't sure if that would be an issue. Feel free to ignore this
>> patch, as the other already got a reply.
>
> Next time pls at least reply to the first 'wrong' one :-)

Yeah, I replied to this one as you had already replied to the first, and 
I didn't want to add confusion and split the conversation into two 
pieces. I saw a 40 minute or so delay before the initial patch was 
posted to vger, and I had thought "maybe it's because I was missing the 
bpf-next prefix" and then resent... only to have the initial message 
appear a few minutes later :). So, my bad, I jumped the gun.

Next time I'll make sure to add the right prefix initially and to take 
into account mailing list delay.

Re: Re: [PATCH bpf-next] libbpf: set close-on-exec flag on gzopen

[Martin Kelly]

On 8/14/23 08:40, Daniel Borkmann wrote:
> On 8/10/23 11:43 PM, Martin Kelly wrote:
>> From: Marco Vedovati <marco.vedovati@crowdstrike.com>
>>
>> Enable the close-on-exec flag when using gzopen
>>
>> This is especially important for multithreaded programs making use of
>> libbpf, where a fork + exec could race with libbpf library calls,
>> potentially resulting in a file descriptor leaked to the new process.
>>
>> Signed-off-by: Marco Vedovati <marco.vedovati@crowdstrike.com>
>
> Looks good, thanks for the fix, applied! Do we also need to convert the
> fmemopen in bpf_object__read_kconfig_mem - if yes, could you also send a
> patch?
>
> Thanks,
> Daniel

Good thinking, but it looks like (from libc sources and a quick test) 
the underlying fmemopen doesn't accept the "e" mode for close-on-exec. 
On glibc 2.35 at least, it's calling into _IO_fopencookie, which then 
executes this switch and rejects the flag.