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] [thread-next>] [day] [month] [year] [list]
Date:   Thu, 6 Oct 2022 11:53:55 +0530
From:   Sumit Garg <sumit.garg@...aro.org>
To:     Phil Chang (張世勳) 
        <Phil.Chang@...iatek.com>
Cc:     "ira.weiny@...el.com" <ira.weiny@...el.com>,
        Jens Wiklander <jens.wiklander@...aro.org>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Al Viro <viro@...iv.linux.org.uk>,
        "Fabio M. De Francesco" <fmdefrancesco@...il.com>,
        Christoph Hellwig <hch@....de>,
        Linus Torvalds <torvalds@...ux-foundation.org>,
        "op-tee@...ts.trustedfirmware.org" <op-tee@...ts.trustedfirmware.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "linux-mm@...ck.org" <linux-mm@...ck.org>
Subject: Re: [PATCH 2/4] tee: Remove vmalloc page support

Hi Phil,

Please don't top-post in the OSS mailing list.

On Wed, 5 Oct 2022 at 08:59, Phil Chang (張世勳) <Phil.Chang@...iatek.com> wrote:
>
> Hi Sumit
>
> Thanks for mentioning that, in fact, our product is low memory devices, and continuous pages are extremely valuable.
> Although our driver is not upstream yet but highly dependent on tee shm vmalloc support,

Sorry but you need to get your driver mainline in order to support
vmalloc interface. As otherwise it's a maintenance nightmare to
support interfaces in the mainline for out-of-tree drivers.

-Sumit

> some scenarios are driver alloc high order pages but system memory is fragmentation so that alloc failed.
> In this situation, vmalloc support is important and gives flexible usage to user.
>
>
> -----Original Message-----
> From: Sumit Garg <sumit.garg@...aro.org>
> Sent: Monday, October 3, 2022 2:57 PM
> To: ira.weiny@...el.com
> Cc: Jens Wiklander <jens.wiklander@...aro.org>; Andrew Morton <akpm@...ux-foundation.org>; Al Viro <viro@...iv.linux.org.uk>; Fabio M. De Francesco <fmdefrancesco@...il.com>; Christoph Hellwig <hch@....de>; Linus Torvalds <torvalds@...ux-foundation.org>; op-tee@...ts.trustedfirmware.org; linux-kernel@...r.kernel.org; linux-mm@...ck.org; Phil Chang (張世勳) <Phil.Chang@...iatek.com>
> Subject: Re: [PATCH 2/4] tee: Remove vmalloc page support
>
> + Phil
>
> Hi Ira,
>
> On Sun, 2 Oct 2022 at 05:53, <ira.weiny@...el.com> wrote:
> >
> > From: Ira Weiny <ira.weiny@...el.com>
> >
> > The kernel pages used by shm_get_kernel_pages() are allocated using
> > GFP_KERNEL through the following call stack:
> >
> > trusted_instantiate()
> >         trusted_payload_alloc() -> GFP_KERNEL
> >         <trusted key op>
> >                 tee_shm_register_kernel_buf()
> >                         register_shm_helper()
> >                                 shm_get_kernel_pages()
> >
> > Where <trusted key op> is one of:
> >
> >         trusted_key_unseal()
> >         trusted_key_get_random()
> >         trusted_key_seal()
> >
> > Remove the vmalloc page support from shm_get_kernel_pages().  Replace
> > with a warn on once.
> >
> > Cc: Jens Wiklander <jens.wiklander@...aro.org>
> > Cc: Al Viro <viro@...iv.linux.org.uk>
> > Cc: "Fabio M. De Francesco" <fmdefrancesco@...il.com>
> > Cc: Christoph Hellwig <hch@....de>
> > Cc: Linus Torvalds <torvalds@...ux-foundation.org>
> > Signed-off-by: Ira Weiny <ira.weiny@...el.com>
> >
> > ---
> > Jens I went with the suggestion from Linus and Christoph and rejected
> > vmalloc addresses.  I did not hear back from you regarding Linus'
> > question if the vmalloc page support was required by an up coming
> > patch set or not.  So I assumed it was something out of tree.
>
> It looks like I wasn't CC'd to that conversation. IIRC, support for vmalloc addresses was added recently by Phil here [1]. So I would like to give him a chance if he is planning to post a corresponding kernel driver upstream.
>
> [1] https://urldefense.com/v3/__https://lists.trustedfirmware.org/archives/list/op-tee@lists.trustedfirmware.org/thread/M7HI3P2M66V27SK35CGQRICZ7DJZ5J2W/__;!!CTRNKA9wMg0ARbw!wGOKR9k3khZJlPt1K_xBCXX4EBM5ZCfWKuruFgSP45H8wTvJrx4_St3Fb5ZrljD5QQ$
>
> -Sumit
>
> > ---
> >  drivers/tee/tee_shm.c | 36 ++++++++++++------------------------
> >  1 file changed, 12 insertions(+), 24 deletions(-)
> >
> > diff --git a/drivers/tee/tee_shm.c b/drivers/tee/tee_shm.c index
> > 27295bda3e0b..527a6eabc03e 100644
> > --- a/drivers/tee/tee_shm.c
> > +++ b/drivers/tee/tee_shm.c
> > @@ -24,37 +24,25 @@ static void shm_put_kernel_pages(struct page
> > **pages, size_t page_count)  static int shm_get_kernel_pages(unsigned long start, size_t page_count,
> >                                 struct page **pages)  {
> > +       struct kvec *kiov;
> >         size_t n;
> >         int rc;
> >
> > -       if (is_vmalloc_addr((void *)start)) {
> > -               struct page *page;
> > -
> > -               for (n = 0; n < page_count; n++) {
> > -                       page = vmalloc_to_page((void *)(start + PAGE_SIZE * n));
> > -                       if (!page)
> > -                               return -ENOMEM;
> > -
> > -                       get_page(page);
> > -                       pages[n] = page;
> > -               }
> > -               rc = page_count;
> > -       } else {
> > -               struct kvec *kiov;
> > -
> > -               kiov = kcalloc(page_count, sizeof(*kiov), GFP_KERNEL);
> > -               if (!kiov)
> > -                       return -ENOMEM;
> > +       if (WARN_ON_ONCE(is_vmalloc_addr((void *)start)))
> > +               return -EINVAL;
> >
> > -               for (n = 0; n < page_count; n++) {
> > -                       kiov[n].iov_base = (void *)(start + n * PAGE_SIZE);
> > -                       kiov[n].iov_len = PAGE_SIZE;
> > -               }
> > +       kiov = kcalloc(page_count, sizeof(*kiov), GFP_KERNEL);
> > +       if (!kiov)
> > +               return -ENOMEM;
> >
> > -               rc = get_kernel_pages(kiov, page_count, 0, pages);
> > -               kfree(kiov);
> > +       for (n = 0; n < page_count; n++) {
> > +               kiov[n].iov_base = (void *)(start + n * PAGE_SIZE);
> > +               kiov[n].iov_len = PAGE_SIZE;
> >         }
> >
> > +       rc = get_kernel_pages(kiov, page_count, 0, pages);
> > +       kfree(kiov);
> > +
> >         return rc;
> >  }
> >
> > --
> > 2.37.2
> >

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