OSS: soundcard: locking bug in sound_ioctl()

We shouldn't return directly here because we're still holding the &soundcard_mutex.

This bug goes all the way back to the start of git. It's strange that no one has complained about it as a runtime bug.

CC [email protected]

Signed-off-by Dan Carpenter <[email protected]>

Quoted 8 lines · On Sunday 10 October 2010 19:33:52 Dan Carpenter wrote:

We shouldn't return directly here because we're still holding the &soundcard_mutex.

This bug goes all the way back to the start of git. It's strange that no one has complained about it as a runtime bug.

CC [email protected]

Signed-off-by Dan Carpenter <[email protected]>

It was only recently converted to a mutex from the BKL, which is much more friendly to misusage because it is automatically released when the kernel sleeps or when the program exits.

The behavior was already broken with the BKL but the problem was far less visible. I fear we might be seeing more of these as fallout from the BKL removal. Sparse should be able to detect most of these cases though, so maybe we can look more carefully for them.

Hmm, actually sparse does *not* warn about sound_ioctl returning in different lock contexts. Sparse developers: is there a known limitation in sparse for this? I expected to see context warnings because sound_ioctl normally releases soundcard_mutex (previously lock_kernel) in some cases returns while holding the lock.

Quoted 5 lines · Arnd Bergmann

Hmm, actually sparse does not warn about sound_ioctl returning in different lock contexts. Sparse developers: is there a known limitation in sparse for this? I expected to see context warnings because sound_ioctl normally releases soundcard_mutex (previously lock_kernel) in some cases returns while holding the lock.

Arnd, mutexes aren't annotated in the kernel source to make use of sparse's context checking.

Quoted 8 lines · Johannes Berg

Quoted 5 lines · Arnd Bergmann

Hmm, actually sparse does not warn about sound_ioctl returning in different lock contexts. Sparse developers: is there a known limitation in sparse for this? I expected to see context warnings because sound_ioctl normally releases soundcard_mutex (previously lock_kernel) in some cases returns while holding the lock.

Quoted 2 lines

Arnd, mutexes aren't annotated in the kernel source to make use of sparse's context checking.

D'oh. I never realized this was only done for some types of locks. Is there a reason why we don't want mutexes to be annotated or do we just need someone to do it?

Quoted 9 lines · Arnd Bergmann

Quoted 8 lines · Johannes Berg

Quoted 5 lines · Arnd Bergmann

Hmm, actually sparse does not warn about sound_ioctl returning in different lock contexts. Sparse developers: is there a known limitation in sparse for this? I expected to see context warnings because sound_ioctl normally releases soundcard_mutex (previously lock_kernel) in some cases returns while holding the lock.

Quoted 6 lines

Quoted 2 lines

Arnd, mutexes aren't annotated in the kernel source to make use of sparse's context checking.

D'oh. I never realized this was only done for some types of locks. Is there a reason why we don't want mutexes to be annotated or do we just need someone to do it?

I don't know. Could be related to trylock issues, could be just historic since semaphores can't really be annotated, or could be something else entirely... I would expect a huge amount of warnings from sparse though if you "just" annotate them since there are things like rtnl_lock() which would have to propagate context.

Quoted 5 lines · Johannes Berg

I don't know. Could be related to trylock issues, could be just historic since semaphores can't really be annotated, or could be something else entirely... I would expect a huge amount of warnings from sparse though if you "just" annotate them since there are things like rtnl_lock() which would have to propagate context.

As far as I know, no reason exists to not just annotate mutexes; I think mutexes just came along later and nobody happened to add the appropriate annotations. (Also, sparse does handle trylock.)

But yes, annotating mutexes will then introduce a giant pile of lock warnings that need further annotation propagation. It will also introduce lock warnings that represent actual bugs, making it important to not just blindly propagate annotations to make warnings go away.

