lkml.org 
[lkml]   [2023]   [May]   [17]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
Date
SubjectRe: [PATCH 3/6] PKEY: Apply PKEY_ENFORCE_API to mprotect
On Tue, May 16, 2023 at 4:37 PM Jeff Xu <jeffxu@google.com> wrote:
>
> On Tue, May 16, 2023 at 4:19 PM Dave Hansen <dave.hansen@intel.com> wrote:
> >
> > On 5/15/23 06:05, jeffxu@chromium.org wrote:
> > > /*
> > > * pkey==-1 when doing a legacy mprotect()
> > > + * syscall==true if this is called by syscall from userspace.
> > > + * Note: this is always true for now, added as a reminder in case that
> > > + * do_mprotect_pkey is called directly by kernel in the future.
> > > + * Also it is consistent with __do_munmap().
> > > */
> > > static int do_mprotect_pkey(unsigned long start, size_t len,
> > > - unsigned long prot, int pkey)
> > > + unsigned long prot, int pkey, bool syscall)
> > > {
> >
> > The 'syscall' seems kinda silly (and a bit confusing). It's easy to
> > check if the caller is a kthread or has a current->mm==NULL. If you
> > *really* want a warning, I'd check for those rather than plumb a
> > apparently unused argument in here.
> >
> > BTW, this warning is one of those things that will probably cause some
> > amount of angst. I'd move it to the end of the series or just axe it
> > completely.
>
Okay, I will move the logging part to the end of the series.


> Agreed. syscall is not a good name here.
> The intention is to check this at the system call entry point
> For example, munmap can get called inside mremap(), but by that time
> mremap() should already check that all the memory is writeable.
>
> I will remove "syscall" from do_mprotect_pkey signature, it seems it caused
> more confusion than helpful. I will keep the comments/note in place to remind
> future developer.

\
 
 \ /
  Last update: 2023-05-17 06:52    [W:0.098 / U:0.048 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site