linux/net/core
Thomas Gleixner 2dcb96bacc net: core: Correct the sock::sk_lock.owned lockdep annotations
lock_sock_fast() and lock_sock_nested() contain lockdep annotations for the
sock::sk_lock.owned 'mutex'. sock::sk_lock.owned is not a regular mutex. It
is just lockdep wise equivalent. In fact it's an open coded trivial mutex
implementation with some interesting features.

sock::sk_lock.slock is a regular spinlock protecting the 'mutex'
representation sock::sk_lock.owned which is a plain boolean. If 'owned' is
true, then some other task holds the 'mutex', otherwise it is uncontended.
As this locking construct is obviously endangered by lock ordering issues as
any other locking primitive it got lockdep annotated via a dedicated
dependency map sock::sk_lock.dep_map which has to be updated at the lock
and unlock sites.

lock_sock_nested() is a straight forward 'mutex' lock operation:

  might_sleep();
  spin_lock_bh(sock::sk_lock.slock)
  while (!try_lock(sock::sk_lock.owned)) {
      spin_unlock_bh(sock::sk_lock.slock);
      wait_for_release();
      spin_lock_bh(sock::sk_lock.slock);
  }

The lockdep annotation for sock::sk_lock.owned is for unknown reasons
_after_ the lock has been acquired, i.e. after the code block above and
after releasing sock::sk_lock.slock, but inside the bottom halves disabled
region:

  spin_unlock(sock::sk_lock.slock);
  mutex_acquire(&sk->sk_lock.dep_map, subclass, 0, _RET_IP_);
  local_bh_enable();

The placement after the unlock is obvious because otherwise the
mutex_acquire() would nest into the spin lock held region.

But that's from the lockdep perspective still the wrong place:

 1) The mutex_acquire() is issued _after_ the successful acquisition which
    is pointless because in a dead lock scenario this point is never
    reached which means that if the deadlock is the first instance of
    exposing the wrong lock order lockdep does not have a chance to detect
    it.

 2) It only works because lockdep is rather lax on the context from which
    the mutex_acquire() is issued. Acquiring a mutex inside a bottom halves
    and therefore non-preemptible region is obviously invalid, except for a
    trylock which is clearly not the case here.

    This 'works' stops working on RT enabled kernels where the bottom halves
    serialization is done via a local lock, which exposes this misplacement
    because the 'mutex' and the local lock nest the wrong way around and
    lockdep complains rightfully about a lock inversion.

