Re: [PATCH] hfsplus: fix cross-page bio requests

[Viacheslav Dubeyko <slava@dubeyko.com>]

On Mon, 2015-06-08 at 19:20 +0200, Sergei Antonov wrote:
> On 8 June 2015 at 19:03, Viacheslav Dubeyko <slava@dubeyko.com> wrote:
> > On Sun, 2015-06-07 at 22:09 +0200, Sergei Antonov wrote:
> >> On 7 June 2015 at 22:05, Sergei Antonov <saproj@gmail.com> wrote:
> >> > On 10 April 2015 at 18:48, Viacheslav Dubeyko <slava@dubeyko.com> 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 <partition> <mountpoint>" 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.