arch/x86/kernel/smpboot.c:100:6: warning: context imbalance in 'cpu_hotplug_driver_lock' - wrong count at exit arch/x86/kernel/smpboot.c:105:6: warning: context imbalance in 'cpu_hotplug_driver_unlock' - unexpected unlock kernel/cpu.c:27:6: warning: context imbalance in 'cpu_maps_update_begin' - wrong count at exit
kernel/cpu.c:32:6: warning: context imbalance in 'cpu_maps_update_done' - unexpected unlock
kernel/cpu.c:110:9: warning: context imbalance in 'cpu_hotplug_begin' - wrong count at exit
kernel/cpu.c:120:13: warning: context imbalance in 'cpu_hotplug_done' - unexpected unlock
kernel/params.c:565:6: warning: context imbalance in '__kernel_param_lock' - wrong count at exit
kernel/params.c:571:6: warning: context imbalance in '__kernel_param_unlock' - unexpected unlock kernel/irq/autoprobe.c:110:9: warning: context imbalance in 'probe_irq_on' - wrong count at exit kernel/irq/autoprobe.c:145:9: warning: context imbalance in 'probe_irq_mask' - unexpected unlock kernel/irq/autoprobe.c:189:9: warning: context imbalance in 'probe_irq_off' - unexpected unlock kernel/trace/trace.c:1716:26: warning: context imbalance in 's_start' - different lock contexts for basic block kernel/trace/trace.c:303:31: warning: context imbalance in 's_stop' - unexpected unlock
kernel/trace/trace.c:2277:9: warning: context imbalance in 't_start' - wrong count at exit
kernel/trace/trace.c:2280:13: warning: context imbalance in 't_stop' - unexpected unlock
kernel/trace/trace.c:3196:28: warning: context imbalance in 'tracing_read_pipe' - different lock contexts for basic block kernel/trace/trace.c:3349:28: warning: context imbalance in 'tracing_splice_read_pipe' - different lock contexts for basic block kernel/trace/trace.c:303:31: warning: context imbalance in 'tracing_buffers_read' - unexpected unlock kernel/trace/trace.c:303:31: warning: context imbalance in 'tracing_buffers_splice_read' - unexpected unlock kernel/trace/trace_stat.c:203:13: warning: context imbalance in 'stat_seq_start' - wrong count at exit kernel/trace/trace_stat.c:240:13: warning: context imbalance in 'stat_seq_stop' - unexpected unlock kernel/trace/trace_events.c:399:9: warning: context imbalance in 't_start' - wrong count at exit kernel/trace/trace_events.c:430:9: warning: context imbalance in 's_start' - wrong count at exit kernel/trace/trace_events.c:444:13: warning: context imbalance in 't_stop' - unexpected unlock kernel/trace/trace_kprobe.c:1064:13: warning: context imbalance in 'probes_seq_start' - wrong count at exit kernel/trace/trace_kprobe.c:1075:13: warning: context imbalance in 'probes_seq_stop' - unexpected unlock kernel/cgroup.c:737:6: warning: context imbalance in 'cgroup_lock' - wrong count at exit
kernel/cgroup.c:748:6: warning: context imbalance in 'cgroup_unlock' - unexpected unlock
kernel/cgroup.c:1867:6: warning: context imbalance in 'cgroup_lock_live_group' - different lock contexts for basic block kernel/cgroup.c:2155:12: warning: context imbalance in 'cgroup_create_file' - different lock contexts for basic block kernel/cgroup.c:3273:47: warning: context imbalance in 'cgroup_lock_hierarchy' - different lock contexts for basic block kernel/cgroup.c:3291:25: warning: context imbalance in 'cgroup_unlock_hierarchy' - unexpected unlock kernel/cgroup.c:3362:9: warning: context imbalance in 'cgroup_create' - unexpected unlock
kernel/cpuset.c:2449:6: warning: context imbalance in 'cpuset_unlock' - unexpected unlock
kernel/audit_watch.c:439:5: warning: context imbalance in 'audit_add_watch' - unexpected unlock kernel/audit_tree.c:662:9: warning: context imbalance in 'audit_add_tree_rule' - unexpected unlock
mm/shmem.c:943:9: warning: context imbalance in 'shmem_unuse_inode' - unexpected unlock
mm/shmem.c:1029:9: warning: context imbalance in 'shmem_unuse' - wrong count at exit
mm/mmap.c:2598:9: warning: context imbalance in 'mm_take_all_locks' - wrong count at exit
mm/mmap.c:2657:9: warning: context imbalance in 'mm_drop_all_locks' - unexpected unlock
mm/swapfile.c:1692:13: warning: context imbalance in 'swap_start' - wrong count at exit
mm/swapfile.c:1737:13: warning: context imbalance in 'swap_stop' - unexpected unlock
mm/swapfile.c:2114:17: warning: context imbalance in 'sys_swapon' - unexpected unlock
fs/super.c:239:6: warning: context imbalance in 'lock_super' - wrong count at exit
fs/super.c:245:6: warning: context imbalance in 'unlock_super' - unexpected unlock
fs/exec.c:1079:5: warning: context imbalance in 'prepare_bprm_creds' - different lock contexts for basic block arch/x86/include/asm/current.h:14:16: warning: context imbalance in 'free_bprm' - unexpected unlock
fs/exec.c:1105:6: warning: context imbalance in 'install_exec_creds' - unexpected unlock
fs/pipe.c:55:9: warning: context imbalance in 'pipe_lock_nested' - wrong count at exit
fs/pipe.c:71:17: warning: context imbalance in 'pipe_unlock' - unexpected unlock
fs/namei.c:1346:15: warning: context imbalance in 'lock_rename' - wrong count at exit
fs/namei.c:1376:6: warning: context imbalance in 'unlock_rename' - unexpected unlock
fs/namei.c:1491:36: warning: context imbalance in '__open_namei_create' - unexpected unlock
fs/namei.c:1730:20: warning: context imbalance in 'do_last' - different lock contexts for basic block fs/namei.c:1905:15: warning: context imbalance in 'lookup_create' - wrong count at exit
fs/namei.c:2040:9: warning: context imbalance in 'sys_mknodat' - unexpected unlock
fs/namei.c:2103:9: warning: context imbalance in 'sys_mkdirat' - unexpected unlock
fs/namei.c:2391:9: warning: context imbalance in 'sys_symlinkat' - unexpected unlock
fs/namei.c:2490:9: warning: context imbalance in 'sys_linkat' - unexpected unlock
fs/namei.c:2577:9: warning: context imbalance in 'vfs_rename_dir' - different lock contexts for basic block fs/namei.c:2609:9: warning: context imbalance in 'vfs_rename_other' - different lock contexts for basic block fs/readdir.c:43:9: warning: context imbalance in 'vfs_readdir' - unexpected unlock
fs/dcache.c:1792:17: warning: context imbalance in '__d_unalias' - unexpected unlock
fs/libfs.c:783:9: warning: context imbalance in 'simple_attr_read' - unexpected unlock
fs/libfs.c:815:9: warning: context imbalance in 'simple_attr_write' - unexpected unlock
fs/direct-io.c:1095:17: warning: context imbalance in 'direct_io_worker' - unexpected unlock
fs/direct-io.c:1253:9: warning: context imbalance in '__blockdev_direct_IO' - different lock contexts for basic block fs/autofs4/root.c:582:17: warning: context imbalance in 'autofs4_lookup' - unexpected unlock
fs/autofs4/waitq.c:274:25: warning: context imbalance in 'validate_request' - unexpected unlock fs/autofs4/waitq.c:366:17: warning: context imbalance in 'autofs4_wait' - different lock contexts for basic block include/linux/spinlock_api_smp.h:152:27: warning: context imbalance in 'journal_lock_updates' - wrong count at exit fs/jbd/transaction.c:484:9: warning: context imbalance in 'journal_unlock_updates' - unexpected unlock fs/proc/base.c:2313:9: warning: context imbalance in 'proc_pid_attr_write' - unexpected unlock fs/proc/proc_tty.c:107:13: warning: context imbalance in 't_start' - wrong count at exit
fs/proc/proc_tty.c:118:13: warning: context imbalance in 't_stop' - unexpected unlock
fs/sysfs/dir.c:350:6: warning: context imbalance in 'sysfs_addrm_start' - wrong count at exit
fs/sysfs/dir.c:504:6: warning: context imbalance in 'sysfs_addrm_finish' - unexpected unlock arch/x86/include/asm/current.h:14:16: warning: context imbalance in 'ata_eh_acquire' - wrong count at exit drivers/ata/libata-eh.c:495:9: warning: context imbalance in 'ata_eh_release' - unexpected unlock drivers/base/bus.c:181:17: warning: context imbalance in 'driver_unbind' - different lock contexts for basic block drivers/base/bus.c:211:17: warning: context imbalance in 'driver_bind' - different lock contexts for basic block drivers/base/bus.c:731:9: warning: context imbalance in 'bus_rescan_devices_helper' - different lock contexts for basic block drivers/base/bus.c:766:9: warning: context imbalance in 'device_reprobe' - different lock contexts for basic block drivers/base/dd.c:265:12: warning: context imbalance in '__driver_attach' - different lock contexts for basic block drivers/base/dd.c:395:17: warning: context imbalance in 'driver_detach' - different lock contexts for basic block drivers/base/power/main.c:73:6: warning: context imbalance in 'device_pm_lock' - wrong count at exit drivers/base/power/main.c:81:6: warning: context imbalance in 'device_pm_unlock' - unexpected unlock drivers/block/loop.c:1064:9: warning: context imbalance in 'loop_clr_fd' - unexpected unlock drivers/block/loop.c:1352:9: warning: context imbalance in 'lo_ioctl' - different lock contexts for basic block drivers/block/loop.c:1553:9: warning: context imbalance in 'lo_release' - different lock contexts for basic block drivers/char/tty_io.c:960:6: warning: context imbalance in 'tty_write_unlock' - unexpected unlock drivers/char/tty_io.c:966:5: warning: context imbalance in 'tty_write_lock' - wrong count at exit drivers/char/tty_io.c:1077:6: warning: context imbalance in 'tty_write_message' - wrong count at exit drivers/char/tty_mutex.c:31:9: warning: context imbalance in 'tty_lock' - wrong count at exit drivers/char/tty_mutex.c:42:9: warning: context imbalance in 'tty_unlock' - unexpected unlock
drivers/char/misc.c:66:13: warning: context imbalance in 'misc_seq_start' - wrong count at exit drivers/char/misc.c:77:13: warning: context imbalance in 'misc_seq_stop' - unexpected unlock drivers/cpuidle/cpuidle.c:138:6: warning: context imbalance in 'cpuidle_pause_and_lock' - wrong count at exit drivers/cpuidle/cpuidle.c:149:6: warning: context imbalance in 'cpuidle_resume_and_unlock' - unexpected unlock drivers/gpu/drm/drm_pci.c:215:9: warning: context imbalance in 'drm_get_pci_dev' - unexpected unlock drivers/gpu/drm/i915/i915_debugfs.c:202:9: warning: context imbalance in 'i915_gem_object_info' - unexpected unlock drivers/gpu/drm/i915/i915_debugfs.c:827:9: warning: context imbalance in 'i915_emon_status' - unexpected unlock drivers/gpu/drm/i915/i915_gem.c:144:12: warning: context imbalance in 'i915_mutex_lock_interruptible' - different lock contexts for basic block drivers/gpu/drm/i915/i915_gem.c:417:9: warning: context imbalance in 'i915_gem_shmem_pread_fast' - unexpected unlock drivers/gpu/drm/i915/i915_gem.c:554:9: warning: context imbalance in 'i915_gem_shmem_pread_slow' - unexpected unlock drivers/gpu/drm/i915/i915_gem.c:706:17: warning: context imbalance in 'i915_gem_gtt_pwrite_fast' - unexpected unlock drivers/gpu/drm/i915/i915_gem.c:847:9: warning: context imbalance in 'i915_gem_gtt_pwrite_slow' - unexpected unlock drivers/gpu/drm/i915/i915_gem.c:918:9: warning: context imbalance in 'i915_gem_shmem_pwrite_fast' - unexpected unlock drivers/gpu/drm/i915/i915_gem.c:1032:9: warning: context imbalance in 'i915_gem_shmem_pwrite_slow' - unexpected unlock drivers/gpu/drm/i915/i915_gem.c:1184:35: warning: context imbalance in 'i915_gem_set_domain_ioctl' - unexpected unlock drivers/gpu/drm/i915/i915_gem.c:1217:35: warning: context imbalance in 'i915_gem_sw_finish_ioctl' - unexpected unlock drivers/gpu/drm/i915/i915_gem.c:1529:43: warning: context imbalance in 'i915_gem_mmap_gtt_ioctl' - unexpected unlock drivers/gpu/drm/i915/i915_gem.c:3770:17: warning: context imbalance in 'i915_gem_do_execbuffer' - unexpected unlock drivers/gpu/drm/i915/i915_gem.c:4248:43: warning: context imbalance in 'i915_gem_pin_ioctl' - unexpected unlock drivers/gpu/drm/i915/i915_gem.c:4310:43: warning: context imbalance in 'i915_gem_unpin_ioctl' - unexpected unlock drivers/gpu/drm/i915/i915_gem.c:4375:35: warning: context imbalance in 'i915_gem_busy_ioctl' - unexpected unlock drivers/gpu/drm/i915/i915_gem.c:4419:43: warning: context imbalance in 'i915_gem_madvise_ioctl' - unexpected unlock drivers/hid/usbhid/hid-core.c:127:47: warning: context imbalance in 'hid_reset' - unexpected unlock drivers/input/input.c:1021:13: warning: context imbalance in 'input_devices_seq_start' - different lock contexts for basic block drivers/input/input.c:1050:17: warning: context imbalance in 'input_seq_stop' - unexpected unlock drivers/input/input.c:1143:13: warning: context imbalance in 'input_handlers_seq_start' - different lock contexts for basic block drivers/input/mouse/psmouse-base.c:1534:9: warning: context imbalance in 'psmouse_attr_set_helper' - unexpected unlock drivers/input/serio/i8042.c:129:6: warning: context imbalance in 'i8042_lock_chip' - wrong count at exit drivers/input/serio/i8042.c:135:6: warning: context imbalance in 'i8042_unlock_chip' - unexpected unlock drivers/input/serio/libps2.c:62:9: warning: context imbalance in 'ps2_begin_command' - wrong count at exit drivers/input/serio/libps2.c:72:9: warning: context imbalance in 'ps2_end_command' - unexpected unlock drivers/md/dm.c:2088:6: warning: context imbalance in 'dm_lock_md_type' - wrong count at exit
drivers/md/dm.c:2093:6: warning: context imbalance in 'dm_unlock_md_type' - unexpected unlock
drivers/md/md.c:578:17: warning: context imbalance in 'mddev_unlock' - unexpected unlock
drivers/md/md.c:546:19: warning: context imbalance in 'rdev_size_store' - different lock contexts for basic block drivers/md/md.c:546:19: warning: context imbalance in 'rdev_attr_show' - different lock contexts for basic block drivers/md/md.c:546:19: warning: context imbalance in 'rdev_attr_store' - different lock contexts for basic block drivers/md/md.c:546:19: warning: context imbalance in 'md_attr_show' - different lock contexts for basic block drivers/md/md.c:546:19: warning: context imbalance in 'md_attr_store' - different lock contexts for basic block drivers/md/md.c:4871:17: warning: context imbalance in 'autorun_devices' - different lock contexts for basic block drivers/md/md.c:546:19: warning: context imbalance in 'md_ioctl' - different lock contexts for basic block drivers/md/md.c:5911:9: warning: context imbalance in 'md_open' - different lock contexts for basic block drivers/md/md.c:546:19: warning: context imbalance in 'md_seq_show' - different lock contexts for basic block drivers/md/md.c:6985:6: warning: context imbalance in 'md_check_recovery' - different lock contexts for basic block drivers/md/md.c:7180:17: warning: context imbalance in 'md_notify_reboot' - different lock contexts for basic block drivers/pci/pci.c:2494:30: warning: context imbalance in 'pci_dev_reset' - unexpected unlock drivers/pcmcia/rsrc_nonstatic.c:281:17: warning: context imbalance in 'readable' - unexpected unlock drivers/serial/serial_core.c:1546:26: warning: context imbalance in 'uart_get' - different lock contexts for basic block drivers/serial/serial_core.c:1632:17: warning: context imbalance in 'uart_open' - unexpected unlock drivers/thermal/thermal_sys.c:74:9: warning: context imbalance in 'get_idr' - different lock contexts for basic block drivers/thermal/thermal_sys.c:88:9: warning: context imbalance in 'release_idr' - different lock contexts for basic block drivers/usb/class/usblp.c:842:9: warning: context imbalance in 'usblp_read' - unexpected unlock arch/x86/include/asm/current.h:14:16: warning: context imbalance in 'usblp_rwait_and_lock' - different lock contexts for basic block drivers/usb/core/usb.c:561:5: warning: context imbalance in 'usb_lock_device_for_reset' - different lock contexts for basic block drivers/usb/core/message.c:1605:17: warning: context imbalance in '__usb_queue_reset_device' - unexpected unlock drivers/usb/storage/transport.c:1315:17: warning: context imbalance in 'usb_stor_port_reset' - unexpected unlock drivers/usb/storage/usb.c:191:5: warning: context imbalance in 'usb_stor_pre_reset' - wrong count at exit drivers/usb/storage/usb.c:203:5: warning: context imbalance in 'usb_stor_post_reset' - unexpected unlock drivers/video/fbmem.c:48:5: warning: context imbalance in 'lock_fb_info' - wrong count at exit drivers/video/fbmem.c:1047:17: warning: context imbalance in 'do_fb_ioctl' - unexpected unlock drivers/video/fbmem.c:1608:9: warning: context imbalance in 'register_framebuffer' - unexpected unlock drivers/video/fbmem.c:1646:9: warning: context imbalance in 'unregister_framebuffer' - unexpected unlock drivers/video/fbmem.c:1696:23: warning: context imbalance in 'fb_set_suspend' - unexpected unlock drivers/video/fbmem.c:1768:17: warning: context imbalance in 'fb_new_modelist' - unexpected unlock drivers/video/fbcmap.c:276:23: warning: context imbalance in 'fb_set_user_cmap' - unexpected unlock drivers/video/console/fbcon.c:2299:9: warning: context imbalance in 'fbcon_generic_blank' - unexpected unlock sound/core/sound.c:124:25: warning: context imbalance in 'autoload_device' - unexpected unlock net/netfilter/core.c:40:9: warning: context imbalance in 'nf_register_afinfo' - unexpected unlock net/netfilter/core.c:71:21: warning: context imbalance in 'nf_register_hook' - unexpected unlock net/netfilter/nf_log.c:132:13: warning: context imbalance in 'seq_start' - wrong count at exit net/netfilter/nf_log.c:152:13: warning: context imbalance in 'seq_stop' - unexpected unlock net/netfilter/x_tables.c:710:17: warning: context imbalance in 'xt_find_table_lock' - different lock contexts for basic block net/netfilter/x_tables.c:726:6: warning: context imbalance in 'xt_table_unlock' - unexpected unlock net/netfilter/x_tables.c:733:6: warning: context imbalance in 'xt_compat_lock' - wrong count at exit net/netfilter/x_tables.c:739:6: warning: context imbalance in 'xt_compat_unlock' - unexpected unlock net/netfilter/x_tables.c:876:9: warning: context imbalance in 'xt_register_table' - unexpected unlock net/netfilter/x_tables.c:903:13: warning: context imbalance in 'xt_table_seq_start' - wrong count at exit net/netfilter/x_tables.c:922:13: warning: context imbalance in 'xt_table_seq_stop' - unexpected unlock net/netfilter/x_tables.c:985:13: warning: context imbalance in 'xt_mttg_seq_next' - wrong count at exit net/netfilter/x_tables.c:1044:17: warning: context imbalance in 'xt_mttg_seq_stop' - unexpected unlock net/netlink/genetlink.c:24:6: warning: context imbalance in 'genl_lock' - wrong count at exit net/netlink/genetlink.c:30:6: warning: context imbalance in 'genl_unlock' - unexpected unlock
net/unix/af_unix.c:919:9: warning: context imbalance in 'unix_bind' - different lock contexts for basic block

