]> git.ipfire.org Git - thirdparty/haproxy.git/commit
BUG/MEDIUM: server: fix race on server_atomic_sync()
authorAmaury Denoyelle <adenoyelle@haproxy.com>
Tue, 2 Jul 2024 16:14:57 +0000 (18:14 +0200)
committerAmaury Denoyelle <adenoyelle@haproxy.com>
Wed, 3 Jul 2024 07:20:24 +0000 (09:20 +0200)
commit50ae717624875cd36aac290ac5953062ee7de692
treed085be47b6b84dcf0752d91c46f45b7c0a63e2f9
parent419b79492a2ae8c9323b907b9d2da85c1208c372
BUG/MEDIUM: server: fix race on server_atomic_sync()

The following patch fixes a race condition during server addr/port
update :
  cd994407a9545a8d84e410dc0cc18c30966b70d8
  BUG/MAJOR: server/addr: fix a race during server addr:svc_port updates

The new update mechanism is implemented via an event update. It uses
thread isolation to guarantee that no other thread is accessing server
addr/port. Furthermore, to ensure server instance is not deleted just
before the event handler, server instance is lookup via its ID in proxy
tree.

However, thread isolation is only entered after server lookup. This
leaves a tiny race condition as the thread will be marked as harmless
and a concurrent thread can delete the server in the meantime. This
causes server_atomic_sync() to manipulated a deleted server instance to
reinsert it in used_server_addr backend tree. This can cause a segfault
during this operation or possibly on a future used_server_addr tree
access.

This issue was detected by criteo. Several backtraces were retrieved,
each related to server addr_node insert or delete operation, either in
srv_set_addr_desc(), or add/delete dynamic server handlers.

To fix this, simply extend thread isolation section to start it before
server lookup. This ensures that once retrieved the server cannot be
deleted until its addr/port are updated. To ensure this issue won't
happen anymore, a new BUG_ON() is added in srv_set_addr_desc().

Also note that ebpt_delete() is now called every time on delete handler
as this is a safe idempotent operation.

To reproduce these crashes, a script was executed to add then remove
different servers every second. In parallel, the following CLI command
was issued repeatdly without any delay to force multiple update on
servers port :

  set server <srv> addr 0.0.0.0 port $((1024 + RANDOM % 1024))

This must be backported at least up to 3.0. If above mentionned patch
has been selected for previous version, this commit must also be
backported on them.
src/server.c