diff mbox series

[v2,5/5] powerpc/configs: Disable SCOM_DEBUGFS in powernv_defconfig

Message ID 20190509051119.7694-5-ajd@linux.ibm.com (mailing list archive)
State Accepted
Commit 8b856a0942a1b4d832966985fcdf1a455eb6ab8c
Headers show
Series [v2,1/5] powerpc/powernv: Move SCOM access code into powernv platform | expand

Checks

Context Check Description
snowpatch_ozlabs/apply_patch success Successfully applied on branch next (8150a153c013aa2dd1ffae43370b89ac1347a7fb)
snowpatch_ozlabs/build-ppc64le success Build succeeded
snowpatch_ozlabs/build-ppc64be success Build succeeded
snowpatch_ozlabs/build-ppc64e success Build succeeded
snowpatch_ozlabs/build-pmac32 success Build succeeded
snowpatch_ozlabs/checkpatch success total: 0 errors, 0 warnings, 0 checks, 8 lines checked

Commit Message

Andrew Donnellan May 9, 2019, 5:11 a.m. UTC
SCOM_DEBUGFS is really not needed for anything other than low-level
hardware debugging.

opal-prd uses its own interface (/dev/prd) for SCOM access, so it doesn't
need SCOM_DEBUGFS.

At some point in the future we'll introduce a debug config fragment where
this can go instead.

Signed-off-by: Andrew Donnellan <ajd@linux.ibm.com>
---
v1->v2:
- new patch
---
 arch/powerpc/configs/powernv_defconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Nicholas Piggin May 9, 2019, 5:37 a.m. UTC | #1
Andrew Donnellan's on May 9, 2019 3:11 pm:
> SCOM_DEBUGFS is really not needed for anything other than low-level
> hardware debugging.
> 
> opal-prd uses its own interface (/dev/prd) for SCOM access, so it doesn't
> need SCOM_DEBUGFS.
> 
> At some point in the future we'll introduce a debug config fragment where
> this can go instead.

That doesn't really explain why you want to disable it. It is useful
for low level hardware debugging, I added it.

obscurity^Wsecurity?

> 
> Signed-off-by: Andrew Donnellan <ajd@linux.ibm.com>
> ---
> v1->v2:
> - new patch
> ---
>  arch/powerpc/configs/powernv_defconfig | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/powerpc/configs/powernv_defconfig b/arch/powerpc/configs/powernv_defconfig
> index ef2ef98d3f28..d5a6608cb2e0 100644
> --- a/arch/powerpc/configs/powernv_defconfig
> +++ b/arch/powerpc/configs/powernv_defconfig
> @@ -38,7 +38,7 @@ CONFIG_MODULE_UNLOAD=y
>  CONFIG_MODVERSIONS=y
>  CONFIG_MODULE_SRCVERSION_ALL=y
>  CONFIG_PARTITION_ADVANCED=y
> -CONFIG_SCOM_DEBUGFS=y
> +# CONFIG_SCOM_DEBUGFS is not set
>  CONFIG_OPAL_PRD=y
>  CONFIG_PPC_MEMTRACE=y
>  # CONFIG_PPC_PSERIES is not set
> -- 
> 2.20.1
> 
>
Oliver O'Halloran May 9, 2019, 5:53 a.m. UTC | #2
On Thu, May 9, 2019 at 3:38 PM Nicholas Piggin <npiggin@gmail.com> wrote:
>
> Andrew Donnellan's on May 9, 2019 3:11 pm:
> > SCOM_DEBUGFS is really not needed for anything other than low-level
> > hardware debugging.
> >
> > opal-prd uses its own interface (/dev/prd) for SCOM access, so it doesn't
> > need SCOM_DEBUGFS.
> >
> > At some point in the future we'll introduce a debug config fragment where
> > this can go instead.
>
> That doesn't really explain why you want to disable it. It is useful
> for low level hardware debugging, I added it.
>
> obscurity^Wsecurity?

Yeah... If you're building powernv_defconfig then the odds are pretty
high that you'll want scom access.

