Re: [PATCH] mISDN: Update parameter type of dsp_cmx_send()

[Kees Cook <keescook@chromium.org>]

On Wed, Aug 02, 2023 at 10:40:29AM -0700, Nathan Chancellor wrote:
> When booting a kernel with CONFIG_MISDN_DSP=y and CONFIG_CFI_CLANG=y,
> there is a failure when dsp_cmx_send() is called indirectly from
> call_timer_fn():
> 
>   [    0.371412] CFI failure at call_timer_fn+0x2f/0x150 (target: dsp_cmx_send+0x0/0x530; expected type: 0x92ada1e9)
> 
> The function pointer prototype that call_timer_fn() expects is
> 
>   void (*fn)(struct timer_list *)
> 
> whereas dsp_cmx_send() has a parameter type of 'void *', which causes
> the control flow integrity checks to fail because the parameter types do
> not match.
> 
> Change dsp_cmx_send()'s parameter type to be 'struct timer_list' to
> match the expected prototype. The argument is unused anyways, so this
> has no functional change, aside from avoiding the CFI failure.
> 
> Reported-by: kernel test robot <oliver.sang@intel.com>
> Closes: https://lore.kernel.org/oe-lkp/202308020936.58787e6c-oliver.sang@intel.com
> Signed-off-by: Nathan Chancellor <nathan@kernel.org>
> ---
> I am not sure if there is an appropriate fixes tag for this, I see this
> area was modified by commit e313ac12eb13 ("mISDN: Convert timers to use
> timer_setup()") but I don't think it was the original source of the
> issue. It could also be commit cf68fffb66d6 ("add support for Clang
> CFI") but I think that just exposes the problem/makes it fatal.

Oh man. I missed one! How did I miss that one? I think "Fixes:
e313ac12eb13" is the most correct. That was the patch that went through
trying to fix all the prototypes, and _did_ fix all the _other_ prototypes
in there.

Thanks for the patch!

Reviewed-by: Kees Cook <keescook@chromium.org>

> 
> Also not sure who should take this or how soon it should go in, I'll let
> that to maintainers to figure out :)

If no one speaks up, I'll snag it, but since this got aimed at netdev, I
suspect someone may pick it up. :)

-Kees

-- 
Kees Cook