pgprot_encrypted macro is broken

pgprot_encrypted macro is broken

[Federico Di Pierro]

Hi everyone,

In our kmod we use the `pgprot_encrypted` macro.
It seems like the macro cannot be used on 5.18+ kernels because commit
b577f542f93cbba57f8d6185ef1fb13a41ddf162 broke it.

Basically, the macro definition was:
`__pgprot(__sme_set(pgprot_val(prot)))`

but after the commit, it was changed to:
`__pgprot(cc_mkenc(pgprot_val(prot)))`.

But `cc_mkenc` symbol is not exported!

This leads to build issues:
> ERROR: modpost: "cc_mkenc" undefined!

Is this a bug?
Is there any workaround?

Thank you,
Regards
Federico

Re: pgprot_encrypted macro is broken

[Jann Horn]

On Tue, Jun 14, 2022 at 3:15 PM Federico Di Pierro <nierro92@gmail.com> wrote:
> In our kmod we use the `pgprot_encrypted` macro.
> It seems like the macro cannot be used on 5.18+ kernels because commit
> b577f542f93cbba57f8d6185ef1fb13a41ddf162 broke it.
>
> Basically, the macro definition was:
> `__pgprot(__sme_set(pgprot_val(prot)))`
>
> but after the commit, it was changed to:
> `__pgprot(cc_mkenc(pgprot_val(prot)))`.
>
> But `cc_mkenc` symbol is not exported!
>
> This leads to build issues:
> > ERROR: modpost: "cc_mkenc" undefined!
>
> Is this a bug?
> Is there any workaround?

Why does your driver need to use that macro? pgprot_encrypted() is
mostly only directly used by core kernel code, not by drivers... and
if memory encryption is enabled, almost all memory mappings created by
the kernel should be marked as encrypted automatically.

Re: pgprot_encrypted macro is broken

[Federico Di Pierro]

> Why does your driver need to use that macro? pgprot_encrypted() is
> mostly only directly used by core kernel code, not by drivers... and
> if memory encryption is enabled, almost all memory mappings created by
> the kernel should be marked as encrypted automatically.

This is interesting; i don't really know the history behind our piece
of code; as far as i understand,
we have a shared ring buffer with userspace, onto which we push tracing events,
and we must mark it as encrypted when
the kmod runs on an AMD SME enabled kernel to allow userspace to grab sane data.

This is the commit that introduced the change (if you wish to give it a look):
https://github.com/falcosecurity/libs/commit/0333501cf429c045c61aaf5909812156f090786e

Do you see any workaround not involving `pgprot_encrypted` ?

Thank you very much for your answer, much appreciated!
Regards,
Federico

Re: pgprot_encrypted macro is broken

[Jann Horn]

On Mon, Jun 20, 2022 at 9:39 AM Federico Di Pierro <nierro92@gmail.com> wrote:
> > Why does your driver need to use that macro? pgprot_encrypted() is
> > mostly only directly used by core kernel code, not by drivers... and
> > if memory encryption is enabled, almost all memory mappings created by
> > the kernel should be marked as encrypted automatically.
>
> This is interesting; i don't really know the history behind our piece
> of code; as far as i understand,
> we have a shared ring buffer with userspace, onto which we push tracing events,
> and we must mark it as encrypted when
> the kmod runs on an AMD SME enabled kernel to allow userspace to grab sane data.
>
> This is the commit that introduced the change (if you wish to give it a look):
> https://github.com/falcosecurity/libs/commit/0333501cf429c045c61aaf5909812156f090786e
>
> Do you see any workaround not involving `pgprot_encrypted` ?

If you do have to use remap_pfn_range() to map normal kernel memory,
then you might want to use vma->vm_page_prot instead, like a few other
places in the kernel do.

(Alternatively you might want to use remap_vmalloc_range() to map
vmalloc pages into userspace, but note that that has very different
semantics - I believe that installs a normal page reference rather
than a raw PFN reference, so that would permit get_user_pages() calls
on the range.)

Re: pgprot_encrypted macro is broken

[Federico Di Pierro]

Hi!

Thank you very much for your hints and for your time!
I solved the issue and I agree that we should not have used that macro
in the first place.

Again, thank you very much for your help,
Regards
Federico

Il giorno lun 20 giu 2022 alle ore 13:32 Jann Horn <jannh@google.com>
ha scritto:
>
> On Mon, Jun 20, 2022 at 9:39 AM Federico Di Pierro <nierro92@gmail.com> wrote:
> > > Why does your driver need to use that macro? pgprot_encrypted() is
> > > mostly only directly used by core kernel code, not by drivers... and
> > > if memory encryption is enabled, almost all memory mappings created by
> > > the kernel should be marked as encrypted automatically.
> >
> > This is interesting; i don't really know the history behind our piece
> > of code; as far as i understand,
> > we have a shared ring buffer with userspace, onto which we push tracing events,
> > and we must mark it as encrypted when
> > the kmod runs on an AMD SME enabled kernel to allow userspace to grab sane data.
> >
> > This is the commit that introduced the change (if you wish to give it a look):
> > https://github.com/falcosecurity/libs/commit/0333501cf429c045c61aaf5909812156f090786e
> >
> > Do you see any workaround not involving `pgprot_encrypted` ?
>
> If you do have to use remap_pfn_range() to map normal kernel memory,
> then you might want to use vma->vm_page_prot instead, like a few other
> places in the kernel do.
>
> (Alternatively you might want to use remap_vmalloc_range() to map
> vmalloc pages into userspace, but note that that has very different
> semantics - I believe that installs a normal page reference rather
> than a raw PFN reference, so that would permit get_user_pages() calls
> on the range.)