user(8) free() before returning in groupmod()
>From NetBSD: Coverity annotation -- although memsave free()s its first argument, it will allocate memory and assign it to its first argument, so it is neutral Coverity CID 3228: memory leak -- failed to free() newname in groupmod() Index: src/usr.sbin/user/user.c === RCS file: /cvs/src/usr.sbin/user/user.c,v retrieving revision 1.98 diff -u -p -r1.98 user.c --- src/usr.sbin/user/user.c23 Nov 2013 17:14:05 - 1.98 +++ src/usr.sbin/user/user.c30 Dec 2013 15:58:48 - @@ -2230,7 +2230,8 @@ groupmod(int argc, char **argv) cc = strlcat(buf, "\n", sizeof(buf)); if (cc >= sizeof(buf)) errx(EXIT_FAILURE, "group `%s' entry too long", grp->gr_name); - + if (newname != NULL) + free(newname); openlog("groupmod", LOG_PID, LOG_USER); if (!modify_gid(*argv, buf)) err(EXIT_FAILURE, "can't change %s file", _PATH_GROUP);
Re: user(8) free() before returning in groupmod()
On Mon, Dec 30, 2013 at 08:23, Loganaden Velvindron wrote: > Index: src/usr.sbin/user/user.c > === > RCS file: /cvs/src/usr.sbin/user/user.c,v > retrieving revision 1.98 > diff -u -p -r1.98 user.c > --- src/usr.sbin/user/user.c 23 Nov 2013 17:14:05 - 1.98 > +++ src/usr.sbin/user/user.c 30 Dec 2013 15:58:48 - > @@ -2230,7 +2230,8 @@ groupmod(int argc, char **argv) > cc = strlcat(buf, "\n", sizeof(buf)); > if (cc >= sizeof(buf)) > errx(EXIT_FAILURE, "group `%s' entry too long", grp->gr_name); > - > + if (newname != NULL) > + free(newname); > openlog("groupmod", LOG_PID, LOG_USER); > if (!modify_gid(*argv, buf)) > err(EXIT_FAILURE, "can't change %s file", _PATH_GROUP); This is also poor style. There's no need to check for NULL.