The use of kmap_atomic() is being deprecated in favor of kmap_local_page().
With kmap_local_page(), the mappings are per thread, CPU local, not
globally visible and can take page faults. Furthermore, the mappings can be
acquired from any context (including interrupts).
Therefore, use kmap_local_page() in copy_string_kernel() instead of
kmap_atomic().
Tested with xfstests on a QEMU + KVM 32-bits VM booting a kernel with
HIGHMEM64GB enabled.
Suggested-by: Ira Weiny <[email protected]>
Signed-off-by: Fabio M. De Francesco <[email protected]>
---
I sent a first patch to fs/exec.c for converting kmap() and kmap_atomic()
to kmap_local_page():
https://lore.kernel.org/lkml/[email protected]/
Some days ago, Ira Weiny, while he was reviewing that patch, made me notice
that I had overlooked a second kmap_atomic() in the same file (thanks):
https://lore.kernel.org/lkml/YsiQptk19txHrG4c@iweiny-desk3/
I've been asked to send this as an additional change. This is why there will
not be any second version of that previous patch.
fs/exec.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/fs/exec.c b/fs/exec.c
index 4a2129c0d422..5fa652ca5823 100644
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -639,11 +639,11 @@ int copy_string_kernel(const char *arg, struct linux_binprm *bprm)
page = get_arg_page(bprm, pos, 1);
if (!page)
return -E2BIG;
- kaddr = kmap_atomic(page);
+ kaddr = kmap_local_page(page);
flush_arg_page(bprm, pos & PAGE_MASK, page);
memcpy(kaddr + offset_in_page(pos), arg, bytes_to_copy);
flush_dcache_page(page);
- kunmap_atomic(kaddr);
+ kunmap_local(kaddr);
put_arg_page(page);
}
--
2.36.1
On Sun, Jul 10, 2022 at 12:01:36PM +0200, Fabio M. De Francesco wrote:
> The use of kmap_atomic() is being deprecated in favor of kmap_local_page().
>
> With kmap_local_page(), the mappings are per thread, CPU local, not
> globally visible and can take page faults. Furthermore, the mappings can be
> acquired from any context (including interrupts).
>
> Therefore, use kmap_local_page() in copy_string_kernel() instead of
> kmap_atomic().
>
> Tested with xfstests on a QEMU + KVM 32-bits VM booting a kernel with
> HIGHMEM64GB enabled.
>
> Suggested-by: Ira Weiny <[email protected]>
> Signed-off-by: Fabio M. De Francesco <[email protected]>
> ---
>
> I sent a first patch to fs/exec.c for converting kmap() and kmap_atomic()
> to kmap_local_page():
> https://lore.kernel.org/lkml/[email protected]/
>
> Some days ago, Ira Weiny, while he was reviewing that patch, made me notice
> that I had overlooked a second kmap_atomic() in the same file (thanks):
> https://lore.kernel.org/lkml/YsiQptk19txHrG4c@iweiny-desk3/
>
> I've been asked to send this as an additional change. This is why there will
> not be any second version of that previous patch.
>
> fs/exec.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/fs/exec.c b/fs/exec.c
> index 4a2129c0d422..5fa652ca5823 100644
> --- a/fs/exec.c
> +++ b/fs/exec.c
> @@ -639,11 +639,11 @@ int copy_string_kernel(const char *arg, struct linux_binprm *bprm)
> page = get_arg_page(bprm, pos, 1);
> if (!page)
> return -E2BIG;
> - kaddr = kmap_atomic(page);
> + kaddr = kmap_local_page(page);
> flush_arg_page(bprm, pos & PAGE_MASK, page);
I really question why we can't use memcpy_to_page() here and move the
flush_arg_page() prior to the mapping?
flush_arg_page() only calls flush_cache_page() which does not need the
mapping to work correctly AFAICT.
Ira
> memcpy(kaddr + offset_in_page(pos), arg, bytes_to_copy);
> flush_dcache_page(page);
> - kunmap_atomic(kaddr);
> + kunmap_local(kaddr);
> put_arg_page(page);
> }
>
> --
> 2.36.1
>
On venerdì 22 luglio 2022 02:14:20 CEST Ira Weiny wrote:
> On Sun, Jul 10, 2022 at 12:01:36PM +0200, Fabio M. De Francesco wrote:
> > The use of kmap_atomic() is being deprecated in favor of
kmap_local_page().
> >
> > With kmap_local_page(), the mappings are per thread, CPU local, not
> > globally visible and can take page faults. Furthermore, the mappings
can be
> > acquired from any context (including interrupts).
> >
> > Therefore, use kmap_local_page() in copy_string_kernel() instead of
> > kmap_atomic().
> >
> > Tested with xfstests on a QEMU + KVM 32-bits VM booting a kernel with
> > HIGHMEM64GB enabled.
> >
> > Suggested-by: Ira Weiny <[email protected]>
> > Signed-off-by: Fabio M. De Francesco <[email protected]>
> > ---
> >
> > I sent a first patch to fs/exec.c for converting kmap() and
kmap_atomic()
> > to kmap_local_page():
> > https://lore.kernel.org/lkml/[email protected]/
> >
> > Some days ago, Ira Weiny, while he was reviewing that patch, made me
notice
> > that I had overlooked a second kmap_atomic() in the same file (thanks):
> > https://lore.kernel.org/lkml/YsiQptk19txHrG4c@iweiny-desk3/
> >
> > I've been asked to send this as an additional change. This is why there
will
> > not be any second version of that previous patch.
> >
> > fs/exec.c | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/fs/exec.c b/fs/exec.c
> > index 4a2129c0d422..5fa652ca5823 100644
> > --- a/fs/exec.c
> > +++ b/fs/exec.c
> > @@ -639,11 +639,11 @@ int copy_string_kernel(const char *arg, struct
linux_binprm *bprm)
> > page = get_arg_page(bprm, pos, 1);
> > if (!page)
> > return -E2BIG;
> > - kaddr = kmap_atomic(page);
> > + kaddr = kmap_local_page(page);
> > flush_arg_page(bprm, pos & PAGE_MASK, page);
>
> I really question why we can't use memcpy_to_page() here and move the
> flush_arg_page() prior to the mapping?
>
> flush_arg_page() only calls flush_cache_page() which does not need the
> mapping to work correctly AFAICT.
You're right here. I'm sorry for being so lazy and not checking that
flush_arg_page() does not need to be called while the task holds the local
mapping :-(
In v2 I'll move flush_arg_page() one line above memcpy_to_page().
Thanks for your comment,
Fabio
>
> Ira
>
> > memcpy(kaddr + offset_in_page(pos), arg,
bytes_to_copy);
> > flush_dcache_page(page);
> > - kunmap_atomic(kaddr);
> > + kunmap_local(kaddr);
> > put_arg_page(page);
> > }
> >
> > --
> > 2.36.1
> >
>