On 08/02/2013 11:22 AM, Daniel J Walsh wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> THis patch fixes all of Eric's and Daniels comments.
>
> [PATCH] virt-login-shell joins users into lxc container.
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.4.14 (GNU/Linux)
> Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/
>
> iEYEARECAAYFAlH7zp8ACgkQrlYvE4MpobNx3gCbBtxw7T4fzIfHSyEEKKyjojXR
> BUUAoOToptiTOi+RC6Bdcp+zvg/xzfRh
> =7zpw
> -----END PGP SIGNATURE-----
>
>
>
The overnight run of Coverity found two issues with this module. I've
cut-n-pasted the relevant portions of the error/traces:
#1: DEADCODE: virLoginShellAllowedUser()
86 if (pp->str[0] == '%') {
(1) Event assignment: Assigning: "ptr" = "pp->str + 1".
Also see events: [notnull][dead_error_condition][dead_error_line]
87 ptr = &pp->str[1];
(2) Event notnull: At condition "ptr", the value of "ptr" cannot be NULL.
(3) Event dead_error_condition: The condition "!ptr" cannot be true.
Also see events: [assignment][dead_error_line]
88 if (!ptr)
(4) Event dead_error_line: Execution cannot reach this statement
"continue;".
Also see events: [assignment][notnull][dead_error_condition]
89 continue;
Did you perhaps mean (!*ptr) (eg, the contents not the value?)
#2: USE_AFTER_FREE: main()
252 if ((ngroups = virGetGroupList(uid, gid, &groups)) < 0)
253 goto cleanup;
254
(18) Event freed_arg: "virLoginShellAllowedUser(virConfPtr, char const *,
gid_t *)" frees "groups". [details]
(19) Event cond_false: Condition "virLoginShellAllowedUser(conf, name, groups)
< 0", taking false branch
Also see events: [double_free]
255 if (virLoginShellAllowedUser(conf, name, groups) < 0)
256 goto cleanup;
257
Which is true:
...
61
62 static int virLoginShellAllowedUser(virConfPtr conf,
63 const char *name,
64 gid_t *groups)
65 {
...
(6) Event label: Reached label "cleanup"
110 cleanup:
111 VIR_FREE(gname);
(7) Event freed_arg: "virFree(void *)" frees parameter "groups". [details]
112 VIR_FREE(groups);
113 return ret;
114 }
...
Then back in main()
338
(24) Event label: Reached label "cleanup"
339 cleanup:
340 virConfFree(conf);
341 virLoginShellFini(conn, dom);
342 virStringFreeList(shargv);
343 VIR_FREE(name);
344 VIR_FREE(homedir);
345 VIR_FREE(seclabel);
346 VIR_FREE(secmodel);
(25) Event double_free: Calling "virFree(void *)" frees pointer
"groups" which has already been freed. [details]
Also see events: [freed_arg]
347 VIR_FREE(groups);
Not quite sure how you want to approach fixing the second error. There are
two VIR_FREE(groups) in virLoginShellAllowedUser() and both would need to
be handled properly. Additionally code in main() seems to perhaps use
groups() after the call - so freeing it could have other consequences too.
John
--
libvir-list mailing list
[email protected]
https://www.redhat.com/mailman/listinfo/libvir-list
