Patchwork removing debugfs

login
register
mail settings
Submitter Tim Gardner
Date Jan. 25, 2011, 2:31 a.m.
Message ID <4D3E3617.9040304@tpi.com>
Download mbox | patch
Permalink /patch/80303/
State Accepted
Delegated to: Tim Gardner
Headers show

Comments

Tim Gardner - Jan. 25, 2011, 2:31 a.m.
On 01/24/2011 07:19 PM, Kees Cook wrote:
> Hi,
>
> On Tue, Jan 25, 2011 at 11:48:13AM +1000, Ben Hutchings wrote:
>> On Mon, 2011-01-24 at 14:13 -0800, Kees Cook wrote:
>>> I have yet another unpopular request: I want to remove debugfs completely
>>> from the built kernels. Upstream continues to put dangerous things in it,
>>> and I want to avoid the problems completely.
>> [...]
>>
>> I agree that it should not be mounted by default, or relied on by any
>> user-space packages.  However, I don't see the need to disable it
>> altogether.
>
> My specific issue with it is the acpi_method interface, which nullifies the
> /dev/mem and /dev/kmem restrictions (i.e. a root user can once again
> arbitrarily write to memory). The defenses for kernel rootkits require that
> the root user not have any way to write to kernel memory (nor load arbitrary
> modules).
>
> For example, without debugfs and barring unknown vulnerabilities,
> if a system owner chooses at boot time to echo 1 into
> /proc/sys/kernel/modules_disabled, there isn't a way to modify the
> running kernel. Unfortunately, with acpi_method, this is no longer true.
>
> I'd like to remove debugfs completely so it cannot just be trivially
> mounted and abused, and to avoid potential future problems.
>
> As mentioned, though, the minimal compromise will be to just flat remove
> acpi_method, as it is a real and present danger as opposed to some set of
> future unknown dangers. :)
>
> -Kees
>

Is this sufficient?

rtg
Kees Cook - Jan. 25, 2011, 3:57 a.m.
Hi Tim,

On Mon, Jan 24, 2011 at 07:31:51PM -0700, Tim Gardner wrote:
> On 01/24/2011 07:19 PM, Kees Cook wrote:
> >I'd like to remove debugfs completely so it cannot just be trivially
> >mounted and abused, and to avoid potential future problems.
> 
> Is this sufficient?

Well, I assume CONFIG_DEBUG_FS=n would be easy to discover, but yeah, that
would turn it off. That doesn't solve the need that things like ureadahead,
and the graphics lock-up investigation tool that apport uses. I suspect
there are more existing users of the debugfs, and it seems like their
interfaces should be moved somewhere not called "debug".

> diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile
> index d113fa5..123e281 100644
> --- a/drivers/acpi/Makefile
> +++ b/drivers/acpi/Makefile
> @@ -39,7 +39,7 @@ acpi-y				+= pci_root.o pci_link.o pci_irq.o pci_bind.o
>  acpi-y				+= power.o
>  acpi-y				+= event.o
>  acpi-y				+= sysfs.o
> -acpi-$(CONFIG_DEBUG_FS)		+= debugfs.o
> +#acpi-$(CONFIG_DEBUG_FS)		+= debugfs.o
>  acpi-$(CONFIG_ACPI_NUMA)	+= numa.o
>  acpi-$(CONFIG_ACPI_PROCFS_POWER) += cm_sbs.o
>  ifdef CONFIG_ACPI_VIDEO
Stefan Bader - Jan. 25, 2011, 11:03 a.m.
On 01/25/2011 03:31 AM, Tim Gardner wrote:
> On 01/24/2011 07:19 PM, Kees Cook wrote:
>> Hi,
>>
>> On Tue, Jan 25, 2011 at 11:48:13AM +1000, Ben Hutchings wrote:
>>> On Mon, 2011-01-24 at 14:13 -0800, Kees Cook wrote:
>>>> I have yet another unpopular request: I want to remove debugfs completely
>>>> from the built kernels. Upstream continues to put dangerous things in it,
>>>> and I want to avoid the problems completely.
>>> [...]
>>>
>>> I agree that it should not be mounted by default, or relied on by any
>>> user-space packages.  However, I don't see the need to disable it
>>> altogether.
>>
>> My specific issue with it is the acpi_method interface, which nullifies the
>> /dev/mem and /dev/kmem restrictions (i.e. a root user can once again
>> arbitrarily write to memory). The defenses for kernel rootkits require that
>> the root user not have any way to write to kernel memory (nor load arbitrary
>> modules).
>>
>> For example, without debugfs and barring unknown vulnerabilities,
>> if a system owner chooses at boot time to echo 1 into
>> /proc/sys/kernel/modules_disabled, there isn't a way to modify the
>> running kernel. Unfortunately, with acpi_method, this is no longer true.
>>
>> I'd like to remove debugfs completely so it cannot just be trivially
>> mounted and abused, and to avoid potential future problems.
>>
>> As mentioned, though, the minimal compromise will be to just flat remove
>> acpi_method, as it is a real and present danger as opposed to some set of
>> future unknown dangers. :)
>>
>> -Kees
>>
> 
> Is this sufficient?
> 
> rtg
> 
Just to add my 1cent: I also would rather prefer to only disable the acpi part
and not the whole of debugfs. Not to mount it by default ok, but there is just
too much useful things in there to track gpu hangs or trace usb traffic that
helps a lot to debug issues and to have to provide a debug enabled kernel just
for that seems more waste than the security risk I see from having it there.
(the paranoid can delete or blacklist the module).

