src/groupmod.c: delete gr_free_members(&grp) to avoid double free with -U

[gOo0se]

Originally PR #1007, discuss with @alejandro-colomar, we found groupmod -U may cause crashes because of double free. If without -a, the first free is in gr_free_members(&grp), and then in gr_update without -n or gr_remove with -n.

Considering the minimal impact of modifications on existing code, delete gr_free_members(&grp) to avoid double free.

Although this may seem reckless, the second free in two different positions will definitely be triggered, and the following two test cases can be used to illustrate the situation :

[root@localhost src]# ./useradd u1
[root@localhost src]# ./useradd u2
[root@localhost src]# ./useradd u3
[root@localhost src]# ./groupadd -U u1,u2,u3 g1
[root@localhost src]# ./groupmod -n g2 -U u1,u2 g1
Segmentation fault

This case would free (*ogrp).gr_mem in gr_free_members(&grp) due to assignment statements grp = *ogrp, then in if (nflg && (gr_remove (group_name) == 0)), which finally calls gr_free_members(grent) to free (*ogrp).gr_mem again.

[root@localhost src]# ./useradd u1
[root@localhost src]# ./useradd u2
[root@localhost src]# ./useradd u3
[root@localhost src]# ./groupadd -U u1,u2,u3 g1
[root@localhost src]# ./groupmod -U u1,u2 g1
Segmentation fault

The other case would free (*ogrp).gr_mem in gr_free_members(&grp) too, then in if (gr_update (&grp) == 0), which finally calls gr_free_members(grent) too to free (*ogrp).gr_mem again.

So the first free is unnecessary, maybe we can drop it.

All test cases passed.

[alejandro-colomar]

I would update the other PR instead of creating a new one. That way, we have less places where the discussion is spread. Does it sound good to you?

[gOo0se]

I would update the other PR instead of creating a new one. That way, we have less places where the discussion is spread. Does it sound good to you?

Yeah, I agree with you.