Messages in this thread | | | Subject | Re: [PATCH v2 4/5] mm, page_poison: remove CONFIG_PAGE_POISONING_NO_SANITY | From | David Hildenbrand <> | Date | Wed, 11 Nov 2020 16:43:43 +0100 |
| |
On 03.11.20 16:22, Vlastimil Babka wrote: > CONFIG_PAGE_POISONING_NO_SANITY skips the check on page alloc whether the > poison pattern was corrupted, suggesting a use-after-free. The motivation to > introduce it in commit 8823b1dbc05f ("mm/page_poison.c: enable PAGE_POISONING > as a separate option") was to simply sanitize freed pages, optimally together > with CONFIG_PAGE_POISONING_ZERO. > > These days we have an init_on_free=1 boot option, which makes this use case of > page poisoning redundant. For sanitizing, writing zeroes is sufficient, there > is pretty much no benefit from writing the 0xAA poison pattern to freed pages, > without checking it back on alloc. Thus, remove this option and suggest > init_on_free instead in the main config's help. > > Signed-off-by: Vlastimil Babka <vbabka@suse.cz> > --- > drivers/virtio/virtio_balloon.c | 4 +--- > mm/Kconfig.debug | 15 ++++----------- > mm/page_poison.c | 3 --- > 3 files changed, 5 insertions(+), 17 deletions(-) > > diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c > index e53faed6ba93..8985fc2cea86 100644 > --- a/drivers/virtio/virtio_balloon.c > +++ b/drivers/virtio/virtio_balloon.c > @@ -1114,9 +1114,7 @@ static int virtballoon_validate(struct virtio_device *vdev) > * page reporting as it could potentially change the contents > * of our free pages. > */ > - if (!want_init_on_free() && > - (IS_ENABLED(CONFIG_PAGE_POISONING_NO_SANITY) || > - !page_poisoning_enabled_static())) > + if (!want_init_on_free() && !page_poisoning_enabled_static()) > __virtio_clear_bit(vdev, VIRTIO_BALLOON_F_PAGE_POISON); > else if (!virtio_has_feature(vdev, VIRTIO_BALLOON_F_PAGE_POISON)) > __virtio_clear_bit(vdev, VIRTIO_BALLOON_F_REPORTING); > diff --git a/mm/Kconfig.debug b/mm/Kconfig.debug > index c57786ad5be9..14e29fe5bfa6 100644 > --- a/mm/Kconfig.debug > +++ b/mm/Kconfig.debug > @@ -74,18 +74,11 @@ config PAGE_POISONING > Note that "poison" here is not the same thing as the "HWPoison" > for CONFIG_MEMORY_FAILURE. This is software poisoning only. > > - If unsure, say N > + If you are only interested in sanitization of freed pages without > + checking the poison pattern on alloc, you can boot the kernel with > + "init_on_free=1" instead of enabling this. > > -config PAGE_POISONING_NO_SANITY > - depends on PAGE_POISONING > - bool "Only poison, don't sanity check" > - help > - Skip the sanity checking on alloc, only fill the pages with > - poison on free. This reduces some of the overhead of the > - poisoning feature. > - > - If you are only interested in sanitization, say Y. Otherwise > - say N. > + If unsure, say N > > config PAGE_POISONING_ZERO > bool "Use zero for poisoning instead of debugging value" > diff --git a/mm/page_poison.c b/mm/page_poison.c > index dd7aeada036f..084fc3ff4c15 100644 > --- a/mm/page_poison.c > +++ b/mm/page_poison.c > @@ -51,9 +51,6 @@ static void check_poison_mem(unsigned char *mem, size_t bytes) > unsigned char *start; > unsigned char *end; > > - if (IS_ENABLED(CONFIG_PAGE_POISONING_NO_SANITY)) > - return; > - > start = memchr_inv(mem, PAGE_POISON, bytes); > if (!start) > return; >
This clearly simplifies things.
Acked-by: David Hildenbrand <david@redhat.com>
-- Thanks,
David / dhildenb
| |