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

[Nathan Chancellor]

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