lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [thread-next>] [day] [month] [year] [list]
Message-Id: <20250318-nvmet-fcloop-v3-0-05fec0fc02f6@kernel.org>
Date: Tue, 18 Mar 2025 11:39:54 +0100
From: Daniel Wagner <wagi@...nel.org>
To: James Smart <james.smart@...adcom.com>, Christoph Hellwig <hch@....de>, 
 Sagi Grimberg <sagi@...mberg.me>, Chaitanya Kulkarni <kch@...dia.com>
Cc: Hannes Reinecke <hare@...e.de>, Keith Busch <kbusch@...nel.org>, 
 linux-nvme@...ts.infradead.org, linux-kernel@...r.kernel.org, 
 Daniel Wagner <wagi@...nel.org>
Subject: [PATCH v3 00/18] nvmet-fcloop: track resources via reference
 counting

It turns out v2 had a memory leak, which sent me down the rabbit hole
once again. The obvious leak was easy to fix. I took one reference too
much on fcloop_lsreq.

The second one was a really tricky. The fcloop_lsreq object are allocated
by either host or target. So the lifetime of these object are tied to the
lifetime of the host or target. I tried several things to avoid UAFs but
everything failed. Eventually, I decided to allocated the fcloop_lsreqs
in fcloop directly. All got way simpler and the mem leak is gone and
there are no UAFs anymore or blocked processes.

Sometimes some blktests fail but these really look like existing problems
which got uncovered by this series.

With this series the fcloop and nvmet-fc should be in way better shape
and hopefully most of the UAFs should be history. This allows futher
refactoring/cleanup/improvements or more nasty blktests.

Signed-off-by: Daniel Wagner <wagi@...nel.org>
---
Changes in v3:
- fixed memory leaks
- allocates fcloop_lsreq in fcloop directly
- prevent double free due to unregister race
- collected tags
- Link to v2: https://lore.kernel.org/r/20250311-nvmet-fcloop-v2-0-fc40cb64edea@kernel.org

Changes in v2:
- drop tport and rport ref counting, use implicit synchronisation
- a bunch of additional fixes in existing ref countig
- replaced kref with refcount
- Link to v1: https://lore.kernel.org/r/20250226-nvmet-fcloop-v1-0-c0bd83d43e6a@kernel.org

---
Daniel Wagner (18):
      nvmet-fcloop: remove nport from list on last user
      nvmet-fcloop: replace kref with refcount
      nvmet-fcloop: add ref counting to lport
      nvmet-fcloop: refactor fcloop_nport_alloc
      nvmet-fcloop: track ref counts for nports
      nvmet-fcloop: sync targetport removal
      nvmet-fcloop: update refs on tfcp_req
      nvmet-fcloop: add missing fcloop_callback_host_done
      nvmet-fcloop: prevent double port deletion
      nvmet-fcloop: allocate/free fcloop_lsreq directly
      nvmet-fc: inline nvmet_fc_delete_assoc
      nvmet-fc: inline nvmet_fc_free_hostport
      nvmet-fc: update tgtport ref per assoc
      nvmet-fc: take tgtport reference only once
      nvmet-fc: free pending reqs on tgtport unregister
      nvmet-fc: take tgtport refs for portentry
      nvmet-fc: put ref when assoc->del_work is already scheduled
      nvme-fc: do not reference lsrsp after failure

 drivers/nvme/host/fc.c       |  13 +-
 drivers/nvme/target/fc.c     | 153 +++++++++------
 drivers/nvme/target/fcloop.c | 435 ++++++++++++++++++++++++++-----------------
 3 files changed, 376 insertions(+), 225 deletions(-)
---
base-commit: a149420f548dc8e2258461522f498252554c0bbd
change-id: 20250214-nvmet-fcloop-a649738b7e6e

Best regards,
-- 
Daniel Wagner <wagi@...nel.org>


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