Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

kcsan, seqlock: Support seqcount_latch_t

While fuzzing an arm64 kernel, Alexander Potapenko reported:

| BUG: KCSAN: data-race in ktime_get_mono_fast_ns / timekeeping_update
|
| write to 0xffffffc082e74248 of 56 bytes by interrupt on cpu 0:
| update_fast_timekeeper kernel/time/timekeeping.c:430 [inline]
| timekeeping_update+0x1d8/0x2d8 kernel/time/timekeeping.c:768
| timekeeping_advance+0x9e8/0xb78 kernel/time/timekeeping.c:2344
| update_wall_time+0x18/0x38 kernel/time/timekeeping.c:2360
| [...]
|
| read to 0xffffffc082e74258 of 8 bytes by task 5260 on cpu 1:
| __ktime_get_fast_ns kernel/time/timekeeping.c:372 [inline]
| ktime_get_mono_fast_ns+0x88/0x174 kernel/time/timekeeping.c:489
| init_srcu_struct_fields+0x40c/0x530 kernel/rcu/srcutree.c:263
| init_srcu_struct+0x14/0x20 kernel/rcu/srcutree.c:311
| [...]
|
| value changed: 0x000002f875d33266 -> 0x000002f877416866
|
| Reported by Kernel Concurrency Sanitizer on:
| CPU: 1 UID: 0 PID: 5260 Comm: syz.2.7483 Not tainted 6.12.0-rc3-dirty #78

This is a false positive data race between a seqcount latch writer and a reader
accessing stale data. Since its introduction, KCSAN has never understood the
seqcount_latch interface (due to being unannotated).

Unlike the regular seqlock interface, the seqcount_latch interface for latch
writers never has had a well-defined critical section, making it difficult to
teach tooling where the critical section starts and ends.

Introduce an instrumentable (non-raw) seqcount_latch interface, with
which we can clearly denote writer critical sections. This both helps
readability and tooling like KCSAN to understand when the writer is done
updating all latch copies.

Fixes: 88ecd153be95 ("seqlock, kcsan: Add annotations for KCSAN")
Reported-by: Alexander Potapenko <glider@google.com>
Co-developed-by: "Peter Zijlstra (Intel)" <peterz@infradead.org>
Signed-off-by: "Peter Zijlstra (Intel)" <peterz@infradead.org>
Signed-off-by: Marco Elver <elver@google.com>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Link: https://lore.kernel.org/r/20241104161910.780003-4-elver@google.com

authored by

Marco Elver and committed by
Peter Zijlstra
5c1806c4 8ab40fc2

