diff options
author | Adhemerval Zanella <adhemerval.zanella@linaro.org> | 2018-05-16 10:51:15 -0300 |
---|---|---|
committer | Adhemerval Zanella <adhemerval.zanella@linaro.org> | 2018-05-16 13:44:53 -0300 |
commit | 8c78faa9ef5c6cae455739f162e4b9d690e32eca (patch) | |
tree | 747cfdcde125142a2af392602043c9b174a67472 /nscd/connections.c | |
parent | 04958880e04264da97873b4d41d9bc34567afaef (diff) | |
download | glibc-8c78faa9ef5c6cae455739f162e4b9d690e32eca.tar.gz glibc-8c78faa9ef5c6cae455739f162e4b9d690e32eca.tar.xz glibc-8c78faa9ef5c6cae455739f162e4b9d690e32eca.zip |
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
Diffstat (limited to 'nscd/connections.c')
-rw-r--r-- | nscd/connections.c | 54 |
1 files changed, 2 insertions, 52 deletions
diff --git a/nscd/connections.c b/nscd/connections.c index 5f91985859..1b3bae4eeb 100644 --- a/nscd/connections.c +++ b/nscd/connections.c @@ -46,9 +46,6 @@ #include <sys/mman.h> #include <sys/param.h> #include <sys/poll.h> -#ifdef HAVE_SENDFILE -# include <sys/sendfile.h> -#endif #include <sys/socket.h> #include <sys/stat.h> #include <sys/un.h> @@ -285,26 +282,6 @@ writeall (int fd, const void *buf, size_t len) } -#ifdef HAVE_SENDFILE -ssize_t -sendfileall (int tofd, int fromfd, off_t off, size_t len) -{ - ssize_t n = len; - ssize_t ret; - - do - { - ret = TEMP_FAILURE_RETRY (sendfile (tofd, fromfd, &off, n)); - if (ret <= 0) - break; - n -= ret; - } - while (n > 0); - return ret < 0 ? ret : len - n; -} -#endif - - enum usekey { use_not = 0, @@ -1163,35 +1140,8 @@ request from '%s' [%ld] not handled due to missing permission"), if (cached != NULL) { /* Hurray it's in the cache. */ - ssize_t nwritten; - -#ifdef HAVE_SENDFILE - if (__glibc_likely (db->mmap_used)) - { - assert (db->wr_fd != -1); - assert ((char *) cached->data > (char *) db->data); - assert ((char *) cached->data - (char *) db->head - + cached->recsize - <= (sizeof (struct database_pers_head) - + db->head->module * sizeof (ref_t) - + db->head->data_size)); - nwritten = sendfileall (fd, db->wr_fd, - (char *) cached->data - - (char *) db->head, cached->recsize); -# ifndef __ASSUME_SENDFILE - if (nwritten == -1 && errno == ENOSYS) - goto use_write; -# endif - } - else -# ifndef __ASSUME_SENDFILE - use_write: -# endif -#endif - nwritten = writeall (fd, cached->data, cached->recsize); - - if (nwritten != cached->recsize - && __builtin_expect (debug_level, 0) > 0) + if (writeall (fd, cached->data, cached->recsize) != cached->recsize + && __glibc_unlikely (debug_level > 0)) { /* We have problems sending the result. */ char buf[256]; |