From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: ARC-Seal: i=1; a=rsa-sha256; t=1516994629; cv=none; d=google.com; s=arc-20160816; b=Ap6BplG9dtEChEbIoxHim04JMonRMLVPhVDmyVqCOTLYzklv/Qm8aLIjzXC/P2NVGm i0BbMrPMIgViE0e08ffJWmcaTpKZJ9W8G4ITkhA5mSmb1tFunEQhhnxqn9T7OBUj0OVc NZbFNoVTOJHKlbbkeKvR5tHax6sNsRvNQs2mgoqQ5BqF+pwnMFIyiJj2sUFywJKnh7OD FAFsVm5eSfdFG18nqUBwbH8nru1ESvGoWddnAEfaiWYBuLxpv5txfEE3mtQJdm89ZAJD NUco3pywJpYhTkhGL89yxR0tqnNd0dd9PMKutxFYvfOyMAfxUcSOVKArQ/Fyj3Ay8nOP hjkQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=cc:to:subject:message-id:date:from:references:in-reply-to :mime-version:dkim-signature:arc-authentication-results; bh=Mc3Vi4Eqkx/5hqssxO0fKM7ugVKhViUDA9HeCNbW5tk=; b=tyJiA8pCwAtMmje75qIJ21onHAE1JSnQKUv/PNcXhCY1bs9t1PTgVf0T3X4EDnJgnl q+VSiV5PIVjrfsHg0yZjp/eZ4sKuCaSieAm7Zm520YGECPn6HOhzxdXe3k8bDkJZMIP4 4p6rRSDadV/g2PtM5zwnAj3e+VmlKJwd/BmAJ00iXoMwUto5+PmQe4NgZ1QjF8PBVgPJ KXuKhIzNhBvv2WMXIgYlgoB0Pvd882YKCp1YfJhjklWwFrLVY86/j9gPhaGBkrTi0prF NL7eMhgF9eJwgT/24nvdrt9y/EfxKFxlPqZFmxcG+xQRp6HItUse6+WRu8pKF62eqMMs RSJw== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@google.com header.s=20161025 header.b=o+hzclCz; spf=pass (google.com: domain of joelaf@google.com designates 209.85.220.65 as permitted sender) smtp.mailfrom=joelaf@google.com; dmarc=pass (p=REJECT sp=REJECT dis=NONE) header.from=google.com Authentication-Results: mx.google.com; dkim=pass header.i=@google.com header.s=20161025 header.b=o+hzclCz; spf=pass (google.com: domain of joelaf@google.com designates 209.85.220.65 as permitted sender) smtp.mailfrom=joelaf@google.com; dmarc=pass (p=REJECT sp=REJECT dis=NONE) header.from=google.com X-Google-Smtp-Source: AH8x225mys9QGXHAkDWbfWOcp62my+2jm1hRAjWQzznxZakP/HdWgvMN2txi6qKYU51qwhShh515W9QK+fWwjSAIFPg= MIME-Version: 1.0 In-Reply-To: <20180126031346.GW13338@ZenIV.linux.org.uk> References: <20180126024649.200330-1-joelaf@google.com> <20180126031346.GW13338@ZenIV.linux.org.uk> From: Joel Fernandes Date: Fri, 26 Jan 2018 11:23:47 -0800 Message-ID: Subject: Re: [PATCH] ashmem: Fix lockdep issue during llseek To: Al Viro Cc: LKML , Todd Kjos , Arve Hjonnevag , Greg Kroah-Hartman Content-Type: text/plain; charset="UTF-8" X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: =?utf-8?q?1590621447299099171?= X-GMAIL-MSGID: =?utf-8?q?1590684160116105126?= X-Mailing-List: linux-kernel@vger.kernel.org List-ID: Hi Al, On Thu, Jan 25, 2018 at 7:13 PM, Al Viro wrote: > On Thu, Jan 25, 2018 at 06:46:49PM -0800, Joel Fernandes wrote: >> ashmem_mutex create a chain of dependencies like so: >> >> (1) >> mmap syscall -> >> mmap_sem -> (acquired) >> ashmem_mmap >> ashmem_mutex (try to acquire) >> (block) >> >> (2) >> llseek syscall -> >> ashmem_llseek -> >> ashmem_mutex -> (acquired) >> inode_lock -> >> inode->i_rwsem (try to acquire) >> (block) >> >> (3) >> getdents -> >> iterate_dir -> >> inode_lock -> >> inode->i_rwsem (acquired) >> copy_to_user -> >> mmap_sem (try to acquire) >> >> There is a lock ordering created between mmap_sem and inode->i_rwsem >> during a syzcaller test, this patch fixes the issue by releasing the >> ashmem_mutex before the call to vfs_llseek, and reacquiring it after. > > That looks odd. If this approach works, what the hell do you need > ashmem_mutex for in ashmem_llseek() in the first place? What is > it protecting there? I was just trying to be careful with the least intrusive solution since I'm not the original author of the driver. But one usecase for the mutex is with concurrent lseeks, you can end up with a file->f_pos that is different from the latest update to asma->file->f_pos. A barrier could fix this it too though. Any thoughts? ================================================================= CPU 1 CPU 2 // vfs_llseek updated // asma->file->f_pos to X // vfs_llseek updated // file->f_pos to Y asma->file->f_pos updated to Y asma->file->f_pos updated to X (lost write) ================================================================= thanks, - Joel