From patchwork Mon Jun 29 15:10:30 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Eric Dumazet X-Patchwork-Id: 6689501 Return-Path: X-Original-To: patchwork-linux-fsdevel@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork2.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork2.web.kernel.org (Postfix) with ESMTP id DD9F9C05AC for ; Mon, 29 Jun 2015 15:10:39 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 01E06204D1 for ; Mon, 29 Jun 2015 15:10:39 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 1B578204AF for ; Mon, 29 Jun 2015 15:10:38 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752102AbbF2PKg (ORCPT ); Mon, 29 Jun 2015 11:10:36 -0400 Received: from mail-wi0-f181.google.com ([209.85.212.181]:34530 "EHLO mail-wi0-f181.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751100AbbF2PKf (ORCPT ); Mon, 29 Jun 2015 11:10:35 -0400 Received: by wiar9 with SMTP id r9so7316740wia.1; Mon, 29 Jun 2015 08:10:33 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=message-id:subject:from:to:cc:date:content-type:mime-version :content-transfer-encoding; bh=t+ejeLS92/IgZ5HC0sjuUz8VQ12vl4znNcvBmAY762I=; b=GVTfpWd3L47F0xWamZ51cFoaqTXt3KqOb/l0P15WMA4SDSDmPEmodxRAwhrk2/deH4 sOBmXvHzld71jIDG5RmBXrkKWCkeZk0de9eXwN2M1A2L+PtqPC4uvGAFNVWOakV5bPEV WYF+gMXgwSIyUM984+W11WIz9sHyWsGd0gUm9Jr/pUIdcrIRzi8QZqexQnTFakLKgQPn M0bkW3CtgRtiTi5Xzv2oBSntjd2jdTaMwll2frhnAQV/IPCyKEODVh9VK22aPZpDAZrU 5pOFOsKs+Wm+aFmx3fMwPLtyaQ+0jBifqCTpDSRge5bXDV3I88SUJC8RQCxP1M/HyWVP ekxw== X-Received: by 10.180.211.109 with SMTP id nb13mr22780119wic.8.1435590633569; Mon, 29 Jun 2015 08:10:33 -0700 (PDT) Received: from [172.16.39.118] ([172.16.39.118]) by mx.google.com with ESMTPSA id v4sm12555791wiy.5.2015.06.29.08.10.31 (version=TLSv1.2 cipher=AES128-GCM-SHA256 bits=128/128); Mon, 29 Jun 2015 08:10:32 -0700 (PDT) Message-ID: <1435590630.4110.107.camel@edumazet-glaptop2.roam.corp.google.com> Subject: [PATCH] fs/file.c: __fget() and dup2() atomicity rules From: Eric Dumazet To: Al Viro Cc: Andrew Morton , "linux-kernel@vger.kernel.org" , Dmitry Vyukov , Eric Dumazet , linux-fsdevel@vger.kernel.org Date: Mon, 29 Jun 2015 17:10:30 +0200 X-Mailer: Evolution 3.10.4-0ubuntu2 Mime-Version: 1.0 Sender: linux-fsdevel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-fsdevel@vger.kernel.org X-Spam-Status: No, score=-7.4 required=5.0 tests=BAYES_00, DKIM_ADSP_CUSTOM_MED, DKIM_SIGNED, FREEMAIL_FROM, RCVD_IN_DNSWL_HI, RP_MATCHES_RCVD, T_DKIM_INVALID, UNPARSEABLE_RELAY autolearn=ham version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP From: Eric Dumazet __fget() makes sure a file refcount is not zero before taking a reference. It should also fetch again file pointer in order to respect dup2() atomicity requirements. It should either read a NULL pointer or a file on which a refcount can be taken. Dmitry had following test failing sometimes : #include #include #include #include #include int fd; void *Thread(void *x) { char buf; int n = read(fd, &buf, 1); if (n != 1) exit(printf("read failed: n=%d errno=%d\n", n, errno)); return 0; } int main() { fd = open("/dev/urandom", O_RDONLY); int fd2 = open("/dev/urandom", O_RDONLY); if (fd == -1 || fd2 == -1) exit(printf("open failed\n")); pthread_t th; pthread_create(&th, 0, Thread, 0); if (dup2(fd2, fd) == -1) exit(printf("dup2 failed\n")); pthread_join(th, 0); if (close(fd) == -1) exit(printf("close failed\n")); if (close(fd2) == -1) exit(printf("close failed\n")); printf("DONE\n"); return 0; } Signed-off-by: Eric Dumazet Reported-by: Dmitry Vyukov --- fs/file.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html diff --git a/fs/file.c b/fs/file.c index 93c5f89c248b..492bd74c4433 100644 --- a/fs/file.c +++ b/fs/file.c @@ -635,11 +635,17 @@ static struct file *__fget(unsigned int fd, fmode_t mask) struct file *file; rcu_read_lock(); +loop: file = fcheck_files(files, fd); if (file) { - /* File object ref couldn't be taken */ - if ((file->f_mode & mask) || !get_file_rcu(file)) + /* File object ref couldn't be taken. + * dup2() atomicity guarantee is the reason + * we loop to catch the new file (or NULL pointer) + */ + if (file->f_mode & mask) file = NULL; + else if (!get_file_rcu(file)) + goto loop; } rcu_read_unlock();