1
0
Fork 0
alistair23-linux/fs/jffs2
Linus Torvalds b4b52b881c Wimplicit-fallthrough patches for 5.2-rc1
Hi Linus,
 
 This is my very first pull-request.  I've been working full-time as
 a kernel developer for more than two years now. During this time I've
 been fixing bugs reported by Coverity all over the tree and, as part
 of my work, I'm also contributing to the KSPP. My work in the kernel
 community has been supervised by Greg KH and Kees Cook.
 
 OK. So, after the quick introduction above, please, pull the following
 patches that mark switch cases where we are expecting to fall through.
 These patches are part of the ongoing efforts to enable -Wimplicit-fallthrough.
 They have been ignored for a long time (most of them more than 3 months,
 even after pinging multiple times), which is the reason why I've created
 this tree. Most of them have been baking in linux-next for a whole development
 cycle. And with Stephen Rothwell's help, we've had linux-next nag-emails
 going out for newly introduced code that triggers -Wimplicit-fallthrough
 to avoid gaining more of these cases while we work to remove the ones
 that are already present.
 
 I'm happy to let you know that we are getting close to completing this
 work.  Currently, there are only 32 of 2311 of these cases left to be
 addressed in linux-next.  I'm auditing every case; I take a look into
 the code and analyze it in order to determine if I'm dealing with an
 actual bug or a false positive, as explained here:
 
 https://lore.kernel.org/lkml/c2fad584-1705-a5f2-d63c-824e9b96cf50@embeddedor.com/
 
 While working on this, I've found and fixed the following missing
 break/return bugs, some of them introduced more than 5 years ago:
 
 84242b82d8
 7850b51b6c
 5e420fe635
 09186e5034
 b5be853181
 7264235ee7
 cc5034a5d2
 479826cc86
 5340f23df8
 df997abeeb
 2f10d82373
 307b00c5e6
 5d25ff7a54
 a7ed5b3e7d
 c24bfa8f21
 ad0eaee619
 9ba8376ce1
 dc586a60a1
 a8e9b186f1
 4e57562b48
 60747828ea
 c5b974bee9
 cc44ba9116
 2c930e3d0a
 
 Once this work is finish, we'll be able to universally enable
 "-Wimplicit-fallthrough" to avoid any of these kinds of bugs from
 entering the kernel again.
 
 Thanks
 
 Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
 -----BEGIN PGP SIGNATURE-----
 
 iQIzBAABCAAdFiEEkmRahXBSurMIg1YvRwW0y0cG2zEFAlzQR2IACgkQRwW0y0cG
 2zEJbQ//X930OcBtT/9DRW4XL1Jeq0Mjssz/GLX2Vpup5CwwcTROG65no80Zezf/
 yQRWnUjGX0OBv/fmUK32/nTxI/7k7NkmIXJHe0HiEF069GEENB7FT6tfDzIPjU8M
 qQkB8NsSUWJs3IH6BVynb/9MGE1VpGBDbYk7CBZRtRJT1RMM+3kQPucgiZMgUBPo
 Yd9zKwn4i/8tcOCli++EUdQ29ukMoY2R3qpK4LftdX9sXLKZBWNwQbiCwSkjnvJK
 I6FDiA7RaWH2wWGlL7BpN5RrvAXp3z8QN/JZnivIGt4ijtAyxFUL/9KOEgQpBQN2
 6TBRhfTQFM73NCyzLgGLNzvd8awem1rKGSBBUvevaPbgesgM+Of65wmmTQRhFNCt
 A7+e286X1GiK3aNcjUKrByKWm7x590EWmDzmpmICxNPdt5DHQ6EkmvBdNjnxCMrO
 aGA24l78tBN09qN45LR7wtHYuuyR0Jt9bCmeQZmz7+x3ICDHi/+Gw7XPN/eM9+T6
 lZbbINiYUyZVxOqwzkYDCsdv9+kUvu3e4rPs20NERWRpV8FEvBIyMjXAg6NAMTue
 K+ikkyMBxCvyw+NMimHJwtD7ho4FkLPcoeXb2ZGJTRHixiZAEtF1RaQ7dA05Q/SL
 gbSc0DgLZeHlLBT+BSWC2Z8SDnoIhQFXW49OmuACwCUC68NHKps=
 =k30z
 -----END PGP SIGNATURE-----

Merge tag 'Wimplicit-fallthrough-5.2-rc1' of git://git.kernel.org/pub/scm/linux/kernel/git/gustavoars/linux

