diff mbox

[25/26] isdn: eicon: mark divascapi incompatible with kasan

Message ID 20170302163834.2273519-26-arnd@arndb.de (mailing list archive)
State Not Applicable
Delegated to: Kalle Valo
Headers show

Commit Message

Arnd Bergmann March 2, 2017, 4:38 p.m. UTC
When CONFIG_KASAN is enabled, we have several functions that use rather
large kernel stacks, e.g.

drivers/isdn/hardware/eicon/message.c: In function 'group_optimization':
drivers/isdn/hardware/eicon/message.c:14841:1: warning: the frame size of 864 bytes is larger than 500 bytes [-Wframe-larger-than=]
drivers/isdn/hardware/eicon/message.c: In function 'add_b1':
drivers/isdn/hardware/eicon/message.c:7925:1: warning: the frame size of 1008 bytes is larger than 500 bytes [-Wframe-larger-than=]
drivers/isdn/hardware/eicon/message.c: In function 'add_b23':
drivers/isdn/hardware/eicon/message.c:8551:1: warning: the frame size of 928 bytes is larger than 500 bytes [-Wframe-larger-than=]
drivers/isdn/hardware/eicon/message.c: In function 'sig_ind':
drivers/isdn/hardware/eicon/message.c:6113:1: warning: the frame size of 2112 bytes is larger than 500 bytes [-Wframe-larger-than=]

To be on the safe side, and to enable a lower frame size warning limit, let's
just mark this driver as broken when KASAN is in use. I have tried to reduce
the stack size as I did with dozens of other drivers, but failed to come up
with a good solution for this one.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/isdn/hardware/eicon/Kconfig | 1 +
 1 file changed, 1 insertion(+)

Comments

Andrey Ryabinin March 3, 2017, 2:20 p.m. UTC | #1
On 03/02/2017 07:38 PM, Arnd Bergmann wrote:
> When CONFIG_KASAN is enabled, we have several functions that use rather
> large kernel stacks, e.g.
> 
> drivers/isdn/hardware/eicon/message.c: In function 'group_optimization':
> drivers/isdn/hardware/eicon/message.c:14841:1: warning: the frame size of 864 bytes is larger than 500 bytes [-Wframe-larger-than=]
> drivers/isdn/hardware/eicon/message.c: In function 'add_b1':
> drivers/isdn/hardware/eicon/message.c:7925:1: warning: the frame size of 1008 bytes is larger than 500 bytes [-Wframe-larger-than=]
> drivers/isdn/hardware/eicon/message.c: In function 'add_b23':
> drivers/isdn/hardware/eicon/message.c:8551:1: warning: the frame size of 928 bytes is larger than 500 bytes [-Wframe-larger-than=]
> drivers/isdn/hardware/eicon/message.c: In function 'sig_ind':
> drivers/isdn/hardware/eicon/message.c:6113:1: warning: the frame size of 2112 bytes is larger than 500 bytes [-Wframe-larger-than=]
> 
> To be on the safe side, and to enable a lower frame size warning limit, let's
> just mark this driver as broken when KASAN is in use. I have tried to reduce
> the stack size as I did with dozens of other drivers, but failed to come up
> with a good solution for this one.
> 

This is kinda radical solution.
Wouldn't be better to just increase -Wframe-larger-than for this driver through Makefile?



> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
>  drivers/isdn/hardware/eicon/Kconfig | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/isdn/hardware/eicon/Kconfig b/drivers/isdn/hardware/eicon/Kconfig
> index 6082b6a5ced3..b64496062421 100644
> --- a/drivers/isdn/hardware/eicon/Kconfig
> +++ b/drivers/isdn/hardware/eicon/Kconfig
> @@ -31,6 +31,7 @@ config ISDN_DIVAS_PRIPCI
>  
>  config ISDN_DIVAS_DIVACAPI
>  	tristate "DIVA CAPI2.0 interface support"
> +	depends on !KASAN || BROKEN
>  	help
>  	  You need this to provide the CAPI interface
>  	  for DIVA Server cards.
>
Arnd Bergmann March 3, 2017, 2:54 p.m. UTC | #2
On Fri, Mar 3, 2017 at 3:20 PM, Andrey Ryabinin <aryabinin@virtuozzo.com> wrote:
>
>
> On 03/02/2017 07:38 PM, Arnd Bergmann wrote:
>> When CONFIG_KASAN is enabled, we have several functions that use rather
>> large kernel stacks, e.g.
>>
>> drivers/isdn/hardware/eicon/message.c: In function 'group_optimization':
>> drivers/isdn/hardware/eicon/message.c:14841:1: warning: the frame size of 864 bytes is larger than 500 bytes [-Wframe-larger-than=]
>> drivers/isdn/hardware/eicon/message.c: In function 'add_b1':
>> drivers/isdn/hardware/eicon/message.c:7925:1: warning: the frame size of 1008 bytes is larger than 500 bytes [-Wframe-larger-than=]
>> drivers/isdn/hardware/eicon/message.c: In function 'add_b23':
>> drivers/isdn/hardware/eicon/message.c:8551:1: warning: the frame size of 928 bytes is larger than 500 bytes [-Wframe-larger-than=]
>> drivers/isdn/hardware/eicon/message.c: In function 'sig_ind':
>> drivers/isdn/hardware/eicon/message.c:6113:1: warning: the frame size of 2112 bytes is larger than 500 bytes [-Wframe-larger-than=]
>>
>> To be on the safe side, and to enable a lower frame size warning limit, let's
>> just mark this driver as broken when KASAN is in use. I have tried to reduce
>> the stack size as I did with dozens of other drivers, but failed to come up
>> with a good solution for this one.
>>
>
> This is kinda radical solution.
> Wouldn't be better to just increase -Wframe-larger-than for this driver through Makefile?

