[PATCH] pagewalk: don't pte_unmap(NULL) in walk_pte_range() - Kernel

This is a discussion on [PATCH] pagewalk: don't pte_unmap(NULL) in walk_pte_range() - Kernel ; This is right isn't it? --- Don't pte_unmap a NULL pointer, but the previous. Signed-off-by: Roel Kluin --- diff --git a/mm/pagewalk.c b/mm/pagewalk.c index 1cf1417..6615f0b 100644 --- a/mm/pagewalk.c +++ b/mm/pagewalk.c @@ -15,7 +15,7 @@ static int walk_pte_range(pmd_t *pmd, unsigned long addr, ...

+ Reply to Thread
Results 1 to 5 of 5

Thread: [PATCH] pagewalk: don't pte_unmap(NULL) in walk_pte_range()

  1. [PATCH] pagewalk: don't pte_unmap(NULL) in walk_pte_range()

    This is right isn't it?
    ---
    Don't pte_unmap a NULL pointer, but the previous.

    Signed-off-by: Roel Kluin <12o3l@tiscali.nl>
    ---
    diff --git a/mm/pagewalk.c b/mm/pagewalk.c
    index 1cf1417..6615f0b 100644
    --- a/mm/pagewalk.c
    +++ b/mm/pagewalk.c
    @@ -15,7 +15,7 @@ static int walk_pte_range(pmd_t *pmd, unsigned long addr, unsigned long end,
    break;
    } while (pte++, addr += PAGE_SIZE, addr != end);

    - pte_unmap(pte);
    + pte_unmap(pte - 1);
    return err;
    }

    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  2. Re: [PATCH] pagewalk: don't pte_unmap(NULL) in walk_pte_range()

    Hi,

    Roel Kluin <12o3l@tiscali.nl> writes:

    > This is right isn't it?
    > ---
    > Don't pte_unmap a NULL pointer, but the previous.


    Which NULL pointer?

    > Signed-off-by: Roel Kluin <12o3l@tiscali.nl>
    > ---
    > diff --git a/mm/pagewalk.c b/mm/pagewalk.c
    > index 1cf1417..6615f0b 100644
    > --- a/mm/pagewalk.c
    > +++ b/mm/pagewalk.c
    > @@ -15,7 +15,7 @@ static int walk_pte_range(pmd_t *pmd, unsigned long addr, unsigned long end,
    > break;
    > } while (pte++, addr += PAGE_SIZE, addr != end);
    >
    > - pte_unmap(pte);
    > + pte_unmap(pte - 1);
    > return err;
    > }


    This does not make any sense to me.

    Hannes
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  3. Re: [PATCH] pagewalk: don't pte_unmap(NULL) in walk_pte_range()

    Johannes Weiner wrote:
    > Hi,
    >
    > Roel Kluin <12o3l@tiscali.nl> writes:
    >
    >> This is right isn't it?
    >> ---
    >> Don't pte_unmap a NULL pointer, but the previous.

    >
    > Which NULL pointer?
    >
    >> Signed-off-by: Roel Kluin <12o3l@tiscali.nl>
    >> ---
    >> diff --git a/mm/pagewalk.c b/mm/pagewalk.c
    >> index 1cf1417..6615f0b 100644
    >> --- a/mm/pagewalk.c
    >> +++ b/mm/pagewalk.c
    >> @@ -15,7 +15,7 @@ static int walk_pte_range(pmd_t *pmd, unsigned long addr, unsigned long end,
    >> break;
    >> } while (pte++, addr += PAGE_SIZE, addr != end);
    >>
    >> - pte_unmap(pte);
    >> + pte_unmap(pte - 1);
    >> return err;
    >> }

    >
    > This does not make any sense to me.


    you are right, please ignore.

    > Hannes


    thanks,

    Roel
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  4. Re: [PATCH] pagewalk: don't pte_unmap(NULL) in walk_pte_range()

    Johannes Weiner writes:

    >> Signed-off-by: Roel Kluin <12o3l@tiscali.nl>
    >> ---
    >> diff --git a/mm/pagewalk.c b/mm/pagewalk.c
    >> index 1cf1417..6615f0b 100644
    >> --- a/mm/pagewalk.c
    >> +++ b/mm/pagewalk.c
    >> @@ -15,7 +15,7 @@ static int walk_pte_range(pmd_t *pmd, unsigned long addr, unsigned long end,
    >> break;
    >> } while (pte++, addr += PAGE_SIZE, addr != end);
    >>
    >> - pte_unmap(pte);
    >> + pte_unmap(pte - 1);
    >> return err;
    >> }

    >
    > This does not make any sense to me.


    There is something fishy here. If the loop ends because addr == end
    then pte has been incremented past the pmd page for addr, no?

    Andreas.

    --
    Andreas Schwab, SuSE Labs, schwab@suse.de
    SuSE Linux Products GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany
    PGP key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5
    "And now for something completely different."
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  5. [PATCH] Stay below upper pmd boundary on pte range walk

    Hi,

    Andreas Schwab writes:

    > Johannes Weiner writes:
    >
    >>> Signed-off-by: Roel Kluin <12o3l@tiscali.nl>
    >>> ---
    >>> diff --git a/mm/pagewalk.c b/mm/pagewalk.c
    >>> index 1cf1417..6615f0b 100644
    >>> --- a/mm/pagewalk.c
    >>> +++ b/mm/pagewalk.c
    >>> @@ -15,7 +15,7 @@ static int walk_pte_range(pmd_t *pmd, unsigned long addr, unsigned long end,
    >>> break;
    >>> } while (pte++, addr += PAGE_SIZE, addr != end);
    >>>
    >>> - pte_unmap(pte);
    >>> + pte_unmap(pte - 1);
    >>> return err;
    >>> }

    >>
    >> This does not make any sense to me.

    >
    > There is something fishy here. If the loop ends because addr == end
    > then pte has been incremented past the pmd page for addr, no?


    Whoops, yes. But Roel's fix breaks if the break is taken in the first
    iteration of the loop, because the pte is then out of the lower bounds
    of the pmd page. Please see attached fix.

    Hannes

    ---

    After the loop in walk_pte_range() pte might point to the first address
    after the pmd it walks. The pte_unmap() is then applied to something
    bad.

    Spotted by Roel Kluin and Andreas Schwab.

    Signed-off-by: Johannes Weiner
    ---

    A bug is unlikely, though. kunmap_atomic() looks up the kmap entry by
    map-type instead of the address the pte points. So the worst thing I
    could find with a quick grep was that a wrong TLB entry is being
    flushed. Still, the code is wrong

    diff --git a/mm/pagewalk.c b/mm/pagewalk.c
    index 1cf1417..cf3c004 100644
    --- a/mm/pagewalk.c
    +++ b/mm/pagewalk.c
    @@ -13,7 +13,7 @@ static int walk_pte_range(pmd_t *pmd, unsigned long addr, unsigned long end,
    err = walk->pte_entry(pte, addr, addr + PAGE_SIZE, private);
    if (err)
    break;
    - } while (pte++, addr += PAGE_SIZE, addr != end);
    + } while (addr += PAGE_SIZE, addr != end && pte++);

    pte_unmap(pte);
    return err;
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

+ Reply to Thread