net/wireless/core.c:150:9: warning: context imbalance in 'cfg80211_get_dev_from_info' - different lock contexts for basic block net/wireless/core.c:172:9: warning: context imbalance in 'cfg80211_get_dev_from_ifindex' - wrong count at exit net/wireless/core.h:191:9: warning: context imbalance in 'cfg80211_wext_siwscan' - unexpected unlock net/wireless/core.h:191:9: warning: context imbalance in 'cfg80211_wext_giwscan' - unexpected unlock net/wireless/core.h:191:9: warning: context imbalance in 'nl80211_finish_netdev_dump' - unexpected unlock net/wireless/nl80211.c:901:12: warning: context imbalance in 'nl80211_set_wiphy' - different lock contexts for basic block net/wireless/core.h:191:9: warning: context imbalance in 'nl80211_pre_doit' - unexpected unlock net/wireless/core.h:191:9: warning: context imbalance in 'nl80211_post_doit' - unexpected unlock

Quoted 8 lines · On Monday 11 October 2010 20:54:56 Josh Triplett wrote:

Quoted 5 lines · Johannes Berg

I don't know. Could be related to trylock issues, could be just historic since semaphores can't really be annotated, or could be something else entirely... I would expect a huge amount of warnings from sparse though if you "just" annotate them since there are things like rtnl_lock() which would have to propagate context.

