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

[Viacheslav Dubeyko <slava@dubeyko.com>]

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? Maybe you can share some guess how it can
occur?

If such situation doesn't take place in the real life then it doesn't
make sense to fix it. Please, prove that your fix is valid.

Thanks,
Vyacheslav Dubeyko.