* [PATCH bpf] tools: bpftool: close prog FD before exit on showing a single program
@ 2019-08-15 14:22 Quentin Monnet
2019-08-15 17:09 ` Andrii Nakryiko
2019-08-16 5:11 ` Alexei Starovoitov
0 siblings, 2 replies; 7+ messages in thread
From: Quentin Monnet @ 2019-08-15 14:22 UTC (permalink / raw)
To: Alexei Starovoitov, Daniel Borkmann
Cc: bpf, netdev, oss-drivers, Quentin Monnet
When showing metadata about a single program by invoking
"bpftool prog show PROG", the file descriptor referring to the program
is not closed before returning from the function. Let's close it.
Fixes: 71bb428fe2c1 ("tools: bpf: add bpftool")
Signed-off-by: Quentin Monnet <quentin.monnet@netronome.com>
Reviewed-by: Jakub Kicinski <jakub.kicinski@netronome.com>
---
tools/bpf/bpftool/prog.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/tools/bpf/bpftool/prog.c b/tools/bpf/bpftool/prog.c
index 66f04a4846a5..43fdbbfe41bb 100644
--- a/tools/bpf/bpftool/prog.c
+++ b/tools/bpf/bpftool/prog.c
@@ -363,7 +363,9 @@ static int do_show(int argc, char **argv)
if (fd < 0)
return -1;
- return show_prog(fd);
+ err = show_prog(fd);
+ close(fd);
+ return err;
}
if (argc)
--
2.17.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH bpf] tools: bpftool: close prog FD before exit on showing a single program
2019-08-15 14:22 [PATCH bpf] tools: bpftool: close prog FD before exit on showing a single program Quentin Monnet
@ 2019-08-15 17:09 ` Andrii Nakryiko
2019-08-15 17:30 ` Jakub Kicinski
2019-08-16 5:11 ` Alexei Starovoitov
1 sibling, 1 reply; 7+ messages in thread
From: Andrii Nakryiko @ 2019-08-15 17:09 UTC (permalink / raw)
To: Quentin Monnet
Cc: Alexei Starovoitov, Daniel Borkmann, bpf, Networking, oss-drivers
On Thu, Aug 15, 2019 at 7:24 AM Quentin Monnet
<quentin.monnet@netronome.com> wrote:
>
> When showing metadata about a single program by invoking
> "bpftool prog show PROG", the file descriptor referring to the program
> is not closed before returning from the function. Let's close it.
>
> Fixes: 71bb428fe2c1 ("tools: bpf: add bpftool")
> Signed-off-by: Quentin Monnet <quentin.monnet@netronome.com>
> Reviewed-by: Jakub Kicinski <jakub.kicinski@netronome.com>
> ---
> tools/bpf/bpftool/prog.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/tools/bpf/bpftool/prog.c b/tools/bpf/bpftool/prog.c
> index 66f04a4846a5..43fdbbfe41bb 100644
> --- a/tools/bpf/bpftool/prog.c
> +++ b/tools/bpf/bpftool/prog.c
> @@ -363,7 +363,9 @@ static int do_show(int argc, char **argv)
> if (fd < 0)
> return -1;
>
> - return show_prog(fd);
> + err = show_prog(fd);
> + close(fd);
> + return err;
There is a similar problem few lines above for special case of argc ==
2, which you didn't fix.
Would it be better to make show_prog(fd) close provided fd instead or
is it used in some other context where FD should live longer (I
haven't checked, sorry)?
> }
>
> if (argc)
> --
> 2.17.1
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH bpf] tools: bpftool: close prog FD before exit on showing a single program
2019-08-15 17:09 ` Andrii Nakryiko
@ 2019-08-15 17:30 ` Jakub Kicinski
2019-08-15 18:05 ` Andrii Nakryiko
0 siblings, 1 reply; 7+ messages in thread
From: Jakub Kicinski @ 2019-08-15 17:30 UTC (permalink / raw)
To: Andrii Nakryiko
Cc: Quentin Monnet, Alexei Starovoitov, Daniel Borkmann, bpf,
Networking, oss-drivers
On Thu, 15 Aug 2019 10:09:38 -0700, Andrii Nakryiko wrote:
> On Thu, Aug 15, 2019 at 7:24 AM Quentin Monnet wrote:
> > When showing metadata about a single program by invoking
> > "bpftool prog show PROG", the file descriptor referring to the program
> > is not closed before returning from the function. Let's close it.
> >
> > Fixes: 71bb428fe2c1 ("tools: bpf: add bpftool")
> > Signed-off-by: Quentin Monnet <quentin.monnet@netronome.com>
> > Reviewed-by: Jakub Kicinski <jakub.kicinski@netronome.com>
> > ---
> > tools/bpf/bpftool/prog.c | 4 +++-
> > 1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/tools/bpf/bpftool/prog.c b/tools/bpf/bpftool/prog.c
> > index 66f04a4846a5..43fdbbfe41bb 100644
> > --- a/tools/bpf/bpftool/prog.c
> > +++ b/tools/bpf/bpftool/prog.c
> > @@ -363,7 +363,9 @@ static int do_show(int argc, char **argv)
> > if (fd < 0)
> > return -1;
> >
> > - return show_prog(fd);
> > + err = show_prog(fd);
> > + close(fd);
> > + return err;
>
> There is a similar problem few lines above for special case of argc ==
> 2, which you didn't fix.
This is the special argc == 2 case.
> Would it be better to make show_prog(fd) close provided fd instead or
> is it used in some other context where FD should live longer (I
> haven't checked, sorry)?
I think it used to close that's how the bug crept in. Other than the bug
it's fine the way it is.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH bpf] tools: bpftool: close prog FD before exit on showing a single program
2019-08-15 17:30 ` Jakub Kicinski
@ 2019-08-15 18:05 ` Andrii Nakryiko
2019-08-15 18:09 ` Jakub Kicinski
0 siblings, 1 reply; 7+ messages in thread
From: Andrii Nakryiko @ 2019-08-15 18:05 UTC (permalink / raw)
To: Jakub Kicinski
Cc: Quentin Monnet, Alexei Starovoitov, Daniel Borkmann, bpf,
Networking, oss-drivers
On Thu, Aug 15, 2019 at 10:30 AM Jakub Kicinski
<jakub.kicinski@netronome.com> wrote:
>
> On Thu, 15 Aug 2019 10:09:38 -0700, Andrii Nakryiko wrote:
> > On Thu, Aug 15, 2019 at 7:24 AM Quentin Monnet wrote:
> > > When showing metadata about a single program by invoking
> > > "bpftool prog show PROG", the file descriptor referring to the program
> > > is not closed before returning from the function. Let's close it.
> > >
> > > Fixes: 71bb428fe2c1 ("tools: bpf: add bpftool")
> > > Signed-off-by: Quentin Monnet <quentin.monnet@netronome.com>
> > > Reviewed-by: Jakub Kicinski <jakub.kicinski@netronome.com>
> > > ---
> > > tools/bpf/bpftool/prog.c | 4 +++-
> > > 1 file changed, 3 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/tools/bpf/bpftool/prog.c b/tools/bpf/bpftool/prog.c
> > > index 66f04a4846a5..43fdbbfe41bb 100644
> > > --- a/tools/bpf/bpftool/prog.c
> > > +++ b/tools/bpf/bpftool/prog.c
> > > @@ -363,7 +363,9 @@ static int do_show(int argc, char **argv)
> > > if (fd < 0)
> > > return -1;
> > >
> > > - return show_prog(fd);
> > > + err = show_prog(fd);
> > > + close(fd);
> > > + return err;
> >
> > There is a similar problem few lines above for special case of argc ==
> > 2, which you didn't fix.
>
> This is the special argc == 2 case.
Yep, you are right, the other one already does this.
>
> > Would it be better to make show_prog(fd) close provided fd instead or
> > is it used in some other context where FD should live longer (I
> > haven't checked, sorry)?
>
> I think it used to close that's how the bug crept in. Other than the bug
> it's fine the way it is.
So are you saying that show_prog() should or should not close FD?
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH bpf] tools: bpftool: close prog FD before exit on showing a single program
2019-08-15 18:05 ` Andrii Nakryiko
@ 2019-08-15 18:09 ` Jakub Kicinski
2019-08-15 20:16 ` Andrii Nakryiko
0 siblings, 1 reply; 7+ messages in thread
From: Jakub Kicinski @ 2019-08-15 18:09 UTC (permalink / raw)
To: Andrii Nakryiko
Cc: Quentin Monnet, Alexei Starovoitov, Daniel Borkmann, bpf,
Networking, oss-drivers
On Thu, 15 Aug 2019 11:05:16 -0700, Andrii Nakryiko wrote:
> > > Would it be better to make show_prog(fd) close provided fd instead or
> > > is it used in some other context where FD should live longer (I
> > > haven't checked, sorry)?
> >
> > I think it used to close that's how the bug crept in. Other than the bug
> > it's fine the way it is.
>
> So are you saying that show_prog() should or should not close FD?
Yup, it we'd have to rename it to indicate it closes the fd, and it's
only called in two places. Not worth the churn.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH bpf] tools: bpftool: close prog FD before exit on showing a single program
2019-08-15 18:09 ` Jakub Kicinski
@ 2019-08-15 20:16 ` Andrii Nakryiko
0 siblings, 0 replies; 7+ messages in thread
From: Andrii Nakryiko @ 2019-08-15 20:16 UTC (permalink / raw)
To: Jakub Kicinski
Cc: Quentin Monnet, Alexei Starovoitov, Daniel Borkmann, bpf,
Networking, oss-drivers
On Thu, Aug 15, 2019 at 11:09 AM Jakub Kicinski
<jakub.kicinski@netronome.com> wrote:
>
> On Thu, 15 Aug 2019 11:05:16 -0700, Andrii Nakryiko wrote:
> > > > Would it be better to make show_prog(fd) close provided fd instead or
> > > > is it used in some other context where FD should live longer (I
> > > > haven't checked, sorry)?
> > >
> > > I think it used to close that's how the bug crept in. Other than the bug
> > > it's fine the way it is.
> >
> > So are you saying that show_prog() should or should not close FD?
>
> Yup, it we'd have to rename it to indicate it closes the fd, and it's
> only called in two places. Not worth the churn.
OK, I'm fine with that.
Acked-by: Andrii Nakryiko <andriin@fb.com>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH bpf] tools: bpftool: close prog FD before exit on showing a single program
2019-08-15 14:22 [PATCH bpf] tools: bpftool: close prog FD before exit on showing a single program Quentin Monnet
2019-08-15 17:09 ` Andrii Nakryiko
@ 2019-08-16 5:11 ` Alexei Starovoitov
1 sibling, 0 replies; 7+ messages in thread
From: Alexei Starovoitov @ 2019-08-16 5:11 UTC (permalink / raw)
To: Quentin Monnet
Cc: Alexei Starovoitov, Daniel Borkmann, bpf, Network Development,
oss-drivers
On Thu, Aug 15, 2019 at 7:22 AM Quentin Monnet
<quentin.monnet@netronome.com> wrote:
>
> When showing metadata about a single program by invoking
> "bpftool prog show PROG", the file descriptor referring to the program
> is not closed before returning from the function. Let's close it.
>
> Fixes: 71bb428fe2c1 ("tools: bpf: add bpftool")
> Signed-off-by: Quentin Monnet <quentin.monnet@netronome.com>
> Reviewed-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Applied. Thanks
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2019-08-16 5:11 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-08-15 14:22 [PATCH bpf] tools: bpftool: close prog FD before exit on showing a single program Quentin Monnet
2019-08-15 17:09 ` Andrii Nakryiko
2019-08-15 17:30 ` Jakub Kicinski
2019-08-15 18:05 ` Andrii Nakryiko
2019-08-15 18:09 ` Jakub Kicinski
2019-08-15 20:16 ` Andrii Nakryiko
2019-08-16 5:11 ` Alexei Starovoitov
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).