[Bug nscd/23178] New: sudo will fail when it is run in concurrent with commands that changes /etc/passwd

classic Classic list List threaded Threaded
7 messages Options
Reply | Threaded
Open this post in threaded view
|

[Bug nscd/23178] New: sudo will fail when it is run in concurrent with commands that changes /etc/passwd

rootedjoy833 at gmail dot com
https://sourceware.org/bugzilla/show_bug.cgi?id=23178

            Bug ID: 23178
           Summary: sudo will fail when it is run in concurrent with
                    commands that changes /etc/passwd
           Product: glibc
           Version: 2.27
            Status: UNCONFIRMED
          Severity: normal
          Priority: P2
         Component: nscd
          Assignee: unassigned at sourceware dot org
          Reporter: huangkaibin at huawei dot com
                CC: drepper.fsp at gmail dot com
  Target Milestone: ---

when nscd service is enabled, sudo will fail with "unknown uid 0: who are you?"
when it is run in concurrent with commands that changes /etc/passwd file(such
usermod or chage).

This problem can be reproduced by the following script.
#-----------------start------------------------
systemctl try-restart nscd.service
(while : ; do ps aux >/dev/null ; done) & #not necessary, but will make this
problem be reproduced easier
(while : ; do touch /etc/passwd ; done) &

while : ; do
        err=$(sudo pwd 2>&1 1>/dev/null)
        if [ $? -eq 1 ]; then
                echo "oops, sudo failed. $err"
        fi
done
#-----------------end--------------------------

This simple script just starts nscd service and does the following things in
concurrent
1. touch /etc/passwd
2. run any sudo command
3. An additional "ps aux" command just to make this problem be reproduced
easier


By looking through source of nscd, I found that this problem is caused by the
using of sendfile syscall. I tried to disable sendfile by replacing sendfileall
function call with writeall in connection.c and pwdcache.c, and this problem
would not happen anymore.

From the manual of sendfile, there is a constraint that the data to be sent
must remain unmodified until it is consumed by the reader.
------------------------------------------------------------------------
If out_fd refers to a socket or pipe with zero-copy support, callers
must ensure the transferred portions of the file referred to by in_fd
remain unmodified until the reader on the other end of out_fd has
consumed the transferred data.
------------------------------------------------------------------------
In the problem, sudo will receive wrong user content, if it has not consumed
the data, but nscd service has already pruned and renewed the user cache.

I don't have any idea to fix this problem if we will use sendfie anyway and
don't think it can be fixed by lock mechanism since it is caused by the
constraint of the sendfile.

--
You are receiving this mail because:
You are on the CC list for the bug.
Reply | Threaded
Open this post in threaded view
|

[Bug nscd/23178] sudo will fail when it is run in concurrent with commands that changes /etc/passwd

rootedjoy833 at gmail dot com
https://sourceware.org/bugzilla/show_bug.cgi?id=23178

Adhemerval Zanella <adhemerval.zanella at linaro dot org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |adhemerval.zanella at linaro dot o
                   |                            |rg

--- Comment #1 from Adhemerval Zanella <adhemerval.zanella at linaro dot org> ---
The explanation sounds reasonable and it seems potentially not only
connection.c and pwdcache.c, but also nscd/aicache.c, nscd/grpcache.c,
nscd/hstcache.c, and nscd/initgrcache.c can potentially hit this sendfile
issue.  The most straightforward fix seems to just use 'writeall' since
sendfile is just an optimization.

--
You are receiving this mail because:
You are on the CC list for the bug.
Reply | Threaded
Open this post in threaded view
|

[Bug nscd/23178] sudo will fail when it is run in concurrent with commands that changes /etc/passwd

rootedjoy833 at gmail dot com
In reply to this post by rootedjoy833 at gmail dot com
https://sourceware.org/bugzilla/show_bug.cgi?id=23178

Florian Weimer <fweimer at redhat dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |fweimer at redhat dot com

