linux-kernel - Re: [PATCH v4 3/9] drivers/firewire/core-iso.c: Convert to use vm_insert

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CAFqt6zZw2jO5qGjG3CdAZR2ZXt19+ykhyw+hhVDP7xTaemusiA@mail.gmail.com>
Date:   Wed, 19 Dec 2018 12:54:18 +0530
From:   Souptick Joarder <jrdr.linux@...il.com>
To:     Russell King - ARM Linux <linux@...linux.org.uk>
Cc:     Andrew Morton <akpm@...ux-foundation.org>,
        Matthew Wilcox <willy@...radead.org>,
        Michal Hocko <mhocko@...e.com>, stefanr@...6.in-berlin.de,
        Linux-MM <linux-mm@...ck.org>,
        linux1394-devel@...ts.sourceforge.net,
        linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org
Subject: Re: [PATCH v4 3/9] drivers/firewire/core-iso.c: Convert to use vm_insert_range

On Tue, Dec 18, 2018 at 4:15 PM Russell King - ARM Linux
<linux@...linux.org.uk> wrote:
>
> On Tue, Dec 18, 2018 at 01:52:46AM +0530, Souptick Joarder wrote:
> > Convert to use vm_insert_range to map range of kernel memory
> > to user vma.
> >
> > Signed-off-by: Souptick Joarder <jrdr.linux@...il.com>
> > Reviewed-by: Matthew Wilcox <willy@...radead.org>
> > ---
> >  drivers/firewire/core-iso.c | 15 ++-------------
> >  1 file changed, 2 insertions(+), 13 deletions(-)
> >
> > diff --git a/drivers/firewire/core-iso.c b/drivers/firewire/core-iso.c
> > index 35e784c..7bf28bb 100644
> > --- a/drivers/firewire/core-iso.c
> > +++ b/drivers/firewire/core-iso.c
> > @@ -107,19 +107,8 @@ int fw_iso_buffer_init(struct fw_iso_buffer *buffer, struct fw_card *card,
> >  int fw_iso_buffer_map_vma(struct fw_iso_buffer *buffer,
> >                         struct vm_area_struct *vma)
> >  {
> > -     unsigned long uaddr;
> > -     int i, err;
> > -
> > -     uaddr = vma->vm_start;
> > -     for (i = 0; i < buffer->page_count; i++) {
> > -             err = vm_insert_page(vma, uaddr, buffer->pages[i]);
> > -             if (err)
> > -                     return err;
> > -
> > -             uaddr += PAGE_SIZE;
> > -     }
> > -
> > -     return 0;
> > +     return vm_insert_range(vma, vma->vm_start, buffer->pages,
> > +                             buffer->page_count);
>
> This looks functionally equivalent.  Note that if we go with my
> proposal to your patch 4, that would cause an issue for this
> implementation.
>
> Maybe we need two functions, but that then causes problems with
> which function should be used (which makes it easy to get wrong.)

I think, apart from patch [4/9] and [6/9], all others places can be
directly replaced
with vm_insert_range(). [4/9] and [6/9] are the places where
*vma->vm_pgoff* need to be
considered and need to adjust *count* accordingly. In my opinion, bugs
around these
[4/9] & [6/9] can be fixed (raised during review) to accommodate it to
use vm_insert_range().
>
> I'm beginning to wonder if the risks of causing regressions and
> introducing bugs is actually worth the effort of trying to clean
> this up.
>
> --
> RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
> FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
> According to speedtest.net: 11.9Mbps down 500kbps up