Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Only free sgent if it was initialized #417

Merged
merged 1 commit into from
Sep 27, 2021
Merged

Conversation

jubalh
Copy link
Member

@jubalh jubalh commented Sep 20, 2021

sgent is only initialized in get_group() if is_shadowgrp is true.
So we should also only attempt to free it if this is actually the case.

Can otherwise lead to:

free() double free detected in tcache 2 (gpasswd)

`sgent` is only initialized in `get_group()` if `is_shadowgrp` is true.
So we should also only attempt to free it if this is actually the case.

Can otherwise lead to:
```
free() double free detected in tcache 2 (gpasswd)
```
bmwiedemann pushed a commit to bmwiedemann/openSUSE that referenced this pull request Sep 22, 2021
https://build.opensuse.org/request/show/920286
by user jubalh + dimstar_suse
- bsc#1190146: Fix empty subid range
  Add shadow-4.9-useradd-subuid.patch
  shadow-maint/shadow#399

- bsc#1190145: Fix double free in gpasswd:
  Add shadow-4.9-sgent-free.patch upstreamed as
  shadow-maint/shadow#417

- Fix shadow-login_defs-check.sh:
  In the last update we switched from calling make to %make_build
  macro. Using sed to adapt the spec file now.

- libsubid-devel: add missing requires for libsubid3
- Remove README.changes-pwdutils, all distros you can upgrade from
  use already shadow

- login.defs: Enable USERGROUPS_ENAB and CREATE_HOME to
  be compatible with other Linux distros and the other tools
  creating user accounts in use on openSUSE. Set HOME_MODE to 700
  for security reasons and compatibility. [bsc#1189139] [bsc
@hallyn
Copy link
Member

hallyn commented Sep 27, 2021

Thanks!

I shouldn't have taken several of those patches. Freeing memory right
before exit just isn't worth it.

Would the result be more future-proof if we just memset sgent to 0 at
the start? (Or move it to BSS)

@jubalh
Copy link
Member Author

jubalh commented Sep 27, 2021

Would the result be more future-proof if we just memset sgent to 0 at
the start?

Most likely would be. I also thought about doing this for this patch, but saw that it has been done this way on some other locations and adhered to it.

@hallyn
Copy link
Member

hallyn commented Sep 27, 2021

Ok, will merge as is - thanks again.

@hallyn hallyn merged commit 5bb28a0 into shadow-maint:master Sep 27, 2021
@jubalh jubalh deleted the doublefree branch September 27, 2021 18:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants