|Summary:||Add support for a list of extra groups for admin users|
|Product:||accountsservice||Reporter:||Cosimo Cecchi <cosimoc>|
|Component:||general||Assignee:||Matthias Clasen <mclasen>|
|Status:||RESOLVED FIXED||QA Contact:|
|Priority:||medium||CC:||bugseforuns, cosimoc, marius.vollmer, oholy, rstrode, stefw|
|i915 platform:||i915 features:|
Add support for a list of extra groups for admin users
Fix useradd failures caused by invalid read
Description Cosimo Cecchi 2017-07-30 13:50:15 UTC
This patch upstreams a functionality that Debian/Ubuntu carry downstream patches for.
Comment 1 Cosimo Cecchi 2017-07-30 13:50:17 UTC
Created attachment 133135 [details] [review] Add support for a list of extra groups for admin users It's common for distributions to want to add administrator users to additional groups (e.g. lpadmin or systemd-journal); Debian and Ubuntu have patches that add this kind of functionality already. This commit adds a configure option to specify a comma-separated list of extra groups for admin users and adds support for it when both adding a new admin user and promoting an user to admin.
Comment 2 Robert Ancell 2017-12-19 02:28:47 UTC
Comment on attachment 133135 [details] [review] Add support for a list of extra groups for admin users Review of attachment 133135 [details] [review]: ----------------------------------------------------------------- LGTM
Comment 3 Cosimo Cecchi 2017-12-19 02:55:38 UTC
Thanks for the review, Robert! Is there any chance you can push this for me? I don't think I have write access to this repo.
Comment 4 Robert Ancell 2017-12-19 20:33:52 UTC
Pushed, I just wanted to check with Ray he was OK with this change first. Thanks for the patch!
Comment 5 Ondrej Holy 2018-05-03 07:42:56 UTC
Created attachment 139293 [details] [review] Fix useradd failures caused by invalid read Commit 93e2a85 added support for extra admin groups, but causes useradd failures, when adding admin accounts. It is because admin_groups are released before they are actually used, which causes that garbage is used instead of the group and useradd fails consequently with invalid group name. Do not release the memory before it is used and also fix one corresponding memory leak.
Comment 6 Ondrej Holy 2018-05-03 07:43:18 UTC
Comment 7 Cosimo Cecchi 2018-05-03 15:01:55 UTC
Whoops, good catch... the patch looks good to me.
Comment 8 Felipe 2018-05-08 23:17:20 UTC
Hello, when will this error be fixed?
Comment 9 Ray Strode [halfline] 2018-05-10 14:23:39 UTC
hey i missed this go by in my mail and did two similar fixes yesterday: https://cgit.freedesktop.org/accountsservice/commit/?h=wip/fix-useradd&id=25437b69da82bd1dccc220f8dff516006a99471a https://cgit.freedesktop.org/accountsservice/commit/?h=wip/fix-useradd&id=c17b57e1ffc3d6086cb4bbe17ff40a091662d1be (but didn't push them to master yet) Anyway, seems good to me, please push !
Comment 10 Ray Strode [halfline] 2018-05-10 14:37:22 UTC
(downstream bug https://bugzilla.redhat.com/show_bug.cgi?id=1575780 )
Comment 11 Ray Strode [halfline] 2018-05-10 17:04:48 UTC
9d14729..1557846 master -> master