| Message ID | 20240207200823.7229-1-maks.mishinFZ@gmail.com (mailing list archive) |
|---|---|
| State | Rejected |
| Delegated to: | Stephen Hemminger |
| Headers | show |
| Series | genl: Fix descriptor leak in get_genl_kind() | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Not a local patch |
Maks Mishin <maks.mishinfz@gmail.com> writes: > Signed-off-by: Maks Mishin <maks.mishinFZ@gmail.com> The subject should say [PATCH iproute2] or [PATCH iproute2-next] since it targets that project. > --- > genl/genl.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/genl/genl.c b/genl/genl.c > index 85cc73bb..74100dad 100644 > --- a/genl/genl.c > +++ b/genl/genl.c > @@ -71,6 +71,9 @@ static struct genl_util *get_genl_kind(const char *str) > snprintf(buf, sizeof(buf), "%s_genl_util", str); > > f = dlsym(dlh, buf); > + if (dlh != NULL) > + dlclose(dlh); This is broken. If the earlier dlopen() actually loaded a .so then this dlclose() will close it again, before f gets used. When f gets dereferenced later, the program will crash. If this works at all, it is because dlopen(NULL, ...) returns a handle to the main program, so dlclose() doesn't unload it. My assumption is that the author is leaving resources to be released at program exit. It is a short-lived command line utility after all. > + > if (f == NULL) > goto noexist; > reg:
diff --git a/genl/genl.c b/genl/genl.c index 85cc73bb..74100dad 100644 --- a/genl/genl.c +++ b/genl/genl.c @@ -71,6 +71,9 @@ static struct genl_util *get_genl_kind(const char *str) snprintf(buf, sizeof(buf), "%s_genl_util", str); f = dlsym(dlh, buf); + if (dlh != NULL) + dlclose(dlh); + if (f == NULL) goto noexist; reg:
Signed-off-by: Maks Mishin <maks.mishinFZ@gmail.com> --- genl/genl.c | 3 +++ 1 file changed, 3 insertions(+)