UAF in ip6_do_table on 4.19 kernel

[stranche@codeaurora.org]

Hi all,

We recently had a crash reported to us on the 4.19 kernel where 
ip6_do_table() appeared to be referencing a jumpstack that had already 
been freed.
Based on the dump, it appears that the scenario was a concurrent use of 
iptables-restore and active data transfer. The kernel has Florian's 
commit
to wait in xt_replace_table instead of get_counters(), 80055dab5de0 
("netfilter: x_tables: make xt_replace_table wait until old rules are 
not used
anymore"), so it appears that xt_replace_table is somehow returning 
prematurely, allowing __do_replace() to free the table while it is still 
in use.

After reviewing the code, we had a question about the following section:
	/* ... so wait for even xt_recseq on all cpus */
	for_each_possible_cpu(cpu) {
		seqcount_t *s = &per_cpu(xt_recseq, cpu);
		u32 seq = raw_read_seqcount(s);

		if (seq & 1) {
			do {
				cond_resched();
				cpu_relax();
			} while (seq == raw_read_seqcount(s));
		}
	}

Based on the other uses of seqcount locks, there should be a paired 
read_seqcount_retry() to mark the end of the read section like below:
	for_each_possible_cpu(cpu) {
		seqcount_t *s = &per_cpu(xt_recseq, cpu);
		u32 seq;

		do {
			seq = raw_read_seqcount(s);
			if (seq & 1) {
				cond_resched();
				cpu_relax();
			}
		} while (read_seqcount_retry(s, seq);
	}

These two snippets are very similar, as the original seems like it 
attempted to open-code this retry() helper, but there is a slight 
difference in
the smp_rmb() placement relative to the "retry" read of the sequence 
value.
Original:
	READ_ONCE(s->sequence);
	smp_rmb();
	... //check and resched
	READ_ONCE(s->sequence);
	smp_rmb();
	... //compare the two sequence values

Modified using read_seqcount_retry():
	READ_ONCE(s->sequence);
	smp_rmb();
	... //check and and resched
	smp_rmb();
	READ_ONCE(s->sequence);
	... //compare the two sequence values

Is it possible that this difference in ordering could lead to an 
incorrect read of the sequence in certain neurotic scenarios? 
Alternatively, is there
some other place that this jumpstack could be freed while still in use?

Thanks,
Sean

---
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora 
Forum,
a Linux Foundation Collaborative Project