On 03/28/11 06:14 AM, Tiago Vignatti wrote: > Signed-off-by: Tiago Vignatti <[email protected]> > --- > hw/xfree86/common/xf86Init.c | 1 + > 1 files changed, 1 insertions(+), 0 deletions(-) > > diff --git a/hw/xfree86/common/xf86Init.c b/hw/xfree86/common/xf86Init.c > index e664ce4..d81303e 100644 > --- a/hw/xfree86/common/xf86Init.c > +++ b/hw/xfree86/common/xf86Init.c > @@ -1433,6 +1433,7 @@ xf86LoadModules(char **list, pointer *optlist) > } > free(name); > } > + free(name); > return !failed; > } >
NAK - this is still obviously a double-free, which is a security vulnerability on some platforms ( http://cwe.mitre.org/data/definitions/415.html ), nor does it solve the leak in all situations. The leak is due to this code: /* Skip empty names */ if (name == NULL || *name == '\0') continue; but since that's continue instead of break, it jumps to the next iteration of the loop, not out of the loop, so if that happens in the middle of the list, the name variable will still be overwritten without being freed. A better fix would seem to be: /* Skip empty names */ if (name == NULL || *name == '\0') { free(name); continue; } (since free(NULL) is guaranteed safe/no-op by ANSI C89 & later), or if you want to be really picky: /* Skip empty names */ if (name == NULL) continue; if (*name == '\0') { free(name); continue; } -- -Alan Coopersmith- [email protected] Oracle Solaris Platform Engineering: X Window System _______________________________________________ [email protected]: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
