mirror of
https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git
synced 2025-01-10 07:00:48 +00:00
b4b52b881c
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: 84242b82d81c54e009a2aaa74d3d9eff70babf56 7850b51b6c21812be96d0200b74cff1f40587d98 5e420fe635813e5746b296cfc8fff4853ae205a2 09186e503486da4a17f16f2f7c679e6e3e2a32f4 b5be853181a8d4a6e20f2073ccd273d6280cad88 7264235ee74f51d26fbdf97bf98c6102a460484f cc5034a5d293dd620484d1d836aa16c6764a1c8c 479826cc86118e0d87e5cefb3df5b748e0480924 5340f23df8fe27a270af3fa1a93cd07293d23dd9 df997abeebadaa4824271009e2d2b526a70a11cb 2f10d823739680d2477ce34437e8a08a53117f40 307b00c5e695857ca92fc6a4b8ab6c48f988a1b1 5d25ff7a544889bc4b749fda31778d6a18dddbcb a7ed5b3e7dca197de4da6273940a7ca6d1d756a1 c24bfa8f21b59283580043dada19a6e943b6e426 ad0eaee6195db1db1749dd46b9e6f4466793d178 9ba8376ce1e2cbf4ce44f7e4bee1d0648e10d594 dc586a60a11d0260308db1bebe788ad8973e2729 a8e9b186f153a44690ad0363a56716e7077ad28c 4e57562b4846e42cd1c2e556f0ece18c1154e116 60747828eac28836b49bed214399b0c972f19df3 c5b974bee9d2ceae4c441ae5a01e498c2674e100 cc44ba91166beb78f9cb29d5e3d41c0a2d0a7329 2c930e3d0aed1505e86e0928d323df5027817740 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 ...
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.