linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Jerome Glisse <jglisse@redhat.com>
To: Andrew Morton <akpm@linux-foundation.org>
Cc: linux-mm@kvack.org, linux-kernel@vger.kernel.org,
	Ralph Campbell <rcampbell@nvidia.com>
Subject: Re: [PATCH] mm/hmm: fix uninitialized use of 'entry' in hmm_vma_walk_pmd()
Date: Mon, 22 Jan 2018 16:56:40 -0500	[thread overview]
Message-ID: <20180122215640.GB5522@redhat.com> (raw)
In-Reply-To: <20180122125836.1aebb001d4c2c4e93029db35@linux-foundation.org>

On Mon, Jan 22, 2018 at 12:58:36PM -0800, Andrew Morton wrote:
> On Mon, 22 Jan 2018 13:57:59 -0500 jglisse@redhat.com wrote:
> 
> > From: Ralph Campbell <rcampbell@nvidia.com>
> > 
> > The variable 'entry' is used before being initialized in
> > hmm_vma_walk_pmd()
> > 
> > ...
> >
> > --- a/mm/hmm.c
> > +++ b/mm/hmm.c
> > @@ -418,7 +418,7 @@ static int hmm_vma_walk_pmd(pmd_t *pmdp,
> >  		}
> >  
> >  		if (!pte_present(pte)) {
> > -			swp_entry_t entry;
> > +			swp_entry_t entry = pte_to_swp_entry(pte);
> >  
> >  			if (!non_swap_entry(entry)) {
> >  				if (hmm_vma_walk->fault)
> > @@ -426,8 +426,6 @@ static int hmm_vma_walk_pmd(pmd_t *pmdp,
> >  				continue;
> >  			}
> >  
> > -			entry = pte_to_swp_entry(pte);
> > -
> >  			/*
> >  			 * This is a special swap entry, ignore migration, use
> >  			 * device and report anything else as error.
> 
> Gee, how did that sneak through.  gcc not clever enough...
> 
> I'll add a cc:stable to this, even though the changelog didn't tell us what
> the runtime effects of the bug are.  It should do so, so can you please
> send us that description and I will add it, thanks.
> 

No bad effect (beside performance hit) so !non_swap_entry(0) evaluate to
true which trigger a fault as if CPU was trying to access migrated memory
and migrate memory back from device memory to regular memory.

This function (hmm_vma_walk_pmd()) is call when device driver tries to
populate its own page table. For migrated memory it should not happen as
the device driver should already have populated its page table correctly
during the migration.

Only case i can think of is multi-GPU where a second GPU trigger migration
back to regular memory. Again this would just result in a performance hit,
nothing bad would happen.


(I will try to keep in mind to always add a more in depth analysis even
for small patch :))

Cheers,
Jerome

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

      reply	other threads:[~2018-01-22 21:56 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-01-22 18:57 jglisse
2018-01-22 20:58 ` Andrew Morton
2018-01-22 21:56   ` Jerome Glisse [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180122215640.GB5522@redhat.com \
    --to=jglisse@redhat.com \
    --cc=akpm@linux-foundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=rcampbell@nvidia.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox