Skip to content

Commit e9cd107

Browse files
Martin Belangerigaw
authored andcommitted
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 70be264 commit e9cd107

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
@@ -443,6 +443,7 @@ struct libnvme_ns {
443443
%rename(set_connection) libnvmf_context::fctx_set_connection;
444444
%rename(set_persistent) libnvmf_context::fctx_set_persistent;
445445
%rename(set_device) libnvmf_context::fctx_set_device;
446+
%rename(set_fabrics_config) libnvmf_context::fctx_set_fabrics_config;
446447

447448
struct libnvmf_context {};
448449

@@ -725,8 +726,7 @@ struct libnvmf_context {};
725726
}
726727
%};
727728

728-
%pythonappend libnvme_ctrl::connect(struct libnvme_host *h,
729-
struct libnvmf_context *fctx) {
729+
%pythonappend libnvme_ctrl::connect(struct libnvme_host *h) {
730730
self.__host = h # Keep a reference to parent to ensure ctrl obj gets GCed before host}
731731
%pythonappend libnvme_ctrl::init(struct libnvme_host *h, int instance) {
732732
self.__host = h # Keep a reference to parent to ensure ctrl obj gets GCed before host}
@@ -756,13 +756,6 @@ struct libnvmf_context {};
756756

757757
void connect(struct libnvme_host *h) {
758758
int ret;
759-
const char *dev;
760-
761-
dev = libnvme_ctrl_get_name($self);
762-
if (dev && $self->cfg.duplicate_connect) {
763-
connect_err = -ENVME_CONNECT_ALREADY;
764-
return;
765-
}
766759

767760
Py_BEGIN_ALLOW_THREADS /* Release Python GIL */
768761
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)