--- Comment #2 from Florian Weimer <fweimer at redhat dot com> ---
(In reply to Adhemerval Zanella from comment #1)
> The explanation sounds reasonable and it seems potentially not only
> connection.c and pwdcache.c, but also nscd/aicache.c, nscd/grpcache.c,
> nscd/hstcache.c, and nscd/initgrcache.c can potentially hit this sendfile
> issue.  The most straightforward fix seems to just use 'writeall' since
> sendfile is just an optimization.

We probably should just run unifdef -UHAVE_SENDFILE nscd/*.c and clean up the
result.

--
You are receiving this mail because:
You are on the CC list for the bug.
Reply | Threaded
Open this post in threaded view
|

[Bug nscd/23178] sudo will fail when it is run in concurrent with commands that changes /etc/passwd

rootedjoy833 at gmail dot com
In reply to this post by rootedjoy833 at gmail dot com
https://sourceware.org/bugzilla/show_bug.cgi?id=23178

--- Comment #3 from Adhemerval Zanella <adhemerval.zanella at linaro dot org> ---
(In reply to Florian Weimer from comment #2)
> (In reply to Adhemerval Zanella from comment #1)
> > The explanation sounds reasonable and it seems potentially not only
> > connection.c and pwdcache.c, but also nscd/aicache.c, nscd/grpcache.c,
> > nscd/hstcache.c, and nscd/initgrcache.c can potentially hit this sendfile
> > issue.  The most straightforward fix seems to just use 'writeall' since
> > sendfile is just an optimization.
>
> We probably should just run unifdef -UHAVE_SENDFILE nscd/*.c and clean up
> the result.

In fact since sendfile on HURD is read/write operation and the underlying issue
on Linux, I think we can just remove to use sendfile altogether.  I am really
skeptical it is hitting hotstop here to justify that extra code complexity.

--
You are receiving this mail because:
You are on the CC list for the bug.
Reply | Threaded
Open this post in threaded view
|

[Bug nscd/23178] sudo will fail when it is run in concurrent with commands that changes /etc/passwd

rootedjoy833 at gmail dot com
In reply to this post by rootedjoy833 at gmail dot com
https://sourceware.org/bugzilla/show_bug.cgi?id=23178

Adhemerval Zanella <adhemerval.zanella at linaro dot org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |ASSIGNED
   Last reconfirmed|                            |2018-05-16
           Assignee|unassigned at sourceware dot org   |adhemerval.zanella at linaro dot o
                   |                            |rg
     Ever confirmed|0                           |1

--
You are receiving this mail because:
You are on the CC list for the bug.
Reply | Threaded
Open this post in threaded view
|

[Bug nscd/23178] sudo will fail when it is run in concurrent with commands that changes /etc/passwd

rootedjoy833 at gmail dot com
In reply to this post by rootedjoy833 at gmail dot com
https://sourceware.org/bugzilla/show_bug.cgi?id=23178

--- Comment #4 from cvs-commit at gcc dot gnu.org <cvs-commit at gcc dot gnu.org> ---
This is an automated email from the git hooks/post-receive script. It was
generated because a ref change was pushed to the repository containing
the project "GNU C Library master sources".

The branch, master has been updated
       via  8c78faa9ef5c6cae455739f162e4b9d690e32eca (commit)
      from  04958880e04264da97873b4d41d9bc34567afaef (commit)

Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.

- Log -----------------------------------------------------------------
https://sourceware.org/git/gitweb.cgi?p=glibc.git;h=8c78faa9ef5c6cae455739f162e4b9d690e32eca

commit 8c78faa9ef5c6cae455739f162e4b9d690e32eca
Author: Adhemerval Zanella <[hidden email]>
Date:   Wed May 16 10:51:15 2018 -0300

    Fix concurrent changes on nscd aware files (BZ #23178)

    As indicated by BZ#23178, concurrent access on some files read by nscd
    may result non expected data send through service requisition.  This is
    due 'sendfile' Linux implementation where for sockets with zero-copy
    support, callers must ensure the transferred portions of the the file
    reffered by input file descriptor remain unmodified until the reader
    on the other end of socket has consumed the transferred data.

    I could not find any explicit documentation stating this behaviour on
    Linux kernel documentation.  However man-pages sendfile entry [1] states
    in NOTES the aforementioned remark.  It was initially pushed on man-pages
    with an explicit testcase [2] that shows changing the file used in
    'sendfile' call prior the socket input data consumption results in
    previous data being lost.

    From commit message it stated on tested Linux version (3.15) only TCP
    socket showed this issues, however on recent kernels (4.4) I noticed the
    same behaviour for local sockets as well.

    Since sendfile on HURD is a read/write operation and the underlying
    issue on Linux, the straightforward fix is just remove sendfile use
    altogether.  I am really skeptical it is hitting some hotstop (there
    are indication over internet that sendfile is helpfull only for large
    files, more than 10kb) here to justify that extra code complexity or
    to pursuit other possible fix (through memory or file locks for
    instance, which I am not sure it is doable).

    Checked on x86_64-linux-gnu.

        [BZ #23178]
        * nscd/nscd-client.h (sendfileall): Remove prototype.
        * nscd/connections.c [HAVE_SENDFILE] (sendfileall): Remove function.
        (handle_request): Use writeall instead of sendfileall.
        * nscd/aicache.c (addhstaiX): Likewise.
        * nscd/grpcache.c (cache_addgr): Likewise.
        * nscd/hstcache.c (cache_addhst): Likewise.
        * nscd/initgrcache.c (addinitgroupsX): Likewise.
        * nscd/netgroupcache.c (addgetnetgrentX, addinnetgrX): Likewise.
        * nscd/pwdcache.c (cache_addpw): Likewise.
        * nscd/servicescache.c (cache_addserv): Likewise.
        * sysdeps/unix/sysv/linux/Makefile [$(subdir) == nscd]
        (sysdep-CFLAGS): Remove -DHAVE_SENDFILE.
        * sysdeps/unix/sysv/linux/kernel-features.h (__ASSUME_SENDFILE):
        Remove define.

    [1] http://man7.org/linux/man-pages/man2/sendfile.2.html
    [2]
https://github.com/mkerrisk/man-pages/commit/7b6a3299776b5c1c4f169a591434a855d50c68b4#diff-efd6af3a70f0f07c578e85b51e83b3c3

-----------------------------------------------------------------------

Summary of changes:
 ChangeLog                                 |   18 +++++++++
 nscd/aicache.c                            |   30 +--------------
 nscd/connections.c                        |   54 +-------------------------
 nscd/grpcache.c                           |   37 +----------------
 nscd/hstcache.c                           |   37 +----------------
 nscd/initgrcache.c                        |   37 +----------------
 nscd/netgroupcache.c                      |   59 +---------------------------
 nscd/nscd-client.h                        |    2 -
 nscd/pwdcache.c                           |   37 +----------------
 nscd/servicescache.c                      |   34 +---------------
 sysdeps/unix/sysv/linux/Makefile          |    2 +-
 sysdeps/unix/sysv/linux/kernel-features.h |    3 -
 12 files changed, 40 insertions(+), 310 deletions(-)

--
You are receiving this mail because:
You are on the CC list for the bug.
Reply | Threaded
Open this post in threaded view
|

[Bug nscd/23178] sudo will fail when it is run in concurrent with commands that changes /etc/passwd

rootedjoy833 at gmail dot com
In reply to this post by rootedjoy833 at gmail dot com
https://sourceware.org/bugzilla/show_bug.cgi?id=23178

Adhemerval Zanella <adhemerval.zanella at linaro dot org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
         Resolution|---                         |FIXED
   Target Milestone|---                         |2.28

--- Comment #5 from Adhemerval Zanella <adhemerval.zanella at linaro dot org> ---
Fixed by 8c78faa9ef5c6cae455739f162e4b9d690e32eca

--
You are receiving this mail because:
You are on the CC list for the bug.