On Thu, Jan 23, 2020 at 7:33 AM Will Deacon <will@xxxxxxxxxx> wrote: > > nf_remove_net_hook() uses WRITE_ONCE() to assign a 'const pointer to a > 'non-const' pointer. Cleanups to the implementation of WRITE_ONCE() mean > that this will give rise to a compiler warning, just like a plain old > assignment would do: > > | In file included from ./include/linux/export.h:43, > | from ./include/linux/linkage.h:7, > | from ./include/linux/kernel.h:8, > | from net/netfilter/core.c:9: > | net/netfilter/core.c: In function ‘nf_remove_net_hook’: > | ./include/linux/compiler.h:216:30: warning: assignment discards ‘const’ qualifier from pointer target type [-Wdiscarded-qualifiers] > | *(volatile typeof(x) *)&(x) = (val); \ > | ^ > | net/netfilter/core.c:379:3: note: in expansion of macro ‘WRITE_ONCE’ > | WRITE_ONCE(orig_ops[i], &dummy_ops); > | ^~~~~~~~~~ > > Follow the pattern used elsewhere in this file and add a cast to 'void *' > to squash the warning. > > Cc: Pablo Neira Ayuso <pablo@xxxxxxxxxxxxx> > Cc: Jozsef Kadlecsik <kadlec@xxxxxxxxxxxxx> > Cc: Florian Westphal <fw@xxxxxxxxx> > Cc: "David S. Miller" <davem@xxxxxxxxxxxxx> > Signed-off-by: Will Deacon <will@xxxxxxxxxx> > --- > net/netfilter/core.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/net/netfilter/core.c b/net/netfilter/core.c > index 78f046ec506f..3ac7c8c1548d 100644 > --- a/net/netfilter/core.c > +++ b/net/netfilter/core.c > @@ -376,7 +376,7 @@ static bool nf_remove_net_hook(struct nf_hook_entries *old, > if (orig_ops[i] != unreg) > continue; > WRITE_ONCE(old->hooks[i].hook, accept_all); > - WRITE_ONCE(orig_ops[i], &dummy_ops); > + WRITE_ONCE(orig_ops[i], (void *)&dummy_ops); Good thing it's the variable being modified was not declared const; I get spooked when I see -Wdiscarded-qualifiers because of Section 6.7.3.6 of the ISO C11 draft spec: ``` If an attempt is made to modify an object defined with a const-qualified type through use of an lvalue with non-const-qualified type, the behavior is undefined. If an attempt is made to refer to an object defined with a volatile-qualified type through use of an lvalue with non-volatile-qualified type, the behavior is undefined.133) 133) This applies to those objects that behave as if they were defined with qualified types, even if they are never actually defined as objects in the program (such as an object at a memory-mapped input/output address). ``` Which is about the modification of a const-declared variable (explicit UB which Clang actively exploits), and doesn't apply in this case. I agree that this is the best way to fix this due to the use of typeof() and it's semantics of dropping qualifiers; declaring `dummy_ops` as non-const would be another, but that is worse IMO. Thanks for the patch. Reviewed-by: Nick Desaulniers <ndesaulniers@xxxxxxxxxx> > return true; > } > > -- > 2.25.0.341.g760bfbb309-goog > -- Thanks, ~Nick Desaulniers
