| Message ID | 20240207202542.9872-1-maks.mishinFZ@gmail.com (mailing list archive) |
|---|---|
| State | Rejected |
| Delegated to: | Stephen Hemminger |
| Headers | show |
| Series | ipaddrlabel: Fix descriptor leak in flush_addrlabel() | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Not a local patch |
Maks Mishin <maks.mishinfz@gmail.com> writes: > Added closure of descriptor `rth2.fd` created by rtnl_open() when > returning from function. > > Signed-off-by: Maks Mishin <maks.mishinFZ@gmail.com> > --- > ip/ipaddrlabel.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/ip/ipaddrlabel.c b/ip/ipaddrlabel.c > index b045827a..3857fcf5 100644 > --- a/ip/ipaddrlabel.c > +++ b/ip/ipaddrlabel.c > @@ -201,8 +201,10 @@ static int flush_addrlabel(struct nlmsghdr *n, void *arg) > n->nlmsg_type = RTM_DELADDRLABEL; > n->nlmsg_flags = NLM_F_REQUEST; > > - if (rtnl_open(&rth2, 0) < 0) > + if (rtnl_open(&rth2, 0) < 0) { > + rtnl_close(&rth2); This change is unnecessary. You're calling rtnl_close() in the case where rtnl_open() just failed so there's nothing to close. > return -1; > + } > > if (rtnl_talk(&rth2, n, NULL) < 0) > return -2;
On Wed, 7 Feb 2024 23:25:42 +0300 Maks Mishin <maks.mishinfz@gmail.com> wrote: > Added closure of descriptor `rth2.fd` created by rtnl_open() when > returning from function. > > Signed-off-by: Maks Mishin <maks.mishinFZ@gmail.com> > --- > ip/ipaddrlabel.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/ip/ipaddrlabel.c b/ip/ipaddrlabel.c > index b045827a..3857fcf5 100644 > --- a/ip/ipaddrlabel.c > +++ b/ip/ipaddrlabel.c > @@ -201,8 +201,10 @@ static int flush_addrlabel(struct nlmsghdr *n, void *arg) > n->nlmsg_type = RTM_DELADDRLABEL; > n->nlmsg_flags = NLM_F_REQUEST; > > - if (rtnl_open(&rth2, 0) < 0) > + if (rtnl_open(&rth2, 0) < 0) { > + rtnl_close(&rth2); > return -1; > + } > > if (rtnl_talk(&rth2, n, NULL) < 0) > return -2; This doesn't make sense, the error path in rtnl_open cleans up itself. Do you have a reproducer or is this just some static analyzer?
diff --git a/ip/ipaddrlabel.c b/ip/ipaddrlabel.c index b045827a..3857fcf5 100644 --- a/ip/ipaddrlabel.c +++ b/ip/ipaddrlabel.c @@ -201,8 +201,10 @@ static int flush_addrlabel(struct nlmsghdr *n, void *arg) n->nlmsg_type = RTM_DELADDRLABEL; n->nlmsg_flags = NLM_F_REQUEST; - if (rtnl_open(&rth2, 0) < 0) + if (rtnl_open(&rth2, 0) < 0) { + rtnl_close(&rth2); return -1; + } if (rtnl_talk(&rth2, n, NULL) < 0) return -2;
Added closure of descriptor `rth2.fd` created by rtnl_open() when returning from function. Signed-off-by: Maks Mishin <maks.mishinFZ@gmail.com> --- ip/ipaddrlabel.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)