I thought about it too, and decided for disabling the driver entirely
since I suspected that
not only the per-function stack frame is overly large here but also
depth of the call chain,
which would then lead us to hiding an actual stack overflow.

Note that this driver is almost certainly broken, it hasn't seen any
updates other than
style and compile-warning fixes in 10 years and doesn't support any of
the hardware
introduced since 2002 (the company still makes PCIe ISDN adapters, but
the driver
only supports legacy PCI versions and older buses).

    Arnd
Andrey Ryabinin March 3, 2017, 3:22 p.m. UTC | #3
On 03/03/2017 05:54 PM, Arnd Bergmann wrote:
> On Fri, Mar 3, 2017 at 3:20 PM, Andrey Ryabinin <aryabinin@virtuozzo.com> wrote:
>>
>>
>> On 03/02/2017 07:38 PM, Arnd Bergmann wrote:
>>> When CONFIG_KASAN is enabled, we have several functions that use rather
>>> large kernel stacks, e.g.
>>>
>>> drivers/isdn/hardware/eicon/message.c: In function 'group_optimization':
>>> drivers/isdn/hardware/eicon/message.c:14841:1: warning: the frame size of 864 bytes is larger than 500 bytes [-Wframe-larger-than=]
>>> drivers/isdn/hardware/eicon/message.c: In function 'add_b1':
>>> drivers/isdn/hardware/eicon/message.c:7925:1: warning: the frame size of 1008 bytes is larger than 500 bytes [-Wframe-larger-than=]
>>> drivers/isdn/hardware/eicon/message.c: In function 'add_b23':
>>> drivers/isdn/hardware/eicon/message.c:8551:1: warning: the frame size of 928 bytes is larger than 500 bytes [-Wframe-larger-than=]
>>> drivers/isdn/hardware/eicon/message.c: In function 'sig_ind':
>>> drivers/isdn/hardware/eicon/message.c:6113:1: warning: the frame size of 2112 bytes is larger than 500 bytes [-Wframe-larger-than=]
>>>
>>> To be on the safe side, and to enable a lower frame size warning limit, let's
>>> just mark this driver as broken when KASAN is in use. I have tried to reduce
>>> the stack size as I did with dozens of other drivers, but failed to come up
>>> with a good solution for this one.
>>>
>>
>> This is kinda radical solution.
>> Wouldn't be better to just increase -Wframe-larger-than for this driver through Makefile?
> 
> I thought about it too, and decided for disabling the driver entirely
> since I suspected that
> not only the per-function stack frame is overly large here but also
> depth of the call chain,
> which would then lead us to hiding an actual stack overflow.
> 

No one complained so far ;)
Disabling the driver like you did will throw it out from allmodconfig so it will receive less compile-testing.


> Note that this driver is almost certainly broken, it hasn't seen any
> updates other than
> style and compile-warning fixes in 10 years and doesn't support any of
> the hardware
> introduced since 2002 (the company still makes PCIe ISDN adapters, but
> the driver
> only supports legacy PCI versions and older buses).

Which means that it's unlikely that someone will run this driver with KASAN and trigger stack overflow (if it's really possible).
Arnd Bergmann March 3, 2017, 3:37 p.m. UTC | #4
On Fri, Mar 3, 2017 at 4:22 PM, Andrey Ryabinin <aryabinin@virtuozzo.com> wrote:
> On 03/03/2017 05:54 PM, Arnd Bergmann wrote:
>> On Fri, Mar 3, 2017 at 3:20 PM, Andrey Ryabinin <aryabinin@virtuozzo.com> wrote:
>>> On 03/02/2017 07:38 PM, Arnd Bergmann wrote:
>>>
>>> This is kinda radical solution.
>>> Wouldn't be better to just increase -Wframe-larger-than for this driver through Makefile?
>>
>> I thought about it too, and decided for disabling the driver entirely
>> since I suspected that
>> not only the per-function stack frame is overly large here but also
>> depth of the call chain,
>> which would then lead us to hiding an actual stack overflow.
>>
>
> No one complained so far ;)
> Disabling the driver like you did will throw it out from allmodconfig so it will receive less compile-testing.

Good point, I'll add a driver specific flag then and leave it there.

>> Note that this driver is almost certainly broken, it hasn't seen any
>> updates other than
>> style and compile-warning fixes in 10 years and doesn't support any of
>> the hardware
>> introduced since 2002 (the company still makes PCIe ISDN adapters, but
>> the driver
>> only supports legacy PCI versions and older buses).
>
> Which means that it's unlikely that someone will run this driver with KASAN
and trigger stack overflow (if it's really possible).

True.

   Arnd
diff mbox

Patch

diff --git a/drivers/isdn/hardware/eicon/Kconfig b/drivers/isdn/hardware/eicon/Kconfig
index 6082b6a5ced3..b64496062421 100644
--- a/drivers/isdn/hardware/eicon/Kconfig
+++ b/drivers/isdn/hardware/eicon/Kconfig
@@ -31,6 +31,7 @@  config ISDN_DIVAS_PRIPCI
 
 config ISDN_DIVAS_DIVACAPI
 	tristate "DIVA CAPI2.0 interface support"
+	depends on !KASAN || BROKEN
 	help
 	  You need this to provide the CAPI interface
 	  for DIVA Server cards.