>
> >
> > Signed-off-by: Andrew Donnellan <ajd@linux.ibm.com>
> > ---
> > v1->v2:
> > - new patch
> > ---
> >  arch/powerpc/configs/powernv_defconfig | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/arch/powerpc/configs/powernv_defconfig b/arch/powerpc/configs/powernv_defconfig
> > index ef2ef98d3f28..d5a6608cb2e0 100644
> > --- a/arch/powerpc/configs/powernv_defconfig
> > +++ b/arch/powerpc/configs/powernv_defconfig
> > @@ -38,7 +38,7 @@ CONFIG_MODULE_UNLOAD=y
> >  CONFIG_MODVERSIONS=y
> >  CONFIG_MODULE_SRCVERSION_ALL=y
> >  CONFIG_PARTITION_ADVANCED=y
> > -CONFIG_SCOM_DEBUGFS=y
> > +# CONFIG_SCOM_DEBUGFS is not set
> >  CONFIG_OPAL_PRD=y
> >  CONFIG_PPC_MEMTRACE=y
> >  # CONFIG_PPC_PSERIES is not set
> > --
> > 2.20.1
> >
> >
Andrew Donnellan May 9, 2019, 5:54 a.m. UTC | #3
On 9/5/19 3:37 pm, Nicholas Piggin wrote:
> Andrew Donnellan's on May 9, 2019 3:11 pm:
>> SCOM_DEBUGFS is really not needed for anything other than low-level
>> hardware debugging.
>>
>> opal-prd uses its own interface (/dev/prd) for SCOM access, so it doesn't
>> need SCOM_DEBUGFS.
>>
>> At some point in the future we'll introduce a debug config fragment where
>> this can go instead.
> 
> That doesn't really explain why you want to disable it. It is useful
> for low level hardware debugging, I added it.
> 
> obscurity^Wsecurity?

Mostly just a general feeling that it's not something we need to have by 
default. Security-wise, PRD still provides SCOM access, though we are 
going to look at how we can further lock that down. Shrinks the build by 
only a few kilobytes...

mpe said he's planning on adding a debug.config where we can shift stuff 
like this, and if/when we do that I would like to see this moved there, 
but perhaps this patch can wait until then. I'll let mpe decide.


Andrew


> 
>>
>> Signed-off-by: Andrew Donnellan <ajd@linux.ibm.com>
>> ---
>> v1->v2:
>> - new patch
>> ---
>>   arch/powerpc/configs/powernv_defconfig | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/arch/powerpc/configs/powernv_defconfig b/arch/powerpc/configs/powernv_defconfig
>> index ef2ef98d3f28..d5a6608cb2e0 100644
>> --- a/arch/powerpc/configs/powernv_defconfig
>> +++ b/arch/powerpc/configs/powernv_defconfig
>> @@ -38,7 +38,7 @@ CONFIG_MODULE_UNLOAD=y
>>   CONFIG_MODVERSIONS=y
>>   CONFIG_MODULE_SRCVERSION_ALL=y
>>   CONFIG_PARTITION_ADVANCED=y
>> -CONFIG_SCOM_DEBUGFS=y
>> +# CONFIG_SCOM_DEBUGFS is not set
>>   CONFIG_OPAL_PRD=y
>>   CONFIG_PPC_MEMTRACE=y
>>   # CONFIG_PPC_PSERIES is not set
>> -- 
>> 2.20.1
>>
>>
>
Andrew Donnellan July 31, 2019, 1:45 a.m. UTC | #4
On 9/5/19 3:54 pm, Andrew Donnellan wrote:
> On 9/5/19 3:37 pm, Nicholas Piggin wrote:
>> Andrew Donnellan's on May 9, 2019 3:11 pm:
>>> SCOM_DEBUGFS is really not needed for anything other than low-level
>>> hardware debugging.
>>>
>>> opal-prd uses its own interface (/dev/prd) for SCOM access, so it 
>>> doesn't
>>> need SCOM_DEBUGFS.
>>>
>>> At some point in the future we'll introduce a debug config fragment 
>>> where
>>> this can go instead.
>>
>> That doesn't really explain why you want to disable it. It is useful
>> for low level hardware debugging, I added it.
>>
>> obscurity^Wsecurity?
> 
> Mostly just a general feeling that it's not something we need to have by 
> default. Security-wise, PRD still provides SCOM access, though we are 
> going to look at how we can further lock that down. Shrinks the build by 
> only a few kilobytes...
> 
> mpe said he's planning on adding a debug.config where we can shift stuff 
> like this, and if/when we do that I would like to see this moved there, 
> but perhaps this patch can wait until then. I'll let mpe decide.
> 

