linux-kernel - Re: [PATCH] arm: Use kernel mm when updating section permissions

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAGXu5jLS8GPxmMQwd9qw+w+fkMqU-GYyME5WUuKZZ4qTesVzCQ@mail.gmail.com>
Date:	Thu, 5 Nov 2015 17:15:43 -0800
From:	Kees Cook <keescook@...omium.org>
To:	Laura Abbott <labbott@...hat.com>
Cc:	Russell King - ARM Linux <linux@....linux.org.uk>,
	Laura Abbott <labbott@...oraproject.org>,
	Catalin Marinas <catalin.marinas@....com>,
	Will Deacon <will.deacon@....com>,
	"linux-arm-kernel@...ts.infradead.org" 
	<linux-arm-kernel@...ts.infradead.org>,
	LKML <linux-kernel@...r.kernel.org>,
	Linux-MM <linux-mm@...ck.org>
Subject: Re: [PATCH] arm: Use kernel mm when updating section permissions

On Thu, Nov 5, 2015 at 5:05 PM, Laura Abbott <labbott@...hat.com> wrote:
> On 11/05/2015 08:27 AM, Russell King - ARM Linux wrote:
>>
>> On Thu, Nov 05, 2015 at 08:20:42AM -0800, Laura Abbott wrote:
>>>
>>> On 11/05/2015 01:46 AM, Russell King - ARM Linux wrote:
>>>>
>>>> On Wed, Nov 04, 2015 at 05:00:39PM -0800, Laura Abbott wrote:
>>>>>
>>>>> Currently, read only permissions are not being applied even
>>>>> when CONFIG_DEBUG_RODATA is set. This is because section_update
>>>>> uses current->mm for adjusting the page tables. current->mm
>>>>> need not be equivalent to the kernel version. Use pgd_offset_k
>>>>> to get the proper page directory for updating.
>>>>
>>>>
>>>> What are you trying to achieve here?  You can't use these functions
>>>> at run time (after the first thread has been spawned) to change
>>>> permissions, because there will be multiple copies of the kernel
>>>> section mappings, and those copies will not get updated.
>>>>
>>>> In any case, this change will probably break kexec and ftrace, as
>>>> the running thread will no longer see the updated page tables.
>>>>
>>>
>>> I think I was hitting that exact problem with multiple copies
>>> not getting updated. The section_update code was being called
>>> and I was seeing the tables get updated but nothing was being
>>> applied when I tried to write to text or check the debugfs
>>> page table. The current flow is:
>>>
>>> rest_init -> kernel_thread(kernel_init) and from that thread
>>> mark_rodata_ro. So mark_rodata_ro is always going to happen
>>> in a thread.
>>>
>>> Do we need to update for both init_mm and the first running
>>> thread?
>>
>>
>> The "first running thread" is merely coincidental for things like kexec.
>>
>> Hmm.  Actually, I think the existing code _should_ be fine.  At the
>> point where mark_rodata_ro() is, we should still be using init_mm, so
>> updating the current threads page tables should actually be updating
>> the swapper_pg_dir.
>
>
> That doesn't seem to hold true. Based on what I'm seeing, we lose
> the the guarantee of init_mm after the first exec. If usermodehelper
> gets called to load a module, that triggers an exec and the kernel
> thread is no longer using init_mm after that. I'm testing with the
> multi-v7 defconfig which uses the smsc911x driver which loads a
> module during initcall. That gets called before mark_rodata_ro so
> the init_mm is never updated. I verified that disabling smsc911x
> makes things work as expected. I suspect the testing was never done
> with a driver that tried to call usermodehelper during init time.

Ooooh. Nice catch. Yeah, my testing didn't include that case.

> I got as far as narrowing it down that it happens after the usermodehelper
> but I wasn't able to pinpoint where exactly the switch happened. It seems
> like we need to have the page tables set up before any initcalls
> happen otherwise we risk having an exec create stray processes which we
> can't update.

Can we just make mark_rodata_ro() a no-op and do the RO setting
earlier when we do the NX setting?

-Kees

-- 
Kees Cook
Chrome OS Security
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/