mbox series

[00/11] Fix fgetsgent_r data corruption bug (20338)

Message ID cover.1594974444.git.fweimer@redhat.com
Headers
Series Fix fgetsgent_r data corruption bug (20338) |

Message

Florian Weimer July 17, 2020, 8:29 a.m. UTC 
  We have recently seen an uptick in reports of this bug.

The __libc_readline_unlocked removal and the nss_compat mmap fix for bug
26258 could potentially be dropped from this series.

Tested on i686-linux-gnu.  I verified that the test case crashes with
glibc 2.31 on both i686 and x86-64, and not just results in failures
under valgrind.

Thanks,
Florian

Florian Weimer (11):
  nss_files: Consolidate file opening in __nss_files_fopen
  nss_compat: Do not use mmap to read database files (bug 26258)
  nss_files: Consolidate line parse declarations in <nss_files.h>
  nss_files: Use generic result pointer in parse_line
  libio: Add fseterr_unlocked for internal use
  nss: Add __nss_fgetent_r
  grp: Implement fgetgrent_r using __nss_fgetent_r
  gshadow: Implement fgetsgent_r using __nss_fgetent_r (bug 20338)
  pwd: Implement fgetpwent_r using __nss_fgetent_r
  shadow: Implement fgetspent_r using __nss_fgetent_r
  libio: Remove __libc_readline_unlocked

 grp/fgetgrent_r.c                  |  54 +------
 gshadow/Makefile                   |   2 +-
 gshadow/fgetsgent_r.c              |  41 +----
 gshadow/tst-fgetsgent_r.c          | 192 +++++++++++++++++++++++
 include/grp.h                      |   6 -
 include/gshadow.h                  |   6 -
 include/netdb.h                    |  13 --
 include/netinet/ether.h            |   6 -
 include/nss_files.h                |  84 ++++++++++
 include/pwd.h                      |   6 -
 include/rpc/netdb.h                |   6 -
 include/shadow.h                   |   6 -
 include/stdio.h                    |  20 +--
 libio/Makefile                     |   4 +-
 libio/Versions                     |   1 -
 libio/readline.c                   | 170 ---------------------
 libio/tst-readline.c               | 237 -----------------------------
 nss/Makefile                       |   4 +-
 nss/Versions                       |   1 +
 nss/nss_compat/compat-grp.c        |   6 +-
 nss/nss_compat/compat-initgroups.c |   6 +-
 nss/nss_compat/compat-pwd.c        |   6 +-
 nss/nss_compat/compat-spwd.c       |   6 +-
 nss/nss_fgetent_r.c                |  55 +++++++
 nss/nss_files/files-XXX.c          |  82 ++++------
 nss/nss_files/files-alias.c        |   5 +-
 nss/nss_files/files-initgroups.c   |   6 +-
 nss/nss_files/files-netgrp.c       |   5 +-
 nss/nss_files/files-parse.c        |   6 +-
 nss/nss_files_fopen.c              |  47 ++++++
 nss/nss_parse_line_result.c        |  46 ++++++
 nss/nss_readline.c                 |  99 ++++++++++++
 pwd/fgetpwent_r.c                  |  43 +-----
 shadow/fgetspent_r.c               |  43 +-----
 34 files changed, 609 insertions(+), 711 deletions(-)
 create mode 100644 gshadow/tst-fgetsgent_r.c
 create mode 100644 include/nss_files.h
 delete mode 100644 libio/readline.c
 delete mode 100644 libio/tst-readline.c
 create mode 100644 nss/nss_fgetent_r.c
 create mode 100644 nss/nss_files_fopen.c
 create mode 100644 nss/nss_parse_line_result.c
 create mode 100644 nss/nss_readline.c

Comments