The placement is wrong since the initial commit a5b5bb9a05 ("[PATCH]
lockdep: annotate sk_locks") which introduced this.

Fix it by moving the mutex_acquire() in front of the actual lock
acquisition, which is what the regular mutex_lock() operation does as well.

lock_sock_fast() is not that straight forward. It looks at the first glance
like a convoluted trylock operation:

  spin_lock_bh(sock::sk_lock.slock)
  if (!sock::sk_lock.owned)
      return false;
  while (!try_lock(sock::sk_lock.owned)) {
      spin_unlock_bh(sock::sk_lock.slock);
      wait_for_release();
      spin_lock_bh(sock::sk_lock.slock);
  }
  spin_unlock(sock::sk_lock.slock);
  mutex_acquire(&sk->sk_lock.dep_map, subclass, 0, _RET_IP_);
  local_bh_enable();
  return true;

But that's not the case: lock_sock_fast() is an interesting optimization
for short critical sections which can run with bottom halves disabled and
sock::sk_lock.slock held. This allows to shortcut the 'mutex' operation in
the non contended case by preventing other lockers to acquire
sock::sk_lock.owned because they are blocked on sock::sk_lock.slock, which
in turn avoids the overhead of doing the heavy processing in release_sock()
including waking up wait queue waiters.

In the contended case, i.e. when sock::sk_lock.owned == true the behavior
is the same as lock_sock_nested().

Semantically this shortcut means, that the task acquired the 'mutex' even
if it does not touch the sock::sk_lock.owned field in the non-contended
case. Not telling lockdep about this shortcut acquisition is hiding
potential lock ordering violations in the fast path.

As a consequence the same reasoning as for the above lock_sock_nested()
case vs. the placement of the lockdep annotation applies.

The current placement of the lockdep annotation was just copied from
the original lock_sock(), now renamed to lock_sock_nested(),
implementation.

Fix this by moving the mutex_acquire() in front of the actual lock
acquisition and adding the corresponding mutex_release() into
unlock_sock_fast(). Also document the fast path return case with a comment.

Reported-by: Sebastian Siewior <bigeasy@linutronix.de>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Cc: netdev@vger.kernel.org
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Jakub Kicinski <kuba@kernel.org>
Cc: Eric Dumazet <edumazet@google.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
2021-09-19 12:48:06 +01:00
..
bpf_sk_storage.c net: in_irq() cleanup 2021-08-13 14:09:19 -07:00
datagram.c udp: fix skb_copy_and_csum_datagram with odd segment sizes 2021-02-04 18:56:56 -08:00
datagram.h
dev_addr_lists.c net-next: When a bond have a massive amount of VLANs with IPv6 addresses, performance of changing link state, attaching a VRF, changing an IPv6 address, etc. go down dramtically. 2021-08-25 10:29:07 +01:00
dev_ioctl.c net: core: don't call SIOCBRADD/DELIF for non-bridge devices 2021-08-05 11:36:59 +01:00
dev.c net: in_irq() cleanup 2021-08-13 14:09:19 -07:00
devlink.c devlink: Clear whole devlink_flash_notify struct 2021-08-14 13:59:10 +01:00
drop_monitor.c net: Remove redundant if statements 2021-08-05 13:27:50 +01:00
dst_cache.c
dst.c net: Remove redundant if statements 2021-08-05 13:27:50 +01:00
failover.c
fib_notifier.c
fib_rules.c memcg: enable accounting for IP address and routing-related objects 2021-07-20 06:00:38 -07:00
filter.c bpf: tcp: Allow bpf-tcp-cc to call bpf_(get|set)sockopt 2021-08-25 17:40:35 -07:00
flow_dissector.c Merge git://git.kernel.org/pub/scm/linux/kernel/git/netdev/net 2021-07-31 09:14:46 -07:00
flow_offload.c net: Fix offloading indirect devices dependency on qdisc order creation 2021-08-19 13:19:30 +01:00
gen_estimator.c net_sched: gen_estimator: support large ewma log 2021-01-15 18:11:06 -08:00
gen_stats.c
gro_cells.c gro_cells: reduce number of synchronize_net() calls 2020-11-25 11:28:12 -08:00
hwbm.c
link_watch.c net: linkwatch: fix failure to restore device state across suspend/resume 2021-08-11 14:43:16 -07:00
lwt_bpf.c lwt_bpf: Replace preempt_disable() with migrate_disable() 2020-12-07 11:53:40 -08:00
lwtunnel.c netfilter: add netfilter hooks to SRv6 data plane 2021-08-30 01:51:36 +02:00
Makefile sock_map: Relax config dependency to CONFIG_NET 2021-07-15 18:17:49 -07:00
neighbour.c net: Support filtering interfaces on no master 2021-08-10 16:03:34 -07:00
net_namespace.c net: net_namespace: Optimize the code 2021-08-18 10:34:48 +01:00
net-procfs.c net: procfs: add seq_puts() statement for dev_mcast 2021-08-18 10:13:20 +01:00
net-sysfs.c net-sysfs: remove possible sleep from an RCU read-side critical section 2021-03-22 13:28:13 -07:00
net-sysfs.h
net-traces.c tcp: add tracepoint for checksum errors 2021-05-14 15:26:03 -07:00
netclassid_cgroup.c bpf, cgroups: Fix cgroup v2 fallback on v1/v2 mixed mode 2021-09-13 16:35:58 -07:00
netevent.c net: core: Correct function name netevent_unregister_notifier() in the kerneldoc 2021-03-28 17:56:56 -07:00
netpoll.c asm-generic/unaligned: Unify asm/unaligned.h around struct helper 2021-07-02 12:43:40 -07:00
netprio_cgroup.c bpf, cgroups: Fix cgroup v2 fallback on v1/v2 mixed mode 2021-09-13 16:35:58 -07:00
page_pool.c page_pool: use relaxed atomic for release side accounting 2021-08-24 10:46:31 +01:00
pktgen.c pktgen: remove unused variable 2021-09-03 11:48:28 +01:00
ptp_classifier.c bpf: Refactor BPF_PROG_RUN into a function 2021-08-17 00:45:07 +02:00
request_sock.c
rtnetlink.c Merge git://git.kernel.org/pub/scm/linux/kernel/git/netdev/net 2021-08-26 17:57:57 -07:00
scm.c memcg: enable accounting for scm_fp_list objects 2021-07-20 06:00:38 -07:00
secure_seq.c
selftests.c net: selftests: add MTU test 2021-07-22 00:52:04 -07:00
skbuff.c skbuff: clean up inconsistent indenting 2021-09-03 11:51:26 +01:00
skmsg.c bpf, sockmap: Fix memleak on ingress msg enqueue 2021-07-27 14:55:30 -07:00
sock_diag.c
sock_map.c af_unix: Add unix_stream_proto for sockmap 2021-08-16 18:43:39 -07:00
sock_reuseport.c tcp: Add stats for socket migration. 2021-06-23 12:56:08 -07:00
sock.c net: core: Correct the sock::sk_lock.owned lockdep annotations 2021-09-19 12:48:06 +01:00
stream.c
sysctl_net_core.c net: change netdev_unregister_timeout_secs min value to 1 2021-03-25 17:24:06 -07:00
timestamping.c
tso.c
utils.c
xdp.c xdp: Move the rxq_info.mem clearing to unreg_mem_model() 2021-06-28 23:07:59 +02:00