linux-kernel - Re: [Nouveau] [RESEND PATCH 1/3] nouveau: fix migrate page regression

Open Source and information security mailing list archives

Message-ID: <CACAvsv5NqqF704HyvTd6DH43-one06OO7Z=uENUS5RSOJzP8Jg@mail.gmail.com>
Date:   Thu, 25 Jun 2020 15:23:48 +1000
From:   Ben Skeggs <skeggsb@...il.com>
To:     John Hubbard <jhubbard@...dia.com>
Cc:     Ralph Campbell <rcampbell@...dia.com>,
        ML nouveau <nouveau@...ts.freedesktop.org>,
        LKML <linux-kernel@...r.kernel.org>,
        Jason Gunthorpe <jgg@...lanox.com>,
        Christoph Hellwig <hch@....de>, Ben Skeggs <bskeggs@...hat.com>
Subject: Re: [Nouveau] [RESEND PATCH 1/3] nouveau: fix migrate page regression

On Tue, 23 Jun 2020 at 10:51, John Hubbard <jhubbard@...dia.com> wrote:
>
> On 2020-06-22 16:38, Ralph Campbell wrote:
> > The patch to add zero page migration to GPU memory inadvertantly included
>
> inadvertently
>
> > part of a future change which broke normal page migration to GPU memory
> > by copying too much data and corrupting GPU memory.
> > Fix this by only copying one page instead of a byte count.
> >
> > Fixes: 9d4296a7d4b3 ("drm/nouveau/nouveau/hmm: fix migrate zero page to GPU")
> > Signed-off-by: Ralph Campbell <rcampbell@...dia.com>
> > ---
> >   drivers/gpu/drm/nouveau/nouveau_dmem.c | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/nouveau/nouveau_dmem.c b/drivers/gpu/drm/nouveau/nouveau_dmem.c
> > index e5c230d9ae24..cc9993837508 100644
> > --- a/drivers/gpu/drm/nouveau/nouveau_dmem.c
> > +++ b/drivers/gpu/drm/nouveau/nouveau_dmem.c
> > @@ -550,7 +550,7 @@ static unsigned long nouveau_dmem_migrate_copy_one(struct nouveau_drm *drm,
> >                                        DMA_BIDIRECTIONAL);
> >               if (dma_mapping_error(dev, *dma_addr))
> >                       goto out_free_page;
> > -             if (drm->dmem->migrate.copy_func(drm, page_size(spage),
> > +             if (drm->dmem->migrate.copy_func(drm, 1,
> >                       NOUVEAU_APER_VRAM, paddr, NOUVEAU_APER_HOST, *dma_addr))
> >                       goto out_dma_unmap;
> >       } else {
> >
>
>
> I Am Not A Nouveau Expert, nor is it really clear to me how
> page_size(spage) came to contain something other than a page's worth of
> byte count, but this fix looks accurate to me. It's better for
> maintenance, too, because the function never intends to migrate "some
> number of bytes". It intends to migrate exactly one page.
>
> Hope I'm not missing something fundamental, but:
I'm actually a bit confused here too.  Because, it *looks* like the
function takes a byte count, not a page count, and unless I'm missing
something too, it's setup the copy class for a byte count also.

>
> Reviewed-by: John Hubbard <jhubbard@...dia.com
>
>
> thanks,
> --
> John Hubbard
> NVIDIA
> _______________________________________________
> Nouveau mailing list
> Nouveau@...ts.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/nouveau

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