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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <20230803185718.GA2864317@dev-arch.thelio-3990X>
Date: Thu, 3 Aug 2023 11:57:18 -0700
From: Nathan Chancellor <nathan@...nel.org>
To: Kees Cook <keescook@...omium.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 12:59:12PM -0700, Kees Cook wrote:
> 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.

Sounds reasonable to me. netdev folks, if you intend to take this, do
you want a v2 or can you pick it up with

Fixes: e313ac12eb13 ("mISDN: Convert timers to use timer_setup()")

added on top?

> 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. :)

Sounds good, I do see it in the netdev patchwork, so we can watch it at
least.

Cheers,
Nathan