Quoted 8 lines

As far as I know, no reason exists to not just annotate mutexes; I think mutexes just came along later and nobody happened to add the appropriate annotations. (Also, sparse does handle trylock.)

But yes, annotating mutexes will then introduce a giant pile of lock warnings that need further annotation propagation. It will also introduce lock warnings that represent actual bugs, making it important to not just blindly propagate annotations to make warnings go away.

I've given it a try, wrapping the trylock/interruptible/killable variants into macros and the number of additional warnings was much less than I had feared. This is the diff for today's sparse with today's linux-next x86_64_defconfig:

And this is the patch I used for testing. There may still be some flaws in it, but it seems to do the trick.

Signed-off-by Arnd Bergmann <[email protected]>

include/linux/mutex.h +5 -6

index f363bc8..d4940af 100644
@@ -14,6 +14,7 @@
 #include <linux/spinlock_types.h>
 #include <linux/linkage.h>
 #include <linux/lockdep.h>
+#include <linux/compiler.h>
 
 #include <asm/atomic.h>
 

@@ -131,20 +132,35 @@ static inline int mutex_is_locked(struct mutex *lock)
  * Also see Documentation/mutex-design.txt.
  */
 #ifdef CONFIG_DEBUG_LOCK_ALLOC
-extern void mutex_lock_nested(struct mutex *lock, unsigned int subclass);
-extern int __must_check mutex_lock_interruptible_nested(struct mutex *lock,
-					unsigned int subclass);
-extern int __must_check mutex_lock_killable_nested(struct mutex *lock,
-					unsigned int subclass);
-
+extern void mutex_lock_nested(struct mutex *lock, unsigned int subclass)
+				__acquires(lock);
+extern int __must_check __mutex_lock_interruptible_nested(struct mutex *lock,
+						unsigned int subclass);

