From mboxrd@z Thu Jan 1 00:00:00 1970 From: Viacheslav Dubeyko Subject: Re: [PATCH] hfsplus: fix cross-page bio requests Date: Tue, 09 Jun 2015 09:36:47 -0700 Message-ID: <1433867807.2668.3.camel@slavad-ubuntu-14.04> References: <1428656543-6790-1-git-send-email-saproj@gmail.com> <1428684500.2798.4.camel@slavad-ubuntu-14.04> <1433782994.2659.8.camel@slavad-ubuntu-14.04> Mime-Version: 1.0 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: 7bit Cc: "linux-fsdevel@vger.kernel.org" , Anton Altaparmakov , Al Viro , Christoph Hellwig , Andrew Morton , Hin-Tak Leung , Sougata Santra To: Sergei Antonov Return-path: Received: from mail-qk0-f172.google.com ([209.85.220.172]:33530 "EHLO mail-qk0-f172.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752521AbbFIQgv (ORCPT ); Tue, 9 Jun 2015 12:36:51 -0400 Received: by qkhg32 with SMTP id g32so12286154qkh.0 for ; Tue, 09 Jun 2015 09:36:51 -0700 (PDT) In-Reply-To: Sender: linux-fsdevel-owner@vger.kernel.org List-ID: On Mon, 2015-06-08 at 19:20 +0200, Sergei Antonov wrote: > On 8 June 2015 at 19:03, Viacheslav Dubeyko wrote: > > On Sun, 2015-06-07 at 22:09 +0200, Sergei Antonov wrote: > >> On 7 June 2015 at 22:05, Sergei Antonov wrote: > >> > On 10 April 2015 at 18:48, Viacheslav Dubeyko wrote: > >> >> On Fri, 2015-04-10 at 11:02 +0200, Sergei Antonov wrote: > >> >>> Function hfsplus_submit_bio() did not work when the passed buffer spanned > >> >>> over more than one page. That was because bio_alloc() is passed 1 as a number > >> >>> of vectors but more than one vector were added inside the 'while' loop. > >> >>> It periodically caused a mount error when the volume header could not be read. > >> >>> > >> >>> This patch modifies the code so that only one vector is used. It works for > >> >>> multiple pages too. Also adds a return code check after bio_alloc(). > >> >> > >> >> I think that it really makes sense to describe the issue's reproducing > >> >> way. It will be really precious for understanding of symptoms and > >> >> reasons of the issue. > >> >> > >> >> Could you add more detailed description? > >> >> > >> >> Then, I will have opportunity to test your patch. > >> > > >> > Well, the description says it all. To put it bluntly, when this line > >> > from wrapper.c > >> > sbi->s_vhdr_buf = kmalloc(hfsplus_min_io_size(sb), GFP_KERNEL); > >> > assigns s_vhdr_buf a value satisfying condition (PAGE_SIZE - (value & > >> > PAGE_SIZE) < 512) then this call (also from wrapper.c) returns an > >> > >> I'm sorry, the right condition is (PAGE_SIZE - (value & (PAGE_SIZE - 1)) < 512). > >> > >> > error: > >> > error = hfsplus_submit_bio(sb, part_start + HFSPLUS_VOLHEAD_SECTOR, > >> > sbi->s_vhdr_buf, (void **)&sbi->s_vhdr, > >> > READ); > >> > > >> > To give a specific example, sbi->s_vhdr_buf equal to > >> > 0xffff8804085acec0 spans two pages and hfsplus_submit_bio() can not > >> > read into such a buffer, returns an error, mount operation fails. > > > > How an ordinary user can discover this issue? Could you describe a real > > use-case for the reproducing? > > I discovered it by running "mount " command. > It was working probabilistically: sometimes it mounted the partition, > sometimes not. If course, the driver was not specially tweaked, it was > just a compiled source code from Linus' git tree. Could you describe your environment with more details? What partition do you have? How did it created? What option did you use for partition creation? What mount option did you use? What Linux version did you use? Thanks, Vyacheslav Dubeyko.