From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 91307 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 91234 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,SPF_PASS 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,SPF_PASS 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:32 +0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1579269870; 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=K9Ovj84tsIX8eHJLQZlB7sPIW+fw5+BBE8wxVyUDJmU=; b=UhdPV/B/J0UFSUYvpnxYPLDTqM4614zx7SwJ20G6/6bYhckkqs6WoBzkrtD4bvLitQthFi KSIjtzbqdPuHwKUAkkMrDr6jhQbB5UAm32RfeE7mObDkzRkQRaCvBQX1kpOWFPYv5Y97os i25KQz5vzbrsNhORC0CgwGCoPPpJgAg= 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-88-PHCh82eGMrmKOP_8ceZAuQ-1; Fri, 17 Jan 2020 09:04:29 -0500 Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.phx2.redhat.com [10.5.11.14]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 5C05C107ACC7 for ; Fri, 17 Jan 2020 14:04:28 +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 E2BC35DA32 for ; Fri, 17 Jan 2020 14:04:27 +0000 (UTC) Received: by oldenburg2.str.redhat.com (Postfix, from userid 1000) id 53FE18299EE2; Fri, 17 Jan 2020 15:04:26 +0100 (CET) Date: Wed, 01 Jan 2020 00:00:00 -0000 To: libc-stable@sourceware.org Subject: [2.30 COMMITTED] login: Replace macro-based control flow with function calls in utmp User-Agent: Heirloom mailx 12.5 7/5/10 MIME-Version: 1.0 Message-Id: <20200117140426.53FE18299EE2@oldenburg2.str.redhat.com> From: Florian Weimer X-Scanned-By: MIMEDefang 2.79 on 10.5.11.14 X-MC-Unique: PHCh82eGMrmKOP_8ceZAuQ-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/msg00006.txt.bz2 (cherry picked from commit 5a3afa9738f3dbbaf8c0a35665318c1af782111b) 2019-08-13 Florian Weimer * login/utmp_file.c (LOCK_FILE, LOCKING_FAILED, UNLOCK_FILE): Remove macros. (struct file_locking): New. (try_file_lock, file_unlock, file_lock_restore): New functions. (__libc_getutent_r): Use the new functions. (internal_getut_r): Likewise. (__libc_getutline_r): Likewise. (__libc_pututline): Likewise. (__libc_updwtmp): Likewise. diff --git a/login/utmp_file.c b/login/utmp_file.c index 9badf11fb3..7bd6034af4 100644 --- a/login/utmp_file.c +++ b/login/utmp_file.c @@ -52,58 +52,71 @@ static struct utmp last_entry; /* Do-nothing handler for locking timeout. */ static void timeout_handler (int signum) {}; -/* LOCK_FILE(fd, type) failure_statement - attempts to get a lock on the utmp file referenced by FD. If it fails, - the failure_statement is executed, otherwise it is skipped. - LOCKING_FAILED() - jumps into the UNLOCK_FILE macro and ensures cleanup of LOCK_FILE. - UNLOCK_FILE(fd) - unlocks the utmp file referenced by FD and performs the cleanup of - LOCK_FILE. - */ -#define LOCK_FILE(fd, type) \ -{ \ - struct flock fl; \ - struct sigaction action, old_action; \ - unsigned int old_timeout; \ - \ - /* Cancel any existing alarm. */ \ - old_timeout = alarm (0); \ - \ - /* Establish signal handler. */ \ - action.sa_handler = timeout_handler; \ - __sigemptyset (&action.sa_mask); \ - action.sa_flags = 0; \ - __sigaction (SIGALRM, &action, &old_action); \ - \ - alarm (TIMEOUT); \ - \ - /* Try to get the lock. */ \ - memset (&fl, '\0', sizeof (struct flock)); \ - fl.l_type = (type); \ - fl.l_whence = SEEK_SET; \ - if (__fcntl64_nocancel ((fd), F_SETLKW, &fl) < 0) - -#define LOCKING_FAILED() \ - goto unalarm_return - -#define UNLOCK_FILE(fd) \ - /* Unlock the file. */ \ - fl.l_type = F_UNLCK; \ - __fcntl64_nocancel ((fd), F_SETLKW, &fl); \ - \ - unalarm_return: \ - /* 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); \ -} while (0) + +/* 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; +}; + +static bool +try_file_lock (struct file_locking *locking, int fd, int type) +{ + /* Cancel any existing alarm. */ + locking->old_timeout = alarm (0); + + /* Establish signal handler. */ + struct sigaction action; + action.sa_handler = timeout_handler; + __sigemptyset (&action.sa_mask); + action.sa_flags = 0; + __sigaction (SIGALRM, &action, &locking->old_action); + + alarm (TIMEOUT); + + /* Try to get the lock. */ + struct flock fl = + { + .l_type = type, + fl.l_whence = SEEK_SET, + }; + return __fcntl64_nocancel (fd, F_SETLKW, &fl) < 0; +} + +static void +file_unlock (int fd) +{ + struct flock fl = + { + .l_type = F_UNLCK, + }; + __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) @@ -153,16 +166,16 @@ __libc_getutent_r (struct utmp *buffer, struct utmp **result) return -1; } - LOCK_FILE (file_fd, F_RDLCK) + struct file_locking fl; + if (try_file_lock (&fl, file_fd, F_RDLCK)) + nbytes = 0; + else { - nbytes = 0; - LOCKING_FAILED (); + /* Read the next entry. */ + nbytes = __read_nocancel (file_fd, &last_entry, sizeof (struct utmp)); + file_unlock (file_fd); } - - /* Read the next entry. */ - nbytes = __read_nocancel (file_fd, &last_entry, sizeof (struct utmp)); - - UNLOCK_FILE (file_fd); + file_lock_restore (&fl); if (nbytes != sizeof (struct utmp)) { @@ -188,10 +201,12 @@ internal_getut_r (const struct utmp *id, struct utmp *buffer, { int result = -1; - LOCK_FILE (file_fd, F_RDLCK) + struct file_locking fl; + if (try_file_lock (&fl, file_fd, F_RDLCK)) { *lock_failed = true; - LOCKING_FAILED (); + file_lock_restore (&fl); + return -1; } if (id->ut_type == RUN_LVL || id->ut_type == BOOT_TIME @@ -241,7 +256,8 @@ internal_getut_r (const struct utmp *id, struct utmp *buffer, result = 0; unlock_return: - UNLOCK_FILE (file_fd); + file_unlock (file_fd); + file_lock_restore (&fl); return result; } @@ -287,10 +303,12 @@ __libc_getutline_r (const struct utmp *line, struct utmp *buffer, return -1; } - LOCK_FILE (file_fd, F_RDLCK) + struct file_locking fl; + if (try_file_lock (&fl, file_fd, F_RDLCK)) { *result = NULL; - LOCKING_FAILED (); + file_lock_restore (&fl); + return -1; } while (1) @@ -318,7 +336,8 @@ __libc_getutline_r (const struct utmp *line, struct utmp *buffer, *result = buffer; unlock_return: - UNLOCK_FILE (file_fd); + file_unlock (file_fd); + file_lock_restore (&fl); return ((*result == NULL) ? -1 : 0); } @@ -375,10 +394,11 @@ __libc_pututline (const struct utmp *data) } } - LOCK_FILE (file_fd, F_WRLCK) + struct file_locking fl; + if (try_file_lock (&fl, file_fd, F_WRLCK)) { - pbuf = NULL; - LOCKING_FAILED (); + file_lock_restore (&fl); + return NULL; } if (found < 0) @@ -421,7 +441,8 @@ __libc_pututline (const struct utmp *data) } unlock_return: - UNLOCK_FILE (file_fd); + file_unlock (file_fd); + file_lock_restore (&fl); return pbuf; } @@ -450,8 +471,13 @@ __libc_updwtmp (const char *file, const struct utmp *utmp) if (fd < 0) return -1; - LOCK_FILE (fd, F_WRLCK) - LOCKING_FAILED (); + struct file_locking fl; + if (try_file_lock (&fl, fd, F_WRLCK)) + { + file_lock_restore (&fl); + __close_nocancel_nostatus (fd); + return -1; + } /* Remember original size of log file. */ offset = __lseek64 (fd, 0, SEEK_END); @@ -477,7 +503,8 @@ __libc_updwtmp (const char *file, const struct utmp *utmp) result = 0; unlock_return: - UNLOCK_FILE (fd); + file_unlock (file_fd); + file_lock_restore (&fl); /* Close WTMP file. */ __close_nocancel_nostatus (fd);