From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:43299) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Ve7rq-00070T-Ld for qemu-devel@nongnu.org; Wed, 06 Nov 2013 13:30:19 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Ve7rl-0007bA-VF for qemu-devel@nongnu.org; Wed, 06 Nov 2013 13:30:14 -0500 Received: from [204.155.152.216] (port=37087 helo=shutemov.name) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Ve7rl-0007Th-QH for qemu-devel@nongnu.org; Wed, 06 Nov 2013 13:30:09 -0500 Date: Wed, 6 Nov 2013 19:33:57 +0200 From: "Kirill A. Shutemov" Message-ID: <20131106173357.GA22790@shutemov.name> References: <1383558955-15033-1-git-send-email-kirill@shutemov.name> <8761s5tsqk.fsf@linux.vnet.ibm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <8761s5tsqk.fsf@linux.vnet.ibm.com> Subject: Re: [Qemu-devel] [RESEND] [PATCH] hw/9pfs: fix P9_STATS_GEN handling List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Aneesh Kumar K.V" Cc: "Kirill A. Shutemov" , qemu-devel@nongnu.org, aliguori@amazon.com On Wed, Nov 06, 2013 at 09:41:47PM +0530, Aneesh Kumar K.V wrote: > "Kirill A. Shutemov" writes: > > > From: "Kirill A. Shutemov" > > > > Currently we have few issues with P9_STATS_GEN: > > > > - We don't try to read st_gen anything except files or directories, but > > still set P9_STATS_GEN bit in st_result_mask. It may mislead client: > > we present garbage as valid st_gen. > > We should return 0 right ? We do > > memset(v9lstat, 0, sizeof(*v9lstat)); > > in stat_to_v9stat_dotl The right way is not set P9_STATS_GEN in st_result_mask if we don't know it. > > - If we failed to get valid st_gen with ENOTTY, we ignore error, but > > still set P9_STATS_GEN bit in st_result_mask. > > and have v9lstat.st_gen set to zero The same as above. And if ioctl(fd, FS_IOC_GETVERSION, st_gen) fails, nobody specifies what will be stored into st_gen, if any. We should not relay that fs will not touch st_gen even if it sounds reasonable. > > > > > - If we failed to get valid st_gen with any other errno, we fail > > getattr altogether. It's excessive: we block valid client use-cases, > > like chdir(2) to non-readable directory with execution bit set. > > > > Can you explain this in detail ? If you have following tree: % mkdir testdir % echo test > testdir/testfile % chmod -r testdir In normal environment it's usable: you can chdir(2) into it and read files inside if you know its name: % cd testdir % cat testfile test You only can't list directory content: % ls ls: cannot open directory .: Permission denied With current qemu 9p implementation you'll get on guest -EACCES on chdir(2) or read, since qemu fill fail to provide basic stats to guess. It happens because qemu try open(2) non-readable file to run FS_IOC_GETVERSION and fails getattr altogether. I believe it also breaks more trivial use-cases: ls -l on non-readable file or directory for the same reason. But I haven't checked that. -- Kirill A. Shutemov