On Wed, Mar 11, 2020 at 07:58:09PM -0700, Andrew Morton wrote: > > The patch titled > Subject: lib/list: prevent compiler reloads inside 'safe' list iteration > has been added to the -mm tree. Its filename is > list-prevent-compiler-reloads-inside-safe-list-iteration.patch > > This patch should soon appear at > http://ozlabs.org/~akpm/mmots/broken-out/list-prevent-compiler-reloads-inside-safe-list-iteration.patch > and later at > http://ozlabs.org/~akpm/mmotm/broken-out/list-prevent-compiler-reloads-inside-safe-list-iteration.patch > > Before you just go and hit "reply", please: > a) Consider who else should be cc'ed > b) Prefer to cc a suitable mailing list as well > c) Ideally: find the original patch on the mailing list and do a > reply-to-all to that, adding suitable additional cc's > > *** Remember to use Documentation/process/submit-checklist.rst when testing your code *** > > The -mm tree is included into linux-next and is updated > there every 3-4 working days > > ------------------------------------------------------ > From: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> > Subject: lib/list: prevent compiler reloads inside 'safe' list iteration > > Instruct the compiler to read the next element in the list iteration > once, and that it is not allowed to reload the value from the stale > element later. This is important as during the course of the safe > iteration, the stale element may be poisoned (unbeknownst to the > compiler). > > This helps prevent kcsan warnings over 'unsafe' conduct in releasing the > list elements during list_for_each_entry_safe() and friends. > > Link: http://lkml.kernel.org/r/20200310092119.14965-1-chris@xxxxxxxxxxxxxxxxxx > Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> > Cc: "Paul E. McKenney" <paulmck@xxxxxxxxxx> > Cc: Randy Dunlap <rdunlap@xxxxxxxxxxxxx> > Cc: David Laight <David.Laight@xxxxxxxxxx> > Cc: Mark Rutland <mark.rutland@xxxxxxx> > Cc: Marco Elver <elver@xxxxxxxxxx> > Cc: <stable@xxxxxxxxxxxxxxx> > Signed-off-by: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx> One possible concern is that we are overloading the suffix "_safe()", but that looks better to me than yet another explosion of this API. It would be good to have uses, of course. Reviewed-by: Paul E. McKenney <paulmck@xxxxxxxxxx> > --- > > include/linux/list.h | 50 +++++++++++++++++++++++++++++------------ > 1 file changed, 36 insertions(+), 14 deletions(-) > > --- a/include/linux/list.h~list-prevent-compiler-reloads-inside-safe-list-iteration > +++ a/include/linux/list.h > @@ -537,6 +537,17 @@ static inline void list_splice_tail_init > list_entry((pos)->member.next, typeof(*(pos)), member) > > /** > + * list_next_entry_safe - get the next element in list [once] > + * @pos: the type * to cursor > + * @member: the name of the list_head within the struct. > + * > + * Like list_next_entry() but prevents the compiler from reloading the > + * next element. > + */ > +#define list_next_entry_safe(pos, member) \ > + list_entry(READ_ONCE((pos)->member.next), typeof(*(pos)), member) > + > +/** > * list_prev_entry - get the prev element in list > * @pos: the type * to cursor > * @member: the name of the list_head within the struct. > @@ -545,6 +556,17 @@ static inline void list_splice_tail_init > list_entry((pos)->member.prev, typeof(*(pos)), member) > > /** > + * list_prev_entry_safe - get the prev element in list [once] > + * @pos: the type * to cursor > + * @member: the name of the list_head within the struct. > + * > + * Like list_prev_entry() but prevents the compiler from reloading the > + * previous element. > + */ > +#define list_prev_entry_safe(pos, member) \ > + list_entry(READ_ONCE((pos)->member.prev), typeof(*(pos)), member) > + > +/** > * list_for_each - iterate over a list > * @pos: the &struct list_head to use as a loop cursor. > * @head: the head for your list. > @@ -686,9 +708,9 @@ static inline void list_splice_tail_init > */ > #define list_for_each_entry_safe(pos, n, head, member) \ > for (pos = list_first_entry(head, typeof(*pos), member), \ > - n = list_next_entry(pos, member); \ > + n = list_next_entry_safe(pos, member); \ > &pos->member != (head); \ > - pos = n, n = list_next_entry(n, member)) > + pos = n, n = list_next_entry_safe(n, member)) > > /** > * list_for_each_entry_safe_continue - continue list iteration safe against removal > @@ -700,11 +722,11 @@ static inline void list_splice_tail_init > * Iterate over list of given type, continuing after current point, > * safe against removal of list entry. > */ > -#define list_for_each_entry_safe_continue(pos, n, head, member) \ > - for (pos = list_next_entry(pos, member), \ > - n = list_next_entry(pos, member); \ > - &pos->member != (head); \ > - pos = n, n = list_next_entry(n, member)) > +#define list_for_each_entry_safe_continue(pos, n, head, member) \ > + for (pos = list_next_entry(pos, member), \ > + n = list_next_entry_safe(pos, member); \ > + &pos->member != (head); \ > + pos = n, n = list_next_entry_safe(n, member)) > > /** > * list_for_each_entry_safe_from - iterate over list from current point safe against removal > @@ -716,10 +738,10 @@ static inline void list_splice_tail_init > * Iterate over list of given type from current point, safe against > * removal of list entry. > */ > -#define list_for_each_entry_safe_from(pos, n, head, member) \ > - for (n = list_next_entry(pos, member); \ > - &pos->member != (head); \ > - pos = n, n = list_next_entry(n, member)) > +#define list_for_each_entry_safe_from(pos, n, head, member) \ > + for (n = list_next_entry_safe(pos, member); \ > + &pos->member != (head); \ > + pos = n, n = list_next_entry_safe(n, member)) > > /** > * list_for_each_entry_safe_reverse - iterate backwards over list safe against removal > @@ -733,9 +755,9 @@ static inline void list_splice_tail_init > */ > #define list_for_each_entry_safe_reverse(pos, n, head, member) \ > for (pos = list_last_entry(head, typeof(*pos), member), \ > - n = list_prev_entry(pos, member); \ > + n = list_prev_entry_safe(pos, member); \ > &pos->member != (head); \ > - pos = n, n = list_prev_entry(n, member)) > + pos = n, n = list_prev_entry_safe(n, member)) > > /** > * list_safe_reset_next - reset a stale list_for_each_entry_safe loop > @@ -750,7 +772,7 @@ static inline void list_splice_tail_init > * completing the current iteration of the loop body. > */ > #define list_safe_reset_next(pos, n, member) \ > - n = list_next_entry(pos, member) > + n = list_next_entry_safe(pos, member) > > /* > * Double linked lists with a single pointer list head. > _ > > Patches currently in -mm which might be from chris@xxxxxxxxxxxxxxxxxx are > > list-prevent-compiler-reloads-inside-safe-list-iteration.patch >
