On Mon, Oct 31, 2022 at 06:52:56PM +0100, Jann Horn wrote:
> We must prevent the CPU from reordering the files->count read with the
> FD table access like this, on architectures where read-read reordering is
> possible:
>
> files_lookup_fd_raw()
> close_fd()
> put_files_struct()
> atomic_read(&files->count)
>
> I would like to mark this for stable, but the stable rules explicitly say
> "no theoretical races", and given that the FD table pointer and
> files->count are explicitly stored in the same cacheline, this sort of
> reordering seems quite unlikely in practice...
Looks sane, but looking at the definition of atomic_read_acquire... ouch.
static __always_inline int
atomic_read_acquire(const atomic_t *v)
{
instrument_atomic_read(v, sizeof(*v));
return arch_atomic_read_acquire(v);
}
OK...
; git grep -n -w arch_atomic_read_acquire
include/linux/atomic/atomic-arch-fallback.h:220:#ifndef arch_atomic_read_acquire
include/linux/atomic/atomic-arch-fallback.h:222:arch_atomic_read_acquire(const atomic_t *v)
include/linux/atomic/atomic-arch-fallback.h:235:#define arch_atomic_read_acquire arch_atomic_read_acquire
include/linux/atomic/atomic-instrumented.h:35: return arch_atomic_read_acquire(v);
include/linux/atomic/atomic-long.h:529: return arch_atomic_read_acquire(v);
No arch-specific instances, so...
static __always_inline int
arch_atomic_read_acquire(const atomic_t *v)
{
int ret;
if (__native_word(atomic_t)) {
ret = smp_load_acquire(&(v)->counter);
} else {
ret = arch_atomic_read(v);
__atomic_acquire_fence();
}
return ret;
}
OK, but when would that test not be true? We have unconditional
typedef struct {
int counter;
} atomic_t;
and
#define __native_word(t) \
(sizeof(t) == sizeof(char) || sizeof(t) == sizeof(short) || \
sizeof(t) == sizeof(int) || sizeof(t) == sizeof(long))
Do we really have any architectures where a structure with one
int field does *not* have a size that would satisfy that check?
Is it future-proofing for masturbation sake, or am I missing something
real here?
