Skip to content

Commit 1dfe1c0

Browse files
author
Martin Belanger
committed
swig: fix bugs introduced in libnvmf_context refactor
Three bugs were introduced when libnvmf_context replaced the old libnvme_fabrics_config parameter in the connect/create flow: 1. The %pythonappend GC guard for connect() had a stale signature that still included the removed fctx parameter. SWIG silently ignores a mismatched %pythonappend, so self.__host = h never ran, leaving a latent use-after-free where Python could collect the host object while the ctrl was still alive. 2. The duplicate_connect check in connect() had an inverted condition (missing !) and belonged in the library, not the binding. Move it to libnvmf_add_ctrl() in fabrics.c where it applies uniformly to all callers, C and Python alike. 3. fctx_set_fabrics_config was missing its %rename directive, exposing it to Python as fctx.fctx_set_fabrics_config() instead of fctx.set_fabrics_config() like all other libnvmf_context methods. Signed-off-by: Martin Belanger <[email protected]> Assisted-by: Claude Sonnet 4.6 <[email protected]>
1 parent af40e0b commit 1dfe1c0

2 files changed

Lines changed: 6 additions & 9 deletions

File tree

libnvme/libnvme/nvme.i

Lines changed: 2 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -453,6 +453,7 @@ struct libnvme_ns {
453453
%rename(set_connection) libnvmf_context::fctx_set_connection;
454454
%rename(set_persistent) libnvmf_context::fctx_set_persistent;
455455
%rename(set_device) libnvmf_context::fctx_set_device;
456+
%rename(set_fabrics_config) libnvmf_context::fctx_set_fabrics_config;
456457

457458
struct libnvmf_context {};
458459

@@ -735,8 +736,7 @@ struct libnvmf_context {};
735736
}
736737
%};
737738

738-
%pythonappend libnvme_ctrl::connect(struct libnvme_host *h,
739-
struct libnvmf_context *fctx) {
739+
%pythonappend libnvme_ctrl::connect(struct libnvme_host *h) {
740740
self.__host = h # Keep a reference to parent to ensure ctrl obj gets GCed before host}
741741
%pythonappend libnvme_ctrl::init(struct libnvme_host *h, int instance) {
742742
self.__host = h # Keep a reference to parent to ensure ctrl obj gets GCed before host}
@@ -766,13 +766,6 @@ struct libnvmf_context {};
766766

767767
void connect(struct libnvme_host *h) {
768768
int ret;
769-
const char *dev;
770-
771-
dev = libnvme_ctrl_get_name($self);
772-
if (dev && $self->cfg.duplicate_connect) {
773-
connect_err = -ENVME_CONNECT_ALREADY;
774-
return;
775-
}
776769

777770
Py_BEGIN_ALLOW_THREADS /* Release Python GIL */
778771
ret = libnvmf_add_ctrl(h, $self);

libnvme/src/nvme/fabrics.c

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1102,6 +1102,10 @@ __public int libnvmf_add_ctrl(libnvme_host_t h, libnvme_ctrl_t c)
11021102
__cleanup_free char *argstr = NULL;
11031103
int ret;
11041104

1105+
/* Are duplicate connections allowed on existing controller */
1106+
if (libnvme_ctrl_get_name(c) && !c->cfg.duplicate_connect)
1107+
return -ENVME_CONNECT_ALREADY;
1108+
11051109
/* apply configuration from config file (JSON) */
11061110
s = libnvme_lookup_subsystem(h, NULL, libnvme_ctrl_get_subsysnqn(c));
11071111
if (s) {

0 commit comments

Comments
 (0)