From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752916AbdHBPPf (ORCPT ); Wed, 2 Aug 2017 11:15:35 -0400 Received: from lhrrgout.huawei.com ([194.213.3.17]:32547 "EHLO lhrrgout.huawei.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752318AbdHBPPd (ORCPT ); Wed, 2 Aug 2017 11:15:33 -0400 To: Linux-MM , LKML , , "kernel-hardening@lists.openwall.com" , Jerome Glisse , Michal Hocko , "Kees Cook" From: Igor Stoppa Subject: [RFC] Tagging of vmalloc pages for supporting the pmalloc allocator Message-ID: <07063abd-2f5d-20d9-a182-8ae9ead26c3c@huawei.com> Date: Wed, 2 Aug 2017 18:14:28 +0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.2.1 MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Language: en-US Content-Transfer-Encoding: 7bit X-Originating-IP: [10.122.225.51] X-CFilter-Loop: Reflected X-Mirapoint-Virus-RAPID-Raw: score=unknown(0), refid=str=0001.0A020203.5981EC92.0210,ss=1,re=0.000,recu=0.000,reip=0.000,cl=1,cld=1,fgs=0, ip=0.0.0.0, so=2013-06-18 04:22:30, dmn=2013-03-21 17:37:32 X-Mirapoint-Loop-Id: 7a3e2ab789f4d29daeb7d6337c9acd04 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi, while I am working to another example of using pmalloc [1], it was pointed out to me that: 1) I had introduced a bug when I switched to using a field of the page structure [2] 2) I was also committing a layer violation in the way I was tagging the pages. I am seeking help to understand what would be the correct way to do the tagging. Here are snippets describing the problems: 1) from pmalloc.c: ... +static const unsigned long pmalloc_signature = (unsigned long)&pmalloc_mutex; ... +int __pmalloc_tag_pages(void *base, const size_t size, const bool set_tag) +{ + void *end = base + size - 1; + + do { + struct page *page; + + if (!is_vmalloc_addr(base)) + return -EINVAL; + page = vmalloc_to_page(base); + if (set_tag) { + BUG_ON(page_private(page) || page->private); + set_page_private(page, 1); + page->private = pmalloc_signature; + } else { + BUG_ON(!(page_private(page) && + page->private == pmalloc_signature)); + set_page_private(page, 0); + page->private = 0; + } + base += PAGE_SIZE; + } while ((PAGE_MASK & (unsigned long)base) <= + (PAGE_MASK & (unsigned long)end)); + return 0; +} ... +static const char msg[] = "Not a valid Pmalloc object."; +const char *pmalloc_check_range(const void *ptr, unsigned long n) +{ + unsigned long p; + + p = (unsigned long)ptr; + n = p + n - 1; + for (; (PAGE_MASK & p) <= (PAGE_MASK & n); p += PAGE_SIZE) { + struct page *page; + + if (!is_vmalloc_addr((void *)p)) + return msg; + page = vmalloc_to_page((void *)p); + if (!(page && page_private(page) && + page->private == pmalloc_signature)) + return msg; + } + return NULL; +} The problem here comes from the way I am using page->private: the fact that the page is marked as private means only that someone is using it, and the way it is used could create (spoiler: it happens) a collision with pmalloc_signature, which can generate false positives. A way to ensure that the address really belongs to pmalloc would be to pre-screen it, against either the signature or some magic number and, if such test is passed, then compare the address against those really available in the pmalloc pools. This would be slower, but it would be limited only to those cases where the signature/magic number matches and the answer is likely to be true. 2) However, both the current (incorrect) implementation and the one I am considering, are abusing something that should be used otherwise (see the following snippet): from include/linux/mm_types.h: struct page { ... union { unsigned long private; /* Mapping-private opaque data: * usually used for buffer_heads * if PagePrivate set; used for * swp_entry_t if PageSwapCache; * indicates order in the buddy * system if PG_buddy is set. */ #if USE_SPLIT_PTE_PTLOCKS #if ALLOC_SPLIT_PTLOCKS spinlock_t *ptl; #else spinlock_t ptl; #endif #endif struct kmem_cache *slab_cache; /* SL[AU]B: Pointer to slab */ }; ... } The "private" field is meant for mapping-private opaque data, which is not how I am using it. Yet it seems the least harmful field to choose. Is this acceptable? Otherwise, what would be the best course of action? thanks, igor [1] https://lkml.org/lkml/2017/7/10/400 [2] https://lkml.org/lkml/2017/7/6/573