Carlos O'Donell July 21, 2020, 3:27 a.m. UTC | #1 
On 7/17/20 4:29 AM, Florian Weimer via Libc-alpha wrote:
> We have recently seen an uptick in reports of this bug.
> 
> The __libc_readline_unlocked removal and the nss_compat mmap fix for bug
> 26258 could potentially be dropped from this series.
> 
> Tested on i686-linux-gnu.  I verified that the test case crashes with
> glibc 2.31 on both i686 and x86-64, and not just results in failures
> under valgrind.

OK for 2.32.

Thanks for undertaking the refactor and the fix. The end result looks
much better than the existing code and uses a lot of the same functionality
but just organized better.

I tested on x86_64 and i686 also and I didn't see any failures.

Thanks for testing under valgrind which is also important.

This series is OK for 2.32 to fix the underlying bug 20338 which has
started to get visibility as it impacts systemd users.
 
> Thanks,
> Florian
> 
> Florian Weimer (11):
>   nss_files: Consolidate file opening in __nss_files_fopen
>   nss_compat: Do not use mmap to read database files (bug 26258)
>   nss_files: Consolidate line parse declarations in <nss_files.h>
>   nss_files: Use generic result pointer in parse_line
>   libio: Add fseterr_unlocked for internal use
>   nss: Add __nss_fgetent_r
>   grp: Implement fgetgrent_r using __nss_fgetent_r
>   gshadow: Implement fgetsgent_r using __nss_fgetent_r (bug 20338)
>   pwd: Implement fgetpwent_r using __nss_fgetent_r
>   shadow: Implement fgetspent_r using __nss_fgetent_r
>   libio: Remove __libc_readline_unlocked
> 
>  grp/fgetgrent_r.c                  |  54 +------
>  gshadow/Makefile                   |   2 +-
>  gshadow/fgetsgent_r.c              |  41 +----
>  gshadow/tst-fgetsgent_r.c          | 192 +++++++++++++++++++++++
>  include/grp.h                      |   6 -
>  include/gshadow.h                  |   6 -
>  include/netdb.h                    |  13 --
>  include/netinet/ether.h            |   6 -
>  include/nss_files.h                |  84 ++++++++++
>  include/pwd.h                      |   6 -
>  include/rpc/netdb.h                |   6 -
>  include/shadow.h                   |   6 -
>  include/stdio.h                    |  20 +--
>  libio/Makefile                     |   4 +-
>  libio/Versions                     |   1 -
>  libio/readline.c                   | 170 ---------------------
>  libio/tst-readline.c               | 237 -----------------------------
>  nss/Makefile                       |   4 +-
>  nss/Versions                       |   1 +
>  nss/nss_compat/compat-grp.c        |   6 +-
>  nss/nss_compat/compat-initgroups.c |   6 +-
>  nss/nss_compat/compat-pwd.c        |   6 +-
>  nss/nss_compat/compat-spwd.c       |   6 +-
>  nss/nss_fgetent_r.c                |  55 +++++++
>  nss/nss_files/files-XXX.c          |  82 ++++------
>  nss/nss_files/files-alias.c        |   5 +-
>  nss/nss_files/files-initgroups.c   |   6 +-
>  nss/nss_files/files-netgrp.c       |   5 +-
>  nss/nss_files/files-parse.c        |   6 +-
>  nss/nss_files_fopen.c              |  47 ++++++
>  nss/nss_parse_line_result.c        |  46 ++++++
>  nss/nss_readline.c                 |  99 ++++++++++++
>  pwd/fgetpwent_r.c                  |  43 +-----
>  shadow/fgetspent_r.c               |  43 +-----
>  34 files changed, 609 insertions(+), 711 deletions(-)
>  create mode 100644 gshadow/tst-fgetsgent_r.c
>  create mode 100644 include/nss_files.h
>  delete mode 100644 libio/readline.c
>  delete mode 100644 libio/tst-readline.c
>  create mode 100644 nss/nss_fgetent_r.c
>  create mode 100644 nss/nss_files_fopen.c
>  create mode 100644 nss/nss_parse_line_result.c
>  create mode 100644 nss/nss_readline.c
>