| Message ID | 20210514172247.176750-5-david@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | fs/proc/kcore: don't read offline sections, logically offline pages and hwpoisoned pages | expand |
On Fri, May 14, 2021 at 07:22:45PM +0200, David Hildenbrand wrote: > A driver might set a page logically offline -- PageOffline() -- and > turn the page inaccessible in the hypervisor; after that, access to page > content can be fatal. One example is virtio-mem; while unplugged memory > -- marked as PageOffline() can currently be read in the hypervisor, this > will no longer be the case in the future; for example, when having > a virtio-mem device backed by huge pages in the hypervisor. > > Some special PFN walkers -- i.e., /proc/kcore -- read content of random > pages after checking PageOffline(); however, these PFN walkers can race > with drivers that set PageOffline(). > > Let's introduce page_offline_(begin|end|freeze|thaw) for > synchronizing. > > page_offline_freeze()/page_offline_thaw() allows for a subsystem to > synchronize with such drivers, achieving that a page cannot be set > PageOffline() while frozen. > > page_offline_begin()/page_offline_end() is used by drivers that care about > such races when setting a page PageOffline(). > > For simplicity, use a rwsem for now; neither drivers nor users are > performance sensitive. > > Acked-by: Michal Hocko <mhocko@suse.com> > Signed-off-by: David Hildenbrand <david@redhat.com> One nit below, otherwise Reviewed-by: Mike Rapoport <rppt@linux.ibm.com> > --- > include/linux/page-flags.h | 10 ++++++++++ > mm/util.c | 40 ++++++++++++++++++++++++++++++++++++++ > 2 files changed, 50 insertions(+) > > diff --git a/include/linux/page-flags.h b/include/linux/page-flags.h > index daed82744f4b..ea2df9a247b3 100644 > --- a/include/linux/page-flags.h > +++ b/include/linux/page-flags.h > @@ -769,9 +769,19 @@ PAGE_TYPE_OPS(Buddy, buddy) > * relies on this feature is aware that re-onlining the memory block will > * require to re-set the pages PageOffline() and not giving them to the > * buddy via online_page_callback_t. > + * > + * There are drivers that mark a page PageOffline() and do not expect any Maybe "and expect there won't be any further access"... > + * further access to page content. PFN walkers that read content of random > + * pages should check PageOffline() and synchronize with such drivers using > + * page_offline_freeze()/page_offline_thaw(). > */ > PAGE_TYPE_OPS(Offline, offline) > > +extern void page_offline_freeze(void); > +extern void page_offline_thaw(void); > +extern void page_offline_begin(void); > +extern void page_offline_end(void); > + > /* > * Marks pages in use as page tables. > */ > diff --git a/mm/util.c b/mm/util.c > index a8bf17f18a81..a034525e7ba2 100644 > --- a/mm/util.c > +++ b/mm/util.c > @@ -1010,3 +1010,43 @@ void mem_dump_obj(void *object) > } > EXPORT_SYMBOL_GPL(mem_dump_obj); > #endif > + > +/* > + * A driver might set a page logically offline -- PageOffline() -- and > + * turn the page inaccessible in the hypervisor; after that, access to page > + * content can be fatal. > + * > + * Some special PFN walkers -- i.e., /proc/kcore -- read content of random > + * pages after checking PageOffline(); however, these PFN walkers can race > + * with drivers that set PageOffline(). > + * > + * page_offline_freeze()/page_offline_thaw() allows for a subsystem to > + * synchronize with such drivers, achieving that a page cannot be set > + * PageOffline() while frozen. > + * > + * page_offline_begin()/page_offline_end() is used by drivers that care about > + * such races when setting a page PageOffline(). > + */ > +static DECLARE_RWSEM(page_offline_rwsem); > + > +void page_offline_freeze(void) > +{ > + down_read(&page_offline_rwsem); > +} > + > +void page_offline_thaw(void) > +{ > + up_read(&page_offline_rwsem); > +} > + > +void page_offline_begin(void) > +{ > + down_write(&page_offline_rwsem); > +} > +EXPORT_SYMBOL(page_offline_begin); > + > +void page_offline_end(void) > +{ > + up_write(&page_offline_rwsem); > +} > +EXPORT_SYMBOL(page_offline_end); > -- > 2.31.1 >
On 17.05.21 08:43, Mike Rapoport wrote: > On Fri, May 14, 2021 at 07:22:45PM +0200, David Hildenbrand wrote: >> A driver might set a page logically offline -- PageOffline() -- and >> turn the page inaccessible in the hypervisor; after that, access to page >> content can be fatal. One example is virtio-mem; while unplugged memory >> -- marked as PageOffline() can currently be read in the hypervisor, this >> will no longer be the case in the future; for example, when having >> a virtio-mem device backed by huge pages in the hypervisor. >> >> Some special PFN walkers -- i.e., /proc/kcore -- read content of random >> pages after checking PageOffline(); however, these PFN walkers can race >> with drivers that set PageOffline(). >> >> Let's introduce page_offline_(begin|end|freeze|thaw) for >> synchronizing. >> >> page_offline_freeze()/page_offline_thaw() allows for a subsystem to >> synchronize with such drivers, achieving that a page cannot be set >> PageOffline() while frozen. >> >> page_offline_begin()/page_offline_end() is used by drivers that care about >> such races when setting a page PageOffline(). >> >> For simplicity, use a rwsem for now; neither drivers nor users are >> performance sensitive. >> >> Acked-by: Michal Hocko <mhocko@suse.com> >> Signed-off-by: David Hildenbrand <david@redhat.com> > > One nit below, otherwise > > Reviewed-by: Mike Rapoport <rppt@linux.ibm.com> > >> --- >> include/linux/page-flags.h | 10 ++++++++++ >> mm/util.c | 40 ++++++++++++++++++++++++++++++++++++++ >> 2 files changed, 50 insertions(+) >> >> diff --git a/include/linux/page-flags.h b/include/linux/page-flags.h >> index daed82744f4b..ea2df9a247b3 100644 >> --- a/include/linux/page-flags.h >> +++ b/include/linux/page-flags.h >> @@ -769,9 +769,19 @@ PAGE_TYPE_OPS(Buddy, buddy) >> * relies on this feature is aware that re-onlining the memory block will >> * require to re-set the pages PageOffline() and not giving them to the >> * buddy via online_page_callback_t. >> + * >> + * There are drivers that mark a page PageOffline() and do not expect any > > Maybe "and expect there won't be any further access"... > Thanks, makes sense. I'll wait a bit before I resend.
On Fri, May 14, 2021 at 07:22:45PM +0200, David Hildenbrand wrote: > A driver might set a page logically offline -- PageOffline() -- and > turn the page inaccessible in the hypervisor; after that, access to page > content can be fatal. One example is virtio-mem; while unplugged memory > -- marked as PageOffline() can currently be read in the hypervisor, this > will no longer be the case in the future; for example, when having > a virtio-mem device backed by huge pages in the hypervisor. > > Some special PFN walkers -- i.e., /proc/kcore -- read content of random > pages after checking PageOffline(); however, these PFN walkers can race > with drivers that set PageOffline(). > > Let's introduce page_offline_(begin|end|freeze|thaw) for > synchronizing. > > page_offline_freeze()/page_offline_thaw() allows for a subsystem to > synchronize with such drivers, achieving that a page cannot be set > PageOffline() while frozen. > > page_offline_begin()/page_offline_end() is used by drivers that care about > such races when setting a page PageOffline(). > > For simplicity, use a rwsem for now; neither drivers nor users are > performance sensitive. > > Acked-by: Michal Hocko <mhocko@suse.com> > Signed-off-by: David Hildenbrand <david@redhat.com> Reviewed-by: Oscar Salvador <osalvador@suse.de>
diff --git a/include/linux/page-flags.h b/include/linux/page-flags.h index daed82744f4b..ea2df9a247b3 100644 --- a/include/linux/page-flags.h +++ b/include/linux/page-flags.h @@ -769,9 +769,19 @@ PAGE_TYPE_OPS(Buddy, buddy) * relies on this feature is aware that re-onlining the memory block will * require to re-set the pages PageOffline() and not giving them to the * buddy via online_page_callback_t. + * + * There are drivers that mark a page PageOffline() and do not expect any + * further access to page content. PFN walkers that read content of random + * pages should check PageOffline() and synchronize with such drivers using + * page_offline_freeze()/page_offline_thaw(). */ PAGE_TYPE_OPS(Offline, offline) +extern void page_offline_freeze(void); +extern void page_offline_thaw(void); +extern void page_offline_begin(void); +extern void page_offline_end(void); + /* * Marks pages in use as page tables. */ diff --git a/mm/util.c b/mm/util.c index a8bf17f18a81..a034525e7ba2 100644 --- a/mm/util.c +++ b/mm/util.c @@ -1010,3 +1010,43 @@ void mem_dump_obj(void *object) } EXPORT_SYMBOL_GPL(mem_dump_obj); #endif + +/* + * A driver might set a page logically offline -- PageOffline() -- and + * turn the page inaccessible in the hypervisor; after that, access to page + * content can be fatal. + * + * Some special PFN walkers -- i.e., /proc/kcore -- read content of random + * pages after checking PageOffline(); however, these PFN walkers can race + * with drivers that set PageOffline(). + * + * page_offline_freeze()/page_offline_thaw() allows for a subsystem to + * synchronize with such drivers, achieving that a page cannot be set + * PageOffline() while frozen. + * + * page_offline_begin()/page_offline_end() is used by drivers that care about + * such races when setting a page PageOffline(). + */ +static DECLARE_RWSEM(page_offline_rwsem); + +void page_offline_freeze(void) +{ + down_read(&page_offline_rwsem); +} + +void page_offline_thaw(void) +{ + up_read(&page_offline_rwsem); +} + +void page_offline_begin(void) +{ + down_write(&page_offline_rwsem); +} +EXPORT_SYMBOL(page_offline_begin); + +void page_offline_end(void) +{ + up_write(&page_offline_rwsem); +} +EXPORT_SYMBOL(page_offline_end);