Bug 84863 - false-positive -Warray-bounds warning with -fsanitize-coverage=object-size
Summary: false-positive -Warray-bounds warning with -fsanitize-coverage=object-size
Status: UNCONFIRMED
Alias: None
Product: gcc
Classification: Unclassified
Component: sanitizer (show other bugs)
Version: 8.0.1
: P3 normal
Target Milestone: ---
Assignee: Not yet assigned to anyone
URL:
Keywords: diagnostic
Depends on:
Blocks: Warray-bounds
  Show dependency treegraph
 
Reported: 2018-03-14 14:01 UTC by Arnd Bergmann
Modified: 2021-03-28 07:10 UTC (History)
4 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed:


Attachments
linux/net/xfrm/xfrm_output.c, preprocessed, not reduced. (350.72 KB, application/gzip)
2018-03-14 14:01 UTC, Arnd Bergmann
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Arnd Bergmann 2018-03-14 14:01:24 UTC
Created attachment 43655 [details]
linux/net/xfrm/xfrm_output.c, preprocessed, not reduced.

Among the linux kernel build warnings I see from enabling sanitizers (CONFIG_UBSAN_SANITIZE_ALL), this is one that seems interesting and not yet reported:

In file included from /git/arm-soc/include/linux/kernel.h:10,
                 from /git/arm-soc/include/linux/list.h:9,
                 from /git/arm-soc/include/linux/module.h:9,
                 from /git/arm-soc/net/xfrm/xfrm_output.c:13:
/git/arm-soc/net/xfrm/xfrm_output.c: In function 'xfrm_output_resume':
/git/arm-soc/include/linux/compiler.h:251:20: error: array subscript 4 is above array bounds of 'struct nf_hook_entries *[3]' [-Werror=array-bounds]
   __read_once_size(&(x), __u.__c, sizeof(x));  \
                    ^~~~
/git/arm-soc/include/linux/compiler.h:257:22: note: in expansion of macro '__READ_ONCE'
 #define READ_ONCE(x) __READ_ONCE(x, 1)
                      ^~~~~~~~~~~
/git/arm-soc/include/linux/rcupdate.h:351:48: note: in expansion of macro 'READ_ONCE'
  typeof(*p) *________p1 = (typeof(*p) *__force)READ_ONCE(p); \
                                                ^~~~~~~~~
/git/arm-soc/include/linux/rcupdate.h:488:2: note: in expansion of macro '__rcu_dereference_check'
  __rcu_dereference_check((p), (c) || rcu_read_lock_held(), __rcu)
  ^~~~~~~~~~~~~~~~~~~~~~~
/git/arm-soc/include/linux/rcupdate.h:546:28: note: in expansion of macro 'rcu_dereference_check'
 #define rcu_dereference(p) rcu_dereference_check(p, 0)
                            ^~~~~~~~~~~~~~~~~~~~~
/git/arm-soc/include/linux/netfilter.h:219:15: note: in expansion of macro 'rcu_dereference'
   hook_head = rcu_dereference(net->nf.hooks_arp[hook]);
               ^~~~~~~~~~~~~~~

The original function looks like

static inline int nf_hook(u_int8_t pf, unsigned int hook, struct net *net,
                          struct sock *sk, struct sk_buff *skb,
                          struct net_device *indev, struct net_device *outdev,
                          int (*okfn)(struct net *, struct sock *, struct sk_buff *))
{
        struct nf_hook_entries *hook_head = NULL;
        int ret = 1;

        rcu_read_lock();
        switch (pf) {
        case NFPROTO_IPV4:
                hook_head = rcu_dereference(net->nf.hooks_ipv4[hook]);
                break;
        case NFPROTO_IPV6:
                hook_head = rcu_dereference(net->nf.hooks_ipv6[hook]);
                break;
        case NFPROTO_ARP:
#ifdef CONFIG_NETFILTER_FAMILY_ARP
                hook_head = rcu_dereference(net->nf.hooks_arp[hook]);
#endif
                break;
        case NFPROTO_BRIDGE:
#ifdef CONFIG_NETFILTER_FAMILY_BRIDGE
                hook_head = rcu_dereference(net->nf.hooks_bridge[hook]);
#endif
                break;
#if IS_ENABLED(CONFIG_DECNET)
        case NFPROTO_DECNET:
                hook_head = rcu_dereference(net->nf.hooks_decnet[hook]);
                break;
#endif
        default:
                WARN_ON_ONCE(1);
                break;
        }

where the net->nf.hooks_* fields all have different sizes. The function is
called with constant arguments for 'pf' and 'hook', and for this caller, the
latter is out of range for the net->nf.hooks_arp[] array, but in a line that
is never reached. Reproduced with all versions that support the object-size
sanitizer (gcc-5 through 8).

With the attached preprocessed file, reproduce with

$ arm-linux-gnueabi-gcc-8.0.1 -Wall -O2 -c net/xfrm/xfrm_output.i -Werror   -fsanitize=object-size  -fno-strict-aliasing

The warning also shows up with an x86 compiler, but that causes other problems. I can produce a reduced version that works on x86 if needed.
Comment 1 Jakub Jelinek 2018-03-14 14:09:47 UTC
Never use -Werror with -fsanitize=*, those really do cause new warnings because the code intentionally is less optimized and the runtime check themselves prevent further optimizations, so warnings that depend on optimizations can't work properly.
Comment 2 Eric Gallager 2018-12-15 03:29:56 UTC
(In reply to Jakub Jelinek from comment #1)
> Never use -Werror with -fsanitize=*, those really do cause new warnings
> because the code intentionally is less optimized and the runtime check
> themselves prevent further optimizations, so warnings that depend on
> optimizations can't work properly.

Maybe gcc should explicitly forbid the 2 of them being combined and error out in the option-handling part of the driver instead.
Comment 3 Arnd Bergmann 2018-12-16 15:44:43 UTC
The problem in the kernel then is that we then have to turn off the sanitizers for the 'allmodconfig' build, since the recommended minimum regression testing for kernel changes involves building a kernel with all options (including UBSAN) enabled and checking that there are no compiler warnings.

This means we would fail to catch any build regressions in the kernel part of UBSAN, as well as any legitimate warnings that are only seen when the sanitizers are active.