+72 -16
+1 -1
Documentation/locking/seqlock.rst
··· 153 153 from interruption by readers. This is typically the case when the read 154 154 side can be invoked from NMI handlers. 155 155 156 - Check `raw_write_seqcount_latch()` for more information. 156 + Check `write_seqcount_latch()` for more information. 157 157 158 158 159 159 .. _seqlock_t:
+71 -15
include/linux/seqlock.h
··· 622 622 } 623 623 624 624 /** 625 + * read_seqcount_latch() - pick even/odd latch data copy 626 + * @s: Pointer to seqcount_latch_t 627 + * 628 + * See write_seqcount_latch() for details and a full reader/writer usage 629 + * example. 630 + * 631 + * Return: sequence counter raw value. Use the lowest bit as an index for 632 + * picking which data copy to read. The full counter must then be checked 633 + * with read_seqcount_latch_retry(). 634 + */ 635 + static __always_inline unsigned read_seqcount_latch(const seqcount_latch_t *s) 636 + { 637 + kcsan_atomic_next(KCSAN_SEQLOCK_REGION_MAX); 638 + return raw_read_seqcount_latch(s); 639 + } 640 + 641 + /** 625 642 * raw_read_seqcount_latch_retry() - end a seqcount_latch_t read section 626 643 * @s: Pointer to seqcount_latch_t 627 644 * @start: count, from raw_read_seqcount_latch() ··· 653 636 } 654 637 655 638 /** 639 + * read_seqcount_latch_retry() - end a seqcount_latch_t read section 640 + * @s: Pointer to seqcount_latch_t 641 + * @start: count, from read_seqcount_latch() 642 + * 643 + * Return: true if a read section retry is required, else false 644 + */ 645 + static __always_inline int 646 + read_seqcount_latch_retry(const seqcount_latch_t *s, unsigned start) 647 + { 648 + kcsan_atomic_next(0); 649 + return raw_read_seqcount_latch_retry(s, start); 650 + } 651 + 652 + /** 656 653 * raw_write_seqcount_latch() - redirect latch readers to even/odd copy 654 + * @s: Pointer to seqcount_latch_t 655 + */ 656 + static __always_inline void raw_write_seqcount_latch(seqcount_latch_t *s) 657 + { 658 + smp_wmb(); /* prior stores before incrementing "sequence" */ 659 + s->seqcount.sequence++; 660 + smp_wmb(); /* increment "sequence" before following stores */ 661 + } 662 + 663 + /** 664 + * write_seqcount_latch_begin() - redirect latch readers to odd copy 657 665 * @s: Pointer to seqcount_latch_t 658 666 * 659 667 * The latch technique is a multiversion concurrency control method that allows ··· 707 665 * 708 666 * void latch_modify(struct latch_struct *latch, ...) 709 667 * { 710 - * smp_wmb(); // Ensure that the last data[1] update is visible 711 - * latch->seq.sequence++; 712 - * smp_wmb(); // Ensure that the seqcount update is visible 713 - * 668 + * write_seqcount_latch_begin(&latch->seq); 714 669 * modify(latch->data[0], ...); 715 - * 716 - * smp_wmb(); // Ensure that the data[0] update is visible 717 - * latch->seq.sequence++; 718 - * smp_wmb(); // Ensure that the seqcount update is visible 719 - * 670 + * write_seqcount_latch(&latch->seq); 720 671 * modify(latch->data[1], ...); 672 + * write_seqcount_latch_end(&latch->seq); 721 673 * } 722 674 * 723 675 * The query will have a form like:: ··· 722 686 * unsigned seq, idx; 723 687 * 724 688 * do { 725 - * seq = raw_read_seqcount_latch(&latch->seq); 689 + * seq = read_seqcount_latch(&latch->seq); 726 690 * 727 691 * idx = seq & 0x01; 728 692 * entry = data_query(latch->data[idx], ...); 729 693 * 730 694 * // This includes needed smp_rmb() 731 - * } while (raw_read_seqcount_latch_retry(&latch->seq, seq)); 695 + * } while (read_seqcount_latch_retry(&latch->seq, seq)); 732 696 * 733 697 * return entry; 734 698 * } ··· 752 716 * When data is a dynamic data structure; one should use regular RCU 753 717 * patterns to manage the lifetimes of the objects within. 754 718 */ 755 - static inline void raw_write_seqcount_latch(seqcount_latch_t *s) 719 + static __always_inline void write_seqcount_latch_begin(seqcount_latch_t *s) 756 720 { 757 - smp_wmb(); /* prior stores before incrementing "sequence" */ 758 - s->seqcount.sequence++; 759 - smp_wmb(); /* increment "sequence" before following stores */ 721 + kcsan_nestable_atomic_begin(); 722 + raw_write_seqcount_latch(s); 723 + } 724 + 725 + /** 726 + * write_seqcount_latch() - redirect latch readers to even copy 727 + * @s: Pointer to seqcount_latch_t 728 + */ 729 + static __always_inline void write_seqcount_latch(seqcount_latch_t *s) 730 + { 731 + raw_write_seqcount_latch(s); 732 + } 733 + 734 + /** 735 + * write_seqcount_latch_end() - end a seqcount_latch_t write section 736 + * @s: Pointer to seqcount_latch_t 737 + * 738 + * Marks the end of a seqcount_latch_t writer section, after all copies of the 739 + * latch-protected data have been updated. 740 + */ 741 + static __always_inline void write_seqcount_latch_end(seqcount_latch_t *s) 742 + { 743 + kcsan_nestable_atomic_end(); 760 744 } 761 745 762 746 #define __SEQLOCK_UNLOCKED(lockname) \