-Stefan
Ben Hutchings - Jan. 27, 2011, 5:38 a.m.
On Tue, 2011-01-25 at 12:03 +0100, Stefan Bader wrote:
[...]
> Just to add my 1cent: I also would rather prefer to only disable the acpi part
> and not the whole of debugfs. Not to mount it by default ok, but there is just
> too much useful things in there to track gpu hangs or trace usb traffic that
> helps a lot to debug issues and to have to provide a debug enabled kernel just
> for that seems more waste than the security risk I see from having it there.
> (the paranoid can delete or blacklist the module).

That won't work, as any driver that uses debugfs would not be loadable.
The paranoid will need some way to prevent mounting debugfs even when
the module is loaded.  Could be done with a module parameter.

Ben.
Stefan Bader - Jan. 27, 2011, 9:52 a.m.
On 01/27/2011 06:38 AM, Ben Hutchings wrote:
> On Tue, 2011-01-25 at 12:03 +0100, Stefan Bader wrote:
> [...]
>> Just to add my 1cent: I also would rather prefer to only disable the acpi part
>> and not the whole of debugfs. Not to mount it by default ok, but there is just
>> too much useful things in there to track gpu hangs or trace usb traffic that
>> helps a lot to debug issues and to have to provide a debug enabled kernel just
>> for that seems more waste than the security risk I see from having it there.
>> (the paranoid can delete or blacklist the module).
> 
> That won't work, as any driver that uses debugfs would not be loadable.
> The paranoid will need some way to prevent mounting debugfs even when
> the module is loaded.  Could be done with a module parameter.
> 
> Ben.
> 

You are right. I did not think of all the stubs that get added when debugfs
support is on. I guess the problem with a module parameter is that this again
could be modified in the running system via sysfs. Of course this can/would be
limited to root. But on the other hand, if debugfs is not in fstab and mountable
by the user, one needs to be root to mount it anyway.

-Stefan
Tim Gardner - Jan. 28, 2011, 3:14 p.m.
On 01/24/2011 08:57 PM, Kees Cook wrote:
> Hi Tim,
>
> On Mon, Jan 24, 2011 at 07:31:51PM -0700, Tim Gardner wrote:
>> On 01/24/2011 07:19 PM, Kees Cook wrote:
>>> I'd like to remove debugfs completely so it cannot just be trivially
>>> mounted and abused, and to avoid potential future problems.
>>
>> Is this sufficient?
>
> Well, I assume CONFIG_DEBUG_FS=n would be easy to discover, but yeah, that
> would turn it off. That doesn't solve the need that things like ureadahead,
> and the graphics lock-up investigation tool that apport uses. I suspect
> there are more existing users of the debugfs, and it seems like their
> interfaces should be moved somewhere not called "debug".

Kees - I'm not sure what you mean by 'I assume CONFIG_DEBUG_FS=n would 
be easy to discover'.

Like Stefan, I'm not quite willing to disable CONFIG_DEBUG_FS across the 
board because it can be very useful. Where there are specific 
vulnerabilities, such as with acpi, I'm quite willing to either fix 'em 
or hack 'em out. In this case just disabling the compile of 
drivers/acpi/debugfs.c looks like it'll work.

rtg
Kees Cook - Jan. 28, 2011, 7:48 p.m.
Hi Tim,

On Fri, Jan 28, 2011 at 08:14:48AM -0700, Tim Gardner wrote:
> Like Stefan, I'm not quite willing to disable CONFIG_DEBUG_FS across
> the board because it can be very useful. Where there are specific
> vulnerabilities, such as with acpi, I'm quite willing to either fix
> 'em or hack 'em out. In this case just disabling the compile of
> drivers/acpi/debugfs.c looks like it'll work.

Yup, let's do that for now.

Thanks,

-Kees
Tim Gardner - Jan. 28, 2011, 8:53 p.m.
On 01/28/2011 12:48 PM, Kees Cook wrote:
> Hi Tim,
>
> On Fri, Jan 28, 2011 at 08:14:48AM -0700, Tim Gardner wrote:
>> Like Stefan, I'm not quite willing to disable CONFIG_DEBUG_FS across
>> the board because it can be very useful. Where there are specific
>> vulnerabilities, such as with acpi, I'm quite willing to either fix
>> 'em or hack 'em out. In this case just disabling the compile of
>> drivers/acpi/debugfs.c looks like it'll work.
>
> Yup, let's do that for now.
>
> Thanks,
>
> -Kees
>

disabled for Natty:

UBUNTU: SAUCE: Disable building the ACPI debugfs source

rtg

Patch

diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile
index d113fa5..123e281 100644
--- a/drivers/acpi/Makefile
+++ b/drivers/acpi/Makefile
@@ -39,7 +39,7 @@  acpi-y				+= pci_root.o pci_link.o pci_irq.o pci_bind.o
 acpi-y				+= power.o
 acpi-y				+= event.o
 acpi-y				+= sysfs.o
-acpi-$(CONFIG_DEBUG_FS)		+= debugfs.o
+#acpi-$(CONFIG_DEBUG_FS)		+= debugfs.o
 acpi-$(CONFIG_ACPI_NUMA)	+= numa.o
 acpi-$(CONFIG_ACPI_PROCFS_POWER) += cm_sbs.o
 ifdef CONFIG_ACPI_VIDEO