Pull Wimplicit-fallthrough updates from Gustavo A. R. Silva:
 "Mark switch cases where we are expecting to fall through.

  This is part of the ongoing efforts to enable -Wimplicit-fallthrough.

  Most of them have been baking in linux-next for a whole development
  cycle. And with Stephen Rothwell's help, we've had linux-next
  nag-emails going out for newly introduced code that triggers
  -Wimplicit-fallthrough to avoid gaining more of these cases while we
  work to remove the ones that are already present.

  We are getting close to completing this work. Currently, there are
  only 32 of 2311 of these cases left to be addressed in linux-next. I'm
  auditing every case; I take a look into the code and analyze it in
  order to determine if I'm dealing with an actual bug or a false
  positive, as explained here:

      https://lore.kernel.org/lkml/c2fad584-1705-a5f2-d63c-824e9b96cf50@embeddedor.com/

  While working on this, I've found and fixed the several missing
  break/return bugs, some of them introduced more than 5 years ago.

  Once this work is finished, we'll be able to universally enable
  "-Wimplicit-fallthrough" to avoid any of these kinds of bugs from
  entering the kernel again"

* tag 'Wimplicit-fallthrough-5.2-rc1' of git://git.kernel.org/pub/scm/linux/kernel/git/gustavoars/linux: (27 commits)
  memstick: mark expected switch fall-throughs
  drm/nouveau/nvkm: mark expected switch fall-throughs
  NFC: st21nfca: Fix fall-through warnings
  NFC: pn533: mark expected switch fall-throughs
  block: Mark expected switch fall-throughs
  ASN.1: mark expected switch fall-through
  lib/cmdline.c: mark expected switch fall-throughs
  lib: zstd: Mark expected switch fall-throughs
  scsi: sym53c8xx_2: sym_nvram: Mark expected switch fall-through
  scsi: sym53c8xx_2: sym_hipd: mark expected switch fall-throughs
  scsi: ppa: mark expected switch fall-through
  scsi: osst: mark expected switch fall-throughs
  scsi: lpfc: lpfc_scsi: Mark expected switch fall-throughs
  scsi: lpfc: lpfc_nvme: Mark expected switch fall-through
  scsi: lpfc: lpfc_nportdisc: Mark expected switch fall-through
  scsi: lpfc: lpfc_hbadisc: Mark expected switch fall-throughs
  scsi: lpfc: lpfc_els: Mark expected switch fall-throughs
  scsi: lpfc: lpfc_ct: Mark expected switch fall-throughs
  scsi: imm: mark expected switch fall-throughs
  scsi: csiostor: csio_wr: mark expected switch fall-through
  ...
