From: Eric Biggers ebiggers@google.com
commit b34e7e298d7a5ed76b3aa327c240c29f1ef6dd22 upstream.
WRITE_ONCE() isn't the correct way to publish a pointer to a data structure, since it doesn't include a write memory barrier. Therefore other tasks may see that the pointer has been set but not see that the pointed-to memory has finished being initialized yet. Instead a primitive with "release" semantics is needed.
Use smp_store_release() for this.
The use of READ_ONCE() on the read side is still potentially correct if there's no control dependency, i.e. if all memory being "published" is transitively reachable via the pointer itself. But this pairing is somewhat confusing and error-prone. So just upgrade the read side to smp_load_acquire() so that it clearly pairs with smp_store_release().
Cc: Arnd Bergmann arnd@arndb.de Cc: Ingo Molnar mingo@redhat.com Cc: Kees Cook keescook@chromium.org Cc: Matthew Wilcox willy@infradead.org Cc: Russell King linux@arm.linux.org.uk Cc: Andrew Morton akpm@linux-foundation.org Fixes: 3234ac664a87 ("/dev/mem: Revoke mappings when a driver claims the region") Signed-off-by: Eric Biggers ebiggers@google.com Cc: stable stable@vger.kernel.org Acked-by: Dan Williams dan.j.williams@intel.com Link: https://lore.kernel.org/r/20200716060553.24618-1-ebiggers@kernel.org Signed-off-by: Greg Kroah-Hartman gregkh@linuxfoundation.org
--- drivers/char/mem.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-)
--- a/drivers/char/mem.c +++ b/drivers/char/mem.c @@ -814,7 +814,8 @@ static struct inode *devmem_inode; #ifdef CONFIG_IO_STRICT_DEVMEM void revoke_devmem(struct resource *res) { - struct inode *inode = READ_ONCE(devmem_inode); + /* pairs with smp_store_release() in devmem_init_inode() */ + struct inode *inode = smp_load_acquire(&devmem_inode);
/* * Check that the initialization has completed. Losing the race @@ -1028,8 +1029,11 @@ static int devmem_init_inode(void) return rc; }
- /* publish /dev/mem initialized */ - WRITE_ONCE(devmem_inode, inode); + /* + * Publish /dev/mem initialized. + * Pairs with smp_load_acquire() in revoke_devmem(). + */ + smp_store_release(&devmem_inode, inode);
return 0; }