mpe do you have thoughts on this? I would like to see at least the rest 
of this series merged.


Andrew


> 
> Andrew
> 
> 
>>
>>>
>>> Signed-off-by: Andrew Donnellan <ajd@linux.ibm.com>
>>> ---
>>> v1->v2:
>>> - new patch
>>> ---
>>>   arch/powerpc/configs/powernv_defconfig | 2 +-
>>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/arch/powerpc/configs/powernv_defconfig 
>>> b/arch/powerpc/configs/powernv_defconfig
>>> index ef2ef98d3f28..d5a6608cb2e0 100644
>>> --- a/arch/powerpc/configs/powernv_defconfig
>>> +++ b/arch/powerpc/configs/powernv_defconfig
>>> @@ -38,7 +38,7 @@ CONFIG_MODULE_UNLOAD=y
>>>   CONFIG_MODVERSIONS=y
>>>   CONFIG_MODULE_SRCVERSION_ALL=y
>>>   CONFIG_PARTITION_ADVANCED=y
>>> -CONFIG_SCOM_DEBUGFS=y
>>> +# CONFIG_SCOM_DEBUGFS is not set
>>>   CONFIG_OPAL_PRD=y
>>>   CONFIG_PPC_MEMTRACE=y
>>>   # CONFIG_PPC_PSERIES is not set
>>> -- 
>>> 2.20.1
>>>
>>>
>>
>
Michael Ellerman July 31, 2019, noon UTC | #5
Andrew Donnellan <ajd@linux.ibm.com> writes:
> On 9/5/19 3:54 pm, Andrew Donnellan wrote:
>> On 9/5/19 3:37 pm, Nicholas Piggin wrote:
>>> Andrew Donnellan's on May 9, 2019 3:11 pm:
>>>> SCOM_DEBUGFS is really not needed for anything other than low-level
>>>> hardware debugging.
>>>>
>>>> opal-prd uses its own interface (/dev/prd) for SCOM access, so it 
>>>> doesn't
>>>> need SCOM_DEBUGFS.
>>>>
>>>> At some point in the future we'll introduce a debug config fragment 
>>>> where
>>>> this can go instead.
>>>
>>> That doesn't really explain why you want to disable it. It is useful
>>> for low level hardware debugging, I added it.
>>>
>>> obscurity^Wsecurity?
>> 
>> Mostly just a general feeling that it's not something we need to have by 
>> default. Security-wise, PRD still provides SCOM access, though we are 
>> going to look at how we can further lock that down. Shrinks the build by 
>> only a few kilobytes...
>> 
>> mpe said he's planning on adding a debug.config where we can shift stuff 
>> like this, and if/when we do that I would like to see this moved there, 
>> but perhaps this patch can wait until then. I'll let mpe decide.
>
> mpe do you have thoughts on this? I would like to see at least the rest 
> of this series merged.

I think it should be off by default, it may be true that root can
attack/break the system in many other ways, but providing unrestricted
SCOM access is a pretty big opening.

This is similar to strict /dev/mem IMO, which "everyone" agrees is a
good idea these days.

It's pretty trivial for folks doing development to turn it on in their
local trees, and I would also welcome a debug.config fragment that
enables it.

So I'll take the whole series.

cheers
diff mbox series

Patch

diff --git a/arch/powerpc/configs/powernv_defconfig b/arch/powerpc/configs/powernv_defconfig
index ef2ef98d3f28..d5a6608cb2e0 100644
--- a/arch/powerpc/configs/powernv_defconfig
+++ b/arch/powerpc/configs/powernv_defconfig
@@ -38,7 +38,7 @@  CONFIG_MODULE_UNLOAD=y
 CONFIG_MODVERSIONS=y
 CONFIG_MODULE_SRCVERSION_ALL=y
 CONFIG_PARTITION_ADVANCED=y
-CONFIG_SCOM_DEBUGFS=y
+# CONFIG_SCOM_DEBUGFS is not set
 CONFIG_OPAL_PRD=y
 CONFIG_PPC_MEMTRACE=y
 # CONFIG_PPC_PSERIES is not set