On Wed, Jan 25, 2023 at 1:04 PM Suren Baghdasaryan <surenb@xxxxxxxxxx> wrote:
>
> On Wed, Jan 25, 2023 at 12:35 PM Matthew Wilcox <willy@xxxxxxxxxxxxx> wrote:
> >
> > [I'm just going to trim the incredibly long list of recipients. Too
> > many bounces, and I doubt any of them really care]
> >
> > On Wed, Jan 25, 2023 at 11:21:56AM -0800, Suren Baghdasaryan wrote:
> > > On Wed, Jan 25, 2023 at 10:37 AM Matthew Wilcox <willy@xxxxxxxxxxxxx> wrote:
> > > > Here's a trick I saw somewhere in the VFS:
> > > >
> > > > union {
> > > > const vm_flags_t vm_flags;
> > > > vm_flags_t __private __vm_flags;
> > > > };
> > > >
> > > > Now it can be read by anybody but written only by those using
> > > > ACCESS_PRIVATE.
> > >
> > > Huh, this is quite nice! I think it does not save us from the cases
> > > when vma->vm_flags is passed by a reference and modified indirectly,
> > > like in ksm_madvise()? Though maybe such usecases are so rare (I found
> > > only 2 cases) that we can ignore this?
> >
> > Taking the address of vma->vm_flags will give you a const-qualified
> > pointer, which gcc will then warn about passing to a non-const-qualified
> > function argument, so I think we're good?
>
> Yes, actually I just realized that too when trying to use
> &ACCESS_PRIVATE() to get the address of vm->vm_flags in dump_vma().
> So, I think your trick gives us an easy way to have a read-only
> vm_flags. Thanks!
Another nasty problem I hit is that READ_ONCE() requires an l-value,
so READ_ONCE(vma->vm_flags) would not be directly convertible into
READ_ONCE(get_vm_flags(vma->vm_flags)). I'm not sure if
ACCESS_PRIVATE() implicitly constitutes READ_ONCE(). With your
approach that would not be needed but I'm still curious how this issue
could be solved.
>
> >
