Re: [PATCH] powerpc/ipic: Fix a bounds check in ipic_set_priority()

On Wed, 5 Dec 2018, Michael Ellerman wrote:

Julia Lawall [off-list ref] writes:

quoted

On Wed, 5 Dec 2018, Michael Ellerman wrote:

quoted

Hi Dan,

Thanks for the patch.

Dan Carpenter [off-list ref] writes:

quoted

The ipic_info[] array only has 95 elements so I have made the bounds
check smaller to prevent a read overflow.  It was Smatch that found
this issue:

    arch/powerpc/sysdev/ipic.c:784 ipic_set_priority()
    error: buffer overflow 'ipic_info' 95 <= 127

Signed-off-by: Dan Carpenter <redacted>
---
I wasn't able to find any callers of this code.  Maybe we removed the
last one in commit b9f0f1bb2bca ("[POWERPC] Adapt ipic driver to new
host_ops interface, add set_irq_type to set IRQ sense").  So perhaps we
should just remove it.  I'm not really comfortable doing that myself,
because I don't know the code well enough and can't build test
it properly.

Hah wow, last usage removed in 2006!

I don't see any mention of it since then, so I'll remove it. If it
breaks something we can put it back.

Can smatch help us find things like this that are defined non-static but
never used?

I wrote a Coccinelle script for this, that just uses grep.  Of course the
results need checking because uses can be constructed within macros using
#.

That would be cool. I can't immediately see it in scripts/coccinelle, is
it somewhere else?

No, it needs improvement...  I'll try to do something with it soon.  I
don't think it  is well suited to scrips/coccinelle, because it needs to
know where the kernel tree is to do the grep.

julia

quoted

Are things that are defined static but are never used useful to keep
around?

No, but the compiler will usually tell us about them via -Wunused-function.

cheers