netdev - Re: [PATCH] mISDN: Update parameter type of dsp_cmx

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date: Wed, 2 Aug 2023 12:59:12 -0700
From: Kees Cook <keescook@...omium.org>
To: Nathan Chancellor <nathan@...nel.org>
Cc: isdn@...ux-pingi.de, netdev@...r.kernel.org, samitolvanen@...gle.com,
	llvm@...ts.linux.dev, patches@...ts.linux.dev,
	kernel test robot <oliver.sang@...el.com>
Subject: Re: [PATCH] mISDN: Update parameter type of dsp_cmx_send()

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@...el.com>
> Closes: https://lore.kernel.org/oe-lkp/202308020936.58787e6c-oliver.sang@intel.com
> Signed-off-by: Nathan Chancellor <nathan@...nel.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@...omium.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