2019-05-07 12:48:10 -07:00
..
Kconfig fs/*/Kconfig: drop links to 404-compliant http://acl.bestbits.at 2018-01-01 12:45:37 -07:00
LICENCE
Makefile License cleanup: add SPDX GPL-2.0 license identifier to files with no license 2017-11-02 11:10:55 +01:00
README.Locking jffs2: Fix page lock / f->sem deadlock 2016-02-25 11:11:26 +00:00
TODO Fix common misspellings 2011-03-31 11:26:23 -03:00
acl.c Convert jffs2 acl to struct_size 2018-06-12 16:19:22 -07:00
acl.h Convert jffs2 acl to struct_size 2018-06-12 16:19:22 -07:00
background.c signal: Remove the siginfo paramater from kernel_dqueue_signal 2018-09-11 21:19:14 +02:00
build.c Fix directory hardlinks from deleted directories 2016-02-25 11:11:28 +00:00
compr.c jffs2: Use pr_fmt and remove jffs: from formats 2012-03-27 00:40:19 +01:00
compr.h jffs2: add compr=lzo and compr=zlib options 2011-10-19 17:22:21 +03:00
compr_lzo.c jffs2: Remove unnecessary OOM messages 2012-03-27 00:21:58 +01:00
compr_rtime.c jffs2: Fix segmentation fault found in stress test 2014-03-10 22:42:28 -07:00
compr_rubin.c jffs2: compr_rubin: Remove unused function 2015-01-12 20:40:03 -08:00
compr_zlib.c initramfs: support initramfs that is bigger than 2GiB 2014-08-08 15:57:26 -07:00
debug.c mm, fs: get rid of PAGE_CACHE_* and page_cache_{get,release} macros 2016-04-04 10:41:08 -07:00
debug.h jffs2: Standardize JFFS_<LEVEL> uses 2012-03-27 00:42:14 +01:00
dir.c jffs2: use 64-bit intermediate timestamps 2018-07-18 16:43:58 +02:00
erase.c mtd: Unconditionally update ->fail_addr and ->addr in part_erase() 2018-03-15 18:22:26 +01:00
file.c jffs2: use 64-bit intermediate timestamps 2018-07-18 16:43:58 +02:00
fs.c fs: mark expected switch fall-throughs 2019-04-08 18:21:02 -05:00
gc.c mm, fs: get rid of PAGE_CACHE_* and page_cache_{get,release} macros 2016-04-04 10:41:08 -07:00
ioctl.c jffs2: Update copyright notices 2010-08-08 14:15:22 +01:00
jffs2_fs_i.h jffs2: Update copyright notices 2010-08-08 14:15:22 +01:00
jffs2_fs_sb.h jffs2: Improve post-mount CRC scan efficiency 2016-02-29 22:29:10 +00:00
malloc.c jffs2: drop null test before destroy functions 2015-09-21 17:04:57 -07:00
nodelist.c mm, fs: get rid of PAGE_CACHE_* and page_cache_{get,release} macros 2016-04-04 10:41:08 -07:00
nodelist.h Fix directory hardlinks from deleted directories 2016-02-25 11:11:28 +00:00
nodemgmt.c sched/headers: Prepare to move signal wakeup & sigpending methods from <linux/sched.h> into <linux/sched/signal.h> 2017-03-02 08:42:32 +01:00
os-linux.h jffs2: use unsigned 32-bit timstamps consistently 2018-07-18 16:44:01 +02:00
read.c jffs2: Use pr_fmt and remove jffs: from formats 2012-03-27 00:40:19 +01:00
readinode.c jffs2: fix use-after-free on symlink traversal 2019-04-01 00:31:02 -04:00
scan.c vfs: make the string hashes salt the hash 2016-06-10 20:21:46 -07:00
security.c switch xattr_handler->set() to passing dentry and inode separately 2016-05-27 15:39:43 -04:00
summary.c vfs: make the string hashes salt the hash 2016-06-10 20:21:46 -07:00
summary.h [JFFS2] Fix allocation of summary buffer 2008-08-01 10:07:51 +01:00
super.c jffs2: switch to ->free_inode() 2019-05-01 22:43:25 -04:00
symlink.c vfs: remove ".readlink = generic_readlink" assignments 2016-12-09 16:45:04 +01:00
wbuf.c treewide: kmalloc() -> kmalloc_array() 2018-06-12 16:19:22 -07:00
write.c vfs: make the string hashes salt the hash 2016-06-10 20:21:46 -07:00
writev.c mtd: harmonize mtd_writev usage 2012-01-09 18:26:19 +00:00
xattr.c xattr handlers: Simplify list operation 2015-12-13 19:46:12 -05:00
xattr.h jffs2: Remove jffs2_{get,set,remove}xattr macros 2016-10-06 22:17:38 -04:00
xattr_trusted.c switch xattr_handler->set() to passing dentry and inode separately 2016-05-27 15:39:43 -04:00
xattr_user.c switch xattr_handler->set() to passing dentry and inode separately 2016-05-27 15:39:43 -04:00

README.Locking

	JFFS2 LOCKING DOCUMENTATION
	---------------------------

This document attempts to describe the existing locking rules for
JFFS2. It is not expected to remain perfectly up to date, but ought to
be fairly close.


	alloc_sem
	---------

The alloc_sem is a per-filesystem mutex, used primarily to ensure
contiguous allocation of space on the medium. It is automatically
obtained during space allocations (jffs2_reserve_space()) and freed
upon write completion (jffs2_complete_reservation()). Note that
the garbage collector will obtain this right at the beginning of
jffs2_garbage_collect_pass() and release it at the end, thereby
preventing any other write activity on the file system during a
garbage collect pass.

When writing new nodes, the alloc_sem must be held until the new nodes
have been properly linked into the data structures for the inode to
which they belong. This is for the benefit of NAND flash - adding new
nodes to an inode may obsolete old ones, and by holding the alloc_sem
until this happens we ensure that any data in the write-buffer at the
time this happens are part of the new node, not just something that
was written afterwards. Hence, we can ensure the newly-obsoleted nodes
don't actually get erased until the write-buffer has been flushed to
the medium.

With the introduction of NAND flash support and the write-buffer, 
the alloc_sem is also used to protect the wbuf-related members of the
jffs2_sb_info structure. Atomically reading the wbuf_len member to see
if the wbuf is currently holding any data is permitted, though.

Ordering constraints: See f->sem.


	File Mutex f->sem
	---------------------

This is the JFFS2-internal equivalent of the inode mutex i->i_sem.
It protects the contents of the jffs2_inode_info private inode data,
including the linked list of node fragments (but see the notes below on
erase_completion_lock), etc.

The reason that the i_sem itself isn't used for this purpose is to
avoid deadlocks with garbage collection -- the VFS will lock the i_sem
before calling a function which may need to allocate space. The
allocation may trigger garbage-collection, which may need to move a
node belonging to the inode which was locked in the first place by the
VFS. If the garbage collection code were to attempt to lock the i_sem
of the inode from which it's garbage-collecting a physical node, this
lead to deadlock, unless we played games with unlocking the i_sem
before calling the space allocation functions.

Instead of playing such games, we just have an extra internal
mutex, which is obtained by the garbage collection code and also
by the normal file system code _after_ allocation of space.

Ordering constraints: 

	1. Never attempt to allocate space or lock alloc_sem with 
	   any f->sem held.
	2. Never attempt to lock two file mutexes in one thread.
	   No ordering rules have been made for doing so.
	3. Never lock a page cache page with f->sem held.


	erase_completion_lock spinlock
	------------------------------

This is used to serialise access to the eraseblock lists, to the
per-eraseblock lists of physical jffs2_raw_node_ref structures, and
(NB) the per-inode list of physical nodes. The latter is a special
case - see below.

As the MTD API no longer permits erase-completion callback functions
to be called from bottom-half (timer) context (on the basis that nobody
ever actually implemented such a thing), it's now sufficient to use
a simple spin_lock() rather than spin_lock_bh().

Note that the per-inode list of physical nodes (f->nodes) is a special
case. Any changes to _valid_ nodes (i.e. ->flash_offset & 1 == 0) in
the list are protected by the file mutex f->sem. But the erase code
may remove _obsolete_ nodes from the list while holding only the
erase_completion_lock. So you can walk the list only while holding the
erase_completion_lock, and can drop the lock temporarily mid-walk as
long as the pointer you're holding is to a _valid_ node, not an
obsolete one.

The erase_completion_lock is also used to protect the c->gc_task
pointer when the garbage collection thread exits. The code to kill the
GC thread locks it, sends the signal, then unlocks it - while the GC
thread itself locks it, zeroes c->gc_task, then unlocks on the exit path.


	inocache_lock spinlock
	----------------------

This spinlock protects the hashed list (c->inocache_list) of the
in-core jffs2_inode_cache objects (each inode in JFFS2 has the
correspondent jffs2_inode_cache object). So, the inocache_lock
has to be locked while walking the c->inocache_list hash buckets.

This spinlock also covers allocation of new inode numbers, which is
currently just '++->highest_ino++', but might one day get more complicated
if we need to deal with wrapping after 4 milliard inode numbers are used.

Note, the f->sem guarantees that the correspondent jffs2_inode_cache
will not be removed. So, it is allowed to access it without locking
the inocache_lock spinlock. 

Ordering constraints: 

	If both erase_completion_lock and inocache_lock are needed, the
	c->erase_completion has to be acquired first.


	erase_free_sem
	--------------

This mutex is only used by the erase code which frees obsolete node
references and the jffs2_garbage_collect_deletion_dirent() function.
The latter function on NAND flash must read _obsolete_ nodes to
determine whether the 'deletion dirent' under consideration can be
discarded or whether it is still required to show that an inode has
been unlinked. Because reading from the flash may sleep, the
erase_completion_lock cannot be held, so an alternative, more
heavyweight lock was required to prevent the erase code from freeing
the jffs2_raw_node_ref structures in question while the garbage
collection code is looking at them.

Suggestions for alternative solutions to this problem would be welcomed.


	wbuf_sem
	--------

This read/write semaphore protects against concurrent access to the
write-behind buffer ('wbuf') used for flash chips where we must write
in blocks. It protects both the contents of the wbuf and the metadata
which indicates which flash region (if any) is currently covered by 
the buffer.

Ordering constraints:
	Lock wbuf_sem last, after the alloc_sem or and f->sem.


	c->xattr_sem
	------------

This read/write semaphore protects against concurrent access to the
xattr related objects which include stuff in superblock and ic->xref.
In read-only path, write-semaphore is too much exclusion. It's enough
by read-semaphore. But you must hold write-semaphore when updating,
creating or deleting any xattr related object.

Once xattr_sem released, there would be no assurance for the existence
of those objects. Thus, a series of processes is often required to retry,
when updating such a object is necessary under holding read semaphore.
For example, do_jffs2_getxattr() holds read-semaphore to scan xref and
xdatum at first. But it retries this process with holding write-semaphore
after release read-semaphore, if it's necessary to load name/value pair
from medium.

Ordering constraints:
	Lock xattr_sem last, after the alloc_sem.