netdev - Re: [PATCH] [RFC] xfrm: work around a clang-19 fortifiy-string false-positive

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: <20240410174540.GB3649628@dev-arch.thelio-3990X>
Date: Wed, 10 Apr 2024 10:45:40 -0700
From: Nathan Chancellor <nathan@...nel.org>
To: Arnd Bergmann <arnd@...db.de>
Cc: Kees Cook <keescook@...omium.org>, Arnd Bergmann <arnd@...nel.org>,
	Steffen Klassert <steffen.klassert@...unet.com>,
	Herbert Xu <herbert@...dor.apana.org.au>,
	"David S . Miller" <davem@...emloft.net>,
	Eric Dumazet <edumazet@...gle.com>,
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
	Nick Desaulniers <ndesaulniers@...gle.com>,
	Bill Wendling <morbo@...gle.com>,
	Justin Stitt <justinstitt@...gle.com>,
	"Gustavo A. R. Silva" <gustavoars@...nel.org>,
	Leon Romanovsky <leon@...nel.org>, Lin Ma <linma@....edu.cn>,
	Simon Horman <horms@...nel.org>, Breno Leitao <leitao@...ian.org>,
	Tobias Brunner <tobias@...ongswan.org>,
	Raed Salem <raeds@...dia.com>, Netdev <netdev@...r.kernel.org>,
	linux-kernel@...r.kernel.org, llvm@...ts.linux.dev
Subject: Re: [PATCH] [RFC] xfrm: work around a clang-19 fortifiy-string
 false-positive

On Tue, Apr 09, 2024 at 09:41:09PM +0200, Arnd Bergmann wrote:
> On Tue, Apr 9, 2024, at 18:15, Nathan Chancellor wrote:
> > On Mon, Apr 08, 2024 at 09:06:21AM +0200, Arnd Bergmann wrote:
> >> >
> >> > The shorter fix (in the issue) is to explicitly range-check before
> >> > the loop:
> >> >
> >> >        if (xp->xfrm_nr > XFRM_MAX_DEPTH)
> >> >                return -ENOBUFS;
> >> 
> >> I ran into this issue again and I see that Nathan's fix has
> >> made it into mainline and backports, but it's apparently
> >> not sufficient.
> >> 
> >> I don't see the warning with my patch from this thread, but
> >> there may still be a better fix.
> >
> > Is it the exact same warning? clang-19 or older?
> > What > architecture/configuration? If my change is not sufficient then maybe
> > there are two separate issues here? I have not seen this warning appear
> > in our CI since my change was applied.
> 
> I only see it with clang-19. I've never seen it with arm32 and
> currently only see it with arm64, though I had seen it with x86-64
> as well in February before your patch.

That seems to line up with my prior experience.

> The warning is the same as before aside from the line number,
> which which is now include/linux/fortify-string.h:462:4
> where it was line 420, but I think that is just a context
> change.
> 
> I have a number of configs that reproduce this bug, see
> https://pastebin.com/tMgfD7cu for an example with current
> linux-next.

Thanks for that. I was able to reproduce this on next-20240410 as well
and I reduced the necessary configurations needed to reproduce this on
top of just defconfig:

  $ echo 'CONFIG_FORTIFY_SOURCE=y
  CONFIG_KASAN=y
  CONFIG_XFRM_USER=y' >arch/arm64/configs/repro.config

  $ make -skj"$(nproc)" ARCH=arm64 LLVM=1 {def,repro.}config net/xfrm/xfrm_user.o
  In file included from net/xfrm/xfrm_user.c:14:
  In file included from include/linux/compat.h:10:
  In file included from include/linux/time.h:60:
  In file included from include/linux/time32.h:13:
  In file included from include/linux/timex.h:67:
  In file included from arch/arm64/include/asm/timex.h:8:
  In file included from arch/arm64/include/asm/arch_timer.h:12:
  In file included from arch/arm64/include/asm/hwcap.h:9:
  In file included from arch/arm64/include/asm/cpufeature.h:27:
  In file included from include/linux/cpumask.h:13:
  In file included from include/linux/bitmap.h:13:
  In file included from include/linux/string.h:371:
  include/linux/fortify-string.h:462:4: warning: call to '__write_overflow_field' declared with 'warning' attribute: detected write beyond size of field (1st parameter); maybe use struct_group()? [-Wattribute-warning]
    462 |                         __write_overflow_field(p_size_field, size);
        |                         ^
  1 warning generated.

Unfortunately, I have no idea why it is complaining nor why your patch
resolves it but the combination of FORTIFY_SOURCE and KASAN certainly
seems like a reasonable place to start looking. I will see if I can come
up with a smaller reproducer to see if it becomes more obvious why this
code triggers this warning.

Cheers,
Nathan