From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 92986 invoked by alias); 17 Jan 2020 14:05:00 -0000 Mailing-List: contact libc-stable-help@sourceware.org; run by ezmlm Precedence: bulk List-Post: List-Help: List-Subscribe: List-Archive: Sender: libc-stable-owner@sourceware.org Received: (qmail 92827 invoked by uid 89); 17 Jan 2020 14:04:59 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Checked: by ClamAV 0.100.3 on sourceware.org X-Virus-Found: No X-Spam-SWARE-Status: No, score=-18.7 required=5.0 tests=AWL,BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,RCVD_IN_DNSWL_NONE autolearn=ham version=3.3.1 spammy=22915 X-Spam-Status: No, score=-18.7 required=5.0 tests=AWL,BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,RCVD_IN_DNSWL_NONE autolearn=ham version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on sourceware.org X-Spam-Level: X-HELO: us-smtp-1.mimecast.com Received: from us-smtp-1.mimecast.com (HELO us-smtp-1.mimecast.com) (207.211.31.81) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Fri, 17 Jan 2020 14:04:47 +0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1579269885; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=vgz/H47g+uvkl5l0LBfRx+3gTT8ReJ68O/erJDb43/w=; b=OYVCWlJbD/Bi7kAppowbVBOlj65nAYqAsX6iYqMwuDPWyDXFeLyv27M2YdcVd6e95LdyC6 C7rC5l8iO/z5HfNa4Oh16nTAeBKoiMemaoFyW200r685SgN5ibrsN2U+A32OL6nqN822V1 oXG9OK8rY7kJnKCWTUl0gVBLmyhQF9w= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-3-kB_06pI2PV6rTIHV3qZa-A-1; Fri, 17 Jan 2020 09:04:44 -0500 Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 682BC801E67 for ; Fri, 17 Jan 2020 14:04:43 +0000 (UTC) Received: from oldenburg2.str.redhat.com (ovpn-117-165.ams2.redhat.com [10.36.117.165]) by smtp.corp.redhat.com (Postfix) with ESMTPS id F10D280F40 for ; Fri, 17 Jan 2020 14:04:42 +0000 (UTC) Received: by oldenburg2.str.redhat.com (Postfix, from userid 1000) id 775238299EE3; Fri, 17 Jan 2020 15:04:41 +0100 (CET) Date: Wed, 01 Jan 2020 00:00:00 -0000 To: libc-stable@sourceware.org Subject: [2.30 COMMITTED] login: Use pread64 in utmp implementation User-Agent: Heirloom mailx 12.5 7/5/10 MIME-Version: 1.0 Message-Id: <20200117140441.775238299EE3@oldenburg2.str.redhat.com> From: Florian Weimer X-Scanned-By: MIMEDefang 2.79 on 10.5.11.11 X-MC-Unique: kB_06pI2PV6rTIHV3qZa-A-1 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: 7bit X-IsSubscribed: yes X-SW-Source: 2020-01/txt/msg00012.txt.bz2 This reduces the possible error scenarios considerably because no longer can file seek fail, leaving the file descriptor in an inconsistent state and out of sync with the cache. As a result, it is possible to avoid setting file_offset to -1 to make an error persistent. Instead, subsequent calls will retry the operation and report any errors returned by the kernel. This change also avoids reading the file from the start if pututline is called multiple times, to work around lock acquisition failures due to timeouts. Change-Id: If21ea0c162c38830a89331ea93cddec14c0974de (cherry picked from commit d4625a19fe64f664119a541b317fb83de01bb273) diff --git a/login/utmp_file.c b/login/utmp_file.c index b28fb762d7..e98bc31899 100644 --- a/login/utmp_file.c +++ b/login/utmp_file.c @@ -162,12 +162,35 @@ maybe_setutent (void) return file_fd >= 0 || __libc_setutent (); } +/* Reads the entry at file_offset, storing it in last_entry and + updating file_offset on success. Returns -1 for a read error, 0 + for EOF, and 1 for a successful read. last_entry and file_offset + are only updated on a successful and complete read. */ +static ssize_t +read_last_entry (void) +{ + struct utmp buffer; + ssize_t nbytes = __pread64_nocancel (file_fd, &buffer, sizeof (buffer), + file_offset); + if (nbytes < 0) + return -1; + else if (nbytes != sizeof (buffer)) + /* Assume EOF. */ + return 0; + else + { + last_entry = buffer; + file_offset += sizeof (buffer); + return 1; + } +} + int __libc_getutent_r (struct utmp *buffer, struct utmp **result) { - ssize_t nbytes; + int saved_errno = errno; - if (!maybe_setutent () || file_offset == -1l) + if (!maybe_setutent ()) { /* Not available. */ *result = NULL; @@ -175,25 +198,22 @@ __libc_getutent_r (struct utmp *buffer, struct utmp **result) } if (try_file_lock (file_fd, F_RDLCK)) - nbytes = 0; - else - { - /* Read the next entry. */ - nbytes = __read_nocancel (file_fd, &last_entry, sizeof (struct utmp)); - file_unlock (file_fd); - } + return -1; + + ssize_t nbytes = read_last_entry (); + file_unlock (file_fd); - if (nbytes != sizeof (struct utmp)) + if (nbytes <= 0) /* Read error or EOF. */ { - if (nbytes != 0) - file_offset = -1l; + if (nbytes == 0) + /* errno should be unchanged to indicate success. A premature + EOF is treated like an EOF (missing complete record at the + end). */ + __set_errno (saved_errno); *result = NULL; return -1; } - /* Update position pointer. */ - file_offset += sizeof (struct utmp); - memcpy (buffer, &last_entry, sizeof (struct utmp)); *result = buffer; @@ -209,15 +229,15 @@ internal_getut_nolock (const struct utmp *id) { while (1) { - /* Read the next entry. */ - if (__read_nocancel (file_fd, &last_entry, sizeof (struct utmp)) - != sizeof (struct utmp)) + ssize_t nbytes = read_last_entry (); + if (nbytes < 0) + return -1; + if (nbytes == 0) { + /* End of file reached. */ __set_errno (ESRCH); - file_offset = -1l; return -1; } - file_offset += sizeof (struct utmp); if (matches_last_entry (id)) break; @@ -249,7 +269,7 @@ int __libc_getutid_r (const struct utmp *id, struct utmp *buffer, struct utmp **result) { - if (!maybe_setutent () || file_offset == -1l) + if (!maybe_setutent ()) { *result = NULL; return -1; @@ -276,7 +296,7 @@ int __libc_getutline_r (const struct utmp *line, struct utmp *buffer, struct utmp **result) { - if (!maybe_setutent () || file_offset == -1l) + if (!maybe_setutent ()) { *result = NULL; return -1; @@ -290,16 +310,21 @@ __libc_getutline_r (const struct utmp *line, struct utmp *buffer, while (1) { - /* Read the next entry. */ - if (__read_nocancel (file_fd, &last_entry, sizeof (struct utmp)) - != sizeof (struct utmp)) + ssize_t nbytes = read_last_entry (); + if (nbytes < 0) { + file_unlock (file_fd); + *result = NULL; + return -1; + } + if (nbytes == 0) + { + /* End of file reached. */ + file_unlock (file_fd); __set_errno (ESRCH); - file_offset = -1l; *result = NULL; - goto unlock_return; + return -1; } - file_offset += sizeof (struct utmp); /* Stop if we found a user or login entry. */ if ((last_entry.ut_type == USER_PROCESS @@ -309,20 +334,18 @@ __libc_getutline_r (const struct utmp *line, struct utmp *buffer, break; } + file_unlock (file_fd); memcpy (buffer, &last_entry, sizeof (struct utmp)); *result = buffer; -unlock_return: - file_unlock (file_fd); - - return ((*result == NULL) ? -1 : 0); + return 0; } struct utmp * __libc_pututline (const struct utmp *data) { - if (!maybe_setutent () || file_offset == -1l) + if (!maybe_setutent ()) return NULL; struct utmp *pbuf; @@ -337,8 +360,7 @@ __libc_pututline (const struct utmp *data) if (new_fd == -1) return NULL; - if (__lseek64 (new_fd, __lseek64 (file_fd, 0, SEEK_CUR), SEEK_SET) == -1 - || __dup2 (new_fd, file_fd) < 0) + if (__dup2 (new_fd, file_fd) < 0) { __close_nocancel_nostatus (new_fd); return NULL; @@ -355,69 +377,70 @@ __libc_pututline (const struct utmp *data) bool found = false; if (matches_last_entry (data)) { - if (__lseek64 (file_fd, file_offset, SEEK_SET) < 0) + /* Read back the entry under the write lock. */ + file_offset -= sizeof (last_entry); + ssize_t nbytes = read_last_entry (); + if (nbytes < 0) { file_unlock (file_fd); return NULL; } - if (__read_nocancel (file_fd, &last_entry, sizeof (last_entry)) - != sizeof (last_entry)) - { - if (__lseek64 (file_fd, file_offset, SEEK_SET) < 0) - { - file_unlock (file_fd); - return NULL; - } - found = false; - } + + if (nbytes == 0) + /* End of file reached. */ + found = false; else found = matches_last_entry (data); } if (!found) + /* Search forward for the entry. */ found = internal_getut_nolock (data) >= 0; + off64_t write_offset; if (!found) { /* We append the next entry. */ - file_offset = __lseek64 (file_fd, 0, SEEK_END); - if (file_offset % sizeof (struct utmp) != 0) - { - file_offset -= file_offset % sizeof (struct utmp); - __ftruncate64 (file_fd, file_offset); + write_offset = __lseek64 (file_fd, 0, SEEK_END); - if (__lseek64 (file_fd, 0, SEEK_END) < 0) - { - pbuf = NULL; - goto unlock_return; - } - } + /* Round down to the next multiple of the entry size. This + ensures any partially-written record is overwritten by the + new record. */ + write_offset = (write_offset / sizeof (struct utmp) + * sizeof (struct utmp)); } else - { - /* We replace the just read entry. */ - file_offset -= sizeof (struct utmp); - __lseek64 (file_fd, file_offset, SEEK_SET); - } + /* Overwrite last_entry. */ + write_offset = file_offset - sizeof (struct utmp); /* Write the new data. */ - if (__write_nocancel (file_fd, data, sizeof (struct utmp)) - != sizeof (struct utmp)) + ssize_t nbytes; + if (__lseek64 (file_fd, write_offset, SEEK_SET) < 0 + || (nbytes = __write_nocancel (file_fd, data, sizeof (struct utmp))) < 0) + { + /* There is no need to recover the file position because all + reads use pread64, and any future write is preceded by + another seek. */ + file_unlock (file_fd); + return NULL; + } + + if (nbytes != sizeof (struct utmp)) { /* If we appended a new record this is only partially written. Remove it. */ if (!found) - (void) __ftruncate64 (file_fd, file_offset); - pbuf = NULL; - } - else - { - file_offset += sizeof (struct utmp); - pbuf = (struct utmp *) data; + (void) __ftruncate64 (file_fd, write_offset); + file_unlock (file_fd); + /* Assume that the write failure was due to missing disk + space. */ + __set_errno (ENOSPC); + return NULL; } - unlock_return: file_unlock (file_fd); + file_offset = write_offset + sizeof (struct utmp); + pbuf = (struct utmp *) data; return pbuf; }