[PATCH] workqueue: Fix an object aliasing bug with `work_data_bits'

classic Classic list List threaded Threaded
3 messages Options
Reply | Threaded
Open this post in threaded view
|

[PATCH] workqueue: Fix an object aliasing bug with `work_data_bits'

Maciej W. Rozycki-3
Fix an aliasing issue causing a MIPS port build error:

In file included from include/linux/srcu.h:34:0,
                 from include/linux/notifier.h:15,
                 from ./arch/mips/include/asm/uprobes.h:9,
                 from include/linux/uprobes.h:61,
                 from include/linux/mm_types.h:13,
                 from ./arch/mips/include/asm/vdso.h:14,
                 from arch/mips/vdso/vdso.h:27,
                 from arch/mips/vdso/gettimeofday.c:11:
include/linux/workqueue.h: In function 'work_static':
include/linux/workqueue.h:186:2: error: dereferencing type-punned pointer will break strict-aliasing rules [-Werror=strict-aliasing]
  return *work_data_bits(work) & WORK_STRUCT_STATIC;
  ^
cc1: all warnings being treated as errors
make[2]: *** [arch/mips/vdso/gettimeofday.o] Error 1

with a CONFIG_DEBUG_OBJECTS_WORK configuration and GCC 5.2.0.  Use a
union to switch between pointers as per ISO C language rules.

Signed-off-by: Maciej W. Rozycki <[hidden email]>
Cc: [hidden email] # v2.6.20+
---
 This has been probably missed with ports which do not use `-Werror' and
consequently let warnings scroll by unnoticed.  Yet undefined behaviour
results as the compiler is free to optimise (e.g. remove) code under the
assumption that the two objects do not alias.

 Please apply then.  This reaches back to 2.6.20 I believe.

  Maciej

linux-work-data-bits.diff
Index: linux-sfr-shell/include/linux/workqueue.h
===================================================================
--- linux-sfr-shell.orig/include/linux/workqueue.h 2016-04-22 17:31:08.000000000 +0100
+++ linux-sfr-shell/include/linux/workqueue.h 2016-05-17 05:26:18.772193000 +0100
@@ -23,7 +23,16 @@ void delayed_work_timer_fn(unsigned long
  * The first word is the work queue pointer and the flags rolled into
  * one
  */
-#define work_data_bits(work) ((unsigned long *)(&(work)->data))
+#define work_data_bits(work) \
+({ \
+ union { \
+ atomic_long_t *a; \
+ unsigned long *l; \
+ } u; \
+ \
+ u.a = &(work)->data; \
+ u.l; \
+})
 
 enum {
  WORK_STRUCT_PENDING_BIT = 0, /* work item is pending execution */
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] workqueue: Fix an object aliasing bug with `work_data_bits'

Tejun Heo-2
On Tue, May 17, 2016 at 12:04:33PM +0100, Maciej W. Rozycki wrote:

> Fix an aliasing issue causing a MIPS port build error:
>
> In file included from include/linux/srcu.h:34:0,
>                  from include/linux/notifier.h:15,
>                  from ./arch/mips/include/asm/uprobes.h:9,
>                  from include/linux/uprobes.h:61,
>                  from include/linux/mm_types.h:13,
>                  from ./arch/mips/include/asm/vdso.h:14,
>                  from arch/mips/vdso/vdso.h:27,
>                  from arch/mips/vdso/gettimeofday.c:11:
> include/linux/workqueue.h: In function 'work_static':
> include/linux/workqueue.h:186:2: error: dereferencing type-punned pointer will break strict-aliasing rules [-Werror=strict-aliasing]
>   return *work_data_bits(work) & WORK_STRUCT_STATIC;
>   ^

Umm... the kernel is built explicitly with -fno-strict-aliasing and
it's not something individual archs can opt out.

Thanks.

--
tejun
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] workqueue: Fix an object aliasing bug with `work_data_bits'

Maciej W. Rozycki-3
On Wed, 25 May 2016, Tejun Heo wrote:

> > Fix an aliasing issue causing a MIPS port build error:
> >
> > In file included from include/linux/srcu.h:34:0,
> >                  from include/linux/notifier.h:15,
> >                  from ./arch/mips/include/asm/uprobes.h:9,
> >                  from include/linux/uprobes.h:61,
> >                  from include/linux/mm_types.h:13,
> >                  from ./arch/mips/include/asm/vdso.h:14,
> >                  from arch/mips/vdso/vdso.h:27,
> >                  from arch/mips/vdso/gettimeofday.c:11:
> > include/linux/workqueue.h: In function 'work_static':
> > include/linux/workqueue.h:186:2: error: dereferencing type-punned pointer will break strict-aliasing rules [-Werror=strict-aliasing]
> >   return *work_data_bits(work) & WORK_STRUCT_STATIC;
> >   ^
>
> Umm... the kernel is built explicitly with -fno-strict-aliasing and
> it's not something individual archs can opt out.

 Hmm, good point, I had forgotten about it, it's been a while -- now I
recall there was once quite a discussion about it, when GCC switched its
default.

 However it's VDSO being built here, i.e. strictly speaking not a part of
the kernel itself, and this uses specialised build recipes so as to make
this code user-callable (PIC, among others).  Which is undoubtedly why the
error triggers so rarely.  So I guess the way to sort this out is to stick
an explicit `-fno-strict-aliasing' option along with `-fno-common' and
some other stuff already present there.  Due to the arcana of MIPS ABIs
this is unfortunately very fragile.

 I'll handle it with the MIPS port then.  Sorry to trouble you with a bad
change, thanks for the advice, and please consider the patch withdrawn.

  Maciej