include/linux/mutex.h +1 -4

 #define mutex_lock(lock) mutex_lock_nested(lock, 0)
 #define mutex_lock_interruptible(lock) mutex_lock_interruptible_nested(lock, 0)
 #define mutex_lock_killable(lock) mutex_lock_killable_nested(lock, 0)
 #else
-extern void mutex_lock(struct mutex *lock);
-extern int __must_check mutex_lock_interruptible(struct mutex *lock);
-extern int __must_check mutex_lock_killable(struct mutex *lock);
-
+extern void mutex_lock(struct mutex *lock) __acquires(lock);

include/linux/mutex.h +1 -3

 # define mutex_lock_nested(lock, subclass) mutex_lock(lock)
 # define mutex_lock_interruptible_nested(lock, subclass) mutex_lock_interruptible(lock)
 # define mutex_lock_killable_nested(lock, subclass) mutex_lock_killable(lock)

@@ -156,8 +172,16 @@ extern int __must_check mutex_lock_killable(struct mutex *lock);
  *
  * Returns 1 if the mutex has been acquired successfully, and 0 on contention.
  */
-extern int mutex_trylock(struct mutex *lock);
-extern void mutex_unlock(struct mutex *lock);
-extern int atomic_dec_and_mutex_lock(atomic_t *cnt, struct mutex *lock);
+extern int __mutex_trylock(struct mutex *lock);

Assuming that the underlying function only returns zero/non-zero and that the actual return value doesn't matter, then you can use the __cond_lock macro from compiler.h for this:

# define __cond_lock(x,c)       ((c) ? ({ __acquire(x); 1; }) : 0)

Quoted 6 lines · On Tuesday 12 October 2010 00:23:08 Josh Triplett wrote:

Assuming that the underlying function only returns zero/non-zero and that the actual return value doesn't matter, then you can use the __cond_lock macro from compiler.h for this:

# define __cond_lock(x,c)       ((c) ? ({ __acquire(x); 1; }) : 0)

The return from mutex_lock_{killable,interruptible} is an error value, not true/false, so it actually matters. We know that the only possible error that is currently returned is -EINTR though, so we could do a similar trick and define another

#define __cond_mutex(x, c) ((!c) ? ({ __acquire(x); 0; }) : -EINTR)

My fear was that this would impact code generation.

We shouldn't return directly here because we're still holding the &soundcard_mutex.

This bug goes all the way back to the start of git. It's strange that no one has complained about it as a runtime bug.

CC [email protected]

Signed-off-by Dan Carpenter <[email protected]>

sound/oss/soundcard.c +2 -2

index 938ed94..a5ab61e 100644
@@ -392,11 +392,11 @@ static long sound_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
 	case SND_DEV_DSP:
 	case SND_DEV_DSP16:
 	case SND_DEV_AUDIO:
-		return audio_ioctl(dev, file, cmd, p);
+		ret = audio_ioctl(dev, file, cmd, p);
 		break;
 
 	case SND_DEV_MIDIN:
-		return MIDIbuf_ioctl(dev, file, cmd, p);
+		ret = MIDIbuf_ioctl(dev, file, cmd, p);
 		break;
 
 	}