From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 91256 invoked by alias); 17 Jan 2020 14:04:43 -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 91233 invoked by uid 89); 17 Jan 2020 14:04:42 -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.6 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=acquired, sigaction X-Spam-Status: No, score=-18.6 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-delivery-1.mimecast.com (HELO us-smtp-1.mimecast.com) (205.139.110.120) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Fri, 17 Jan 2020 14:04:37 +0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1579269876; 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=NzJbdPexQt3Ij9gW8ZDXQ5rcxboZCg3WzLhYul0vLbM=; b=MmERUC2+wUBY1rJYW4slUCQvCKqFdpQMBwPWReRR2llgFapSyL3jxYIgMAGKEKYnHVwRON VuMVFmVXfqc2K8mmXa4I0+O1XOhgekz/ETQNRBk7yUTHAu+zHQgdm5hRL/Gky2CYGqxu8L HvjaI4HKBKGbUplsdZnA8GJDWubVmwQ= 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-300-MkktQcU_OuyCjAZpelDAiQ-1; Fri, 17 Jan 2020 09:04:33 -0500 Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.phx2.redhat.com [10.5.11.13]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id A1DD5A0CC5 for ; Fri, 17 Jan 2020 14:04:32 +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 4FF6E6609E for ; Fri, 17 Jan 2020 14:04:32 +0000 (UTC) Received: by oldenburg2.str.redhat.com (Postfix, from userid 1000) id ABC1F8299EE3; Fri, 17 Jan 2020 15:04:30 +0100 (CET) Date: Wed, 01 Jan 2020 00:00:00 -0000 To: libc-stable@sourceware.org Subject: [2.30 COMMITTED] login: Disarm timer after utmp lock acquisition [BZ #24879] User-Agent: Heirloom mailx 12.5 7/5/10 MIME-Version: 1.0 Message-Id: <20200117140430.ABC1F8299EE3@oldenburg2.str.redhat.com> From: Florian Weimer X-Scanned-By: MIMEDefang 2.79 on 10.5.11.13 X-MC-Unique: MkktQcU_OuyCjAZpelDAiQ-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/msg00002.txt.bz2 If the file processing takes a long time for some reason, SIGALRM can arrive while the file is still being processed. At that point, file access will fail with EINTR. Disarming the timer after lock acquisition avoids that. (If there was a previous alarm, it is the responsibility of the caller to deal with the EINTR error.) (cherry picked from commit 628598be7e1bfaa04f34df71ef6678f2c5103dfd) 2019-08-15 Florian Weimer [BZ #24879] login: Disarm timer after utmp lock acquisition. * login/utmp_file.c (struct file_locking): Remove. (try_file_lock): Adjust. (file_lock_restore): Remove function. (__libc_getutent_r): . (internal_getut_r): Likewise. (__libc_getutline_r): Likewise. (__libc_pututline): Likewise. (__libc_updwtmp): Likewise. diff --git a/NEWS b/NEWS index 4d26e01248..96458cf0b9 100644 --- a/NEWS +++ b/NEWS @@ -21,6 +21,7 @@ The following bugs are resolved with this release: [24682] localedata: zh_CN first weekday should be Monday per GB/T 7408-2005 [24867] malloc: Remove unwanted leading whitespace in malloc_info + [24879] login: Disarm timer after utmp lock acquisition [24986] alpha: new getegid, geteuid and getppid syscalls used unconditionally [25189] Don't use a custom wrapper macro around __has_include diff --git a/login/utmp_file.c b/login/utmp_file.c index 5e4e66d1d0..f3c528384f 100644 --- a/login/utmp_file.c +++ b/login/utmp_file.c @@ -55,32 +55,23 @@ static void timeout_handler (int signum) {}; /* try_file_lock (LOCKING, FD, TYPE) returns true if the locking operation failed and recovery needs to be performed. - (file_lock_restore (LOCKING) still needs to be called.) file_unlock (FD) removes the lock (which must have been - acquired). - - file_lock_restore (LOCKING) is needed to clean up in both - cases. */ - -struct file_locking -{ - struct sigaction old_action; - unsigned int old_timeout; -}; + successfully acquired). */ static bool -try_file_lock (struct file_locking *locking, int fd, int type) +try_file_lock (int fd, int type) { /* Cancel any existing alarm. */ - locking->old_timeout = alarm (0); + int old_timeout = alarm (0); /* Establish signal handler. */ + struct sigaction old_action; struct sigaction action; action.sa_handler = timeout_handler; __sigemptyset (&action.sa_mask); action.sa_flags = 0; - __sigaction (SIGALRM, &action, &locking->old_action); + __sigaction (SIGALRM, &action, &old_action); alarm (TIMEOUT); @@ -90,7 +81,23 @@ try_file_lock (struct file_locking *locking, int fd, int type) .l_type = type, fl.l_whence = SEEK_SET, }; - return __fcntl64_nocancel (fd, F_SETLKW, &fl) < 0; + + bool status = __fcntl64_nocancel (fd, F_SETLKW, &fl) < 0; + int saved_errno = errno; + + /* Reset the signal handler and alarm. We must reset the alarm + before resetting the handler so our alarm does not generate a + spurious SIGALRM seen by the user. However, we cannot just set + the user's old alarm before restoring the handler, because then + it's possible our handler could catch the user alarm's SIGARLM and + then the user would never see the signal he expected. */ + alarm (0); + __sigaction (SIGALRM, &old_action, NULL); + if (old_timeout != 0) + alarm (old_timeout); + + __set_errno (saved_errno); + return status; } static void @@ -103,21 +110,6 @@ file_unlock (int fd) __fcntl64_nocancel (fd, F_SETLKW, &fl); } -static void -file_lock_restore (struct file_locking *locking) -{ - /* Reset the signal handler and alarm. We must reset the alarm - before resetting the handler so our alarm does not generate a - spurious SIGALRM seen by the user. However, we cannot just set - the user's old alarm before restoring the handler, because then - it's possible our handler could catch the user alarm's SIGARLM - and then the user would never see the signal he expected. */ - alarm (0); - __sigaction (SIGALRM, &locking->old_action, NULL); - if (locking->old_timeout != 0) - alarm (locking->old_timeout); -} - #ifndef TRANSFORM_UTMP_FILE_NAME # define TRANSFORM_UTMP_FILE_NAME(file_name) (file_name) #endif @@ -166,8 +158,7 @@ __libc_getutent_r (struct utmp *buffer, struct utmp **result) return -1; } - struct file_locking fl; - if (try_file_lock (&fl, file_fd, F_RDLCK)) + if (try_file_lock (file_fd, F_RDLCK)) nbytes = 0; else { @@ -175,7 +166,6 @@ __libc_getutent_r (struct utmp *buffer, struct utmp **result) nbytes = __read_nocancel (file_fd, &last_entry, sizeof (struct utmp)); file_unlock (file_fd); } - file_lock_restore (&fl); if (nbytes != sizeof (struct utmp)) { @@ -201,11 +191,9 @@ internal_getut_r (const struct utmp *id, struct utmp *buffer, { int result = -1; - struct file_locking fl; - if (try_file_lock (&fl, file_fd, F_RDLCK)) + if (try_file_lock (file_fd, F_RDLCK)) { *lock_failed = true; - file_lock_restore (&fl); return -1; } @@ -257,7 +245,6 @@ internal_getut_r (const struct utmp *id, struct utmp *buffer, unlock_return: file_unlock (file_fd); - file_lock_restore (&fl); return result; } @@ -303,11 +290,9 @@ __libc_getutline_r (const struct utmp *line, struct utmp *buffer, return -1; } - struct file_locking fl; - if (try_file_lock (&fl, file_fd, F_RDLCK)) + if (try_file_lock (file_fd, F_RDLCK)) { *result = NULL; - file_lock_restore (&fl); return -1; } @@ -337,7 +322,6 @@ __libc_getutline_r (const struct utmp *line, struct utmp *buffer, unlock_return: file_unlock (file_fd); - file_lock_restore (&fl); return ((*result == NULL) ? -1 : 0); } @@ -394,12 +378,8 @@ __libc_pututline (const struct utmp *data) } } - struct file_locking fl; - if (try_file_lock (&fl, file_fd, F_WRLCK)) - { - file_lock_restore (&fl); - return NULL; - } + if (try_file_lock (file_fd, F_WRLCK)) + return NULL; if (found < 0) { @@ -442,7 +422,6 @@ __libc_pututline (const struct utmp *data) unlock_return: file_unlock (file_fd); - file_lock_restore (&fl); return pbuf; } @@ -471,10 +450,8 @@ __libc_updwtmp (const char *file, const struct utmp *utmp) if (fd < 0) return -1; - struct file_locking fl; - if (try_file_lock (&fl, fd, F_WRLCK)) + if (try_file_lock (fd, F_WRLCK)) { - file_lock_restore (&fl); __close_nocancel_nostatus (fd); return -1; } @@ -504,7 +481,6 @@ __libc_updwtmp (const char *file, const struct utmp *utmp) unlock_return: file_unlock (fd); - file_lock_restore (&fl); /* Close WTMP file. */ __close_nocancel_nostatus (fd);