From c0d61ae45912551f5ad44fc571bb2d170742b3d7 Mon Sep 17 00:00:00 2001 From: Lawrence D'Oliveiro Date: Thu, 3 Oct 2019 01:44:54 +0000 Subject: [PATCH 1/3] Rework some routines to fix holes in error checking and potential memory leaks from error recovery. Control paths are now organized so that you can determine with high confidence that all allocated storage will be freed once, and once only, regardless of what errors might occur. --- smbc/context.c | 287 +++++++++++++++++++++++++++------------------- smbc/dir.c | 172 ++++++++++++++++----------- smbc/smbcmodule.h | 1 + 3 files changed, 271 insertions(+), 189 deletions(-) diff --git a/smbc/context.c b/smbc/context.c index 6a736b1..f507952 100644 --- a/smbc/context.c +++ b/smbc/context.c @@ -212,97 +212,123 @@ Context_set_credentials_with_fallback (Context *self, PyObject *args) static PyObject * Context_open (Context *self, PyObject *args) -{ - PyObject *largs, *lkwlist; - char *uri; - File *file; - int flags = 0; - int mode = 0; - smbc_open_fn fn; - - debugprintf ("%p -> Context_open()\n", self->context); - if (!PyArg_ParseTuple (args, "s|ii", &uri, &flags, &mode)) - { - debugprintf ("%p <- Context_open() EXCEPTION\n", self->context); - return NULL; - } - - largs = Py_BuildValue ("()"); - lkwlist = PyDict_New (); - PyDict_SetItemString (lkwlist, "context", (PyObject *) self); - file = (File *)smbc_FileType.tp_new (&smbc_FileType, largs, lkwlist); - if (!file) - { - return PyErr_NoMemory (); - } - - if (smbc_FileType.tp_init ((PyObject *)file, largs, lkwlist) < 0) - { - smbc_FileType.tp_dealloc ((PyObject *)file); - debugprintf ("%p <- Context_open() EXCEPTION\n", self->context); - // already set error - return NULL; - } - - fn = smbc_getFunctionOpen (self->context); - errno = 0; - file->file = (*fn) (self->context, uri, (int)flags, (mode_t)mode); - if (!file->file) - { - pysmbc_SetFromErrno (); - smbc_FileType.tp_dealloc ((PyObject *)file); - file = NULL; - } - - Py_DECREF (largs); - Py_DECREF (lkwlist); - debugprintf ("%p <- Context_open() = File\n", self->context); - return (PyObject *)file; -} + { + PyObject *result = NULL; + PyObject *largs = NULL; + PyObject *lkwlist = NULL; + char *uri; + File *file = NULL; + int flags = 0; + int mode = 0; + smbc_open_fn fn_open; + + debugprintf ("%p -> Context_open()\n", self->context); + do /*once*/ + { + if (!PyArg_ParseTuple (args, "s|ii", &uri, &flags, &mode)) + { + debugprintf ("%p <- Context_open() EXCEPTION\n", self->context); + break; + } /*if*/ + largs = Py_BuildValue("()"); + if (PyErr_Occurred()) + break; + lkwlist = PyDict_New(); + if (PyErr_Occurred()) + break; + PyDict_SetItemString(lkwlist, "context", (PyObject *)self); + if (PyErr_Occurred()) + break; + file = (File *)smbc_FileType.tp_new(&smbc_FileType, largs, lkwlist); + if (file == NULL) + { + PyErr_NoMemory(); + break; + } /*if*/ + if (smbc_FileType.tp_init((PyObject *)file, largs, lkwlist) < 0) + { + debugprintf ("%p <- Context_open() EXCEPTION\n", self->context); + // already set error + break; + } /*if*/ + fn_open = smbc_getFunctionOpen(self->context); + errno = 0; + file->file = fn_open(self->context, uri, (int)flags, (mode_t)mode); + if (file->file == NULL) + { + pysmbc_SetFromErrno(); + break; + } /*if*/ + debugprintf ("%p <- Context_open() = File\n", self->context); + /* all done */ + result = (PyObject *)file; + file = NULL; /* so I don't dispose of it yet */ + } + while (false); + if (file != NULL) + { + smbc_FileType.tp_dealloc((PyObject *)file); + } /*if*/ + Py_XDECREF(largs); + Py_XDECREF(lkwlist); + return + result; + } /*Context_open*/ static PyObject * Context_creat (Context *self, PyObject *args) -{ - PyObject *largs, *lkwlist; - char *uri; - int mode = 0; - File *file; - smbc_creat_fn fn; - - if (!PyArg_ParseTuple (args, "s|i", &uri, &mode)) - { - return NULL; - } - - largs = Py_BuildValue ("()"); - lkwlist = PyDict_New (); - PyDict_SetItemString (lkwlist, "context", (PyObject *) self); - file = (File *)smbc_FileType.tp_new (&smbc_FileType, largs, lkwlist); - if (!file) - { - return PyErr_NoMemory(); - } - - if (smbc_FileType.tp_init ((PyObject *)file, largs, lkwlist) < 0) - { - smbc_FileType.tp_dealloc ((PyObject *)file); - return NULL; - } - - fn = smbc_getFunctionCreat (self->context); - errno = 0; - file->file = (*fn) (self->context, uri, mode); - if (!file->file) - { - pysmbc_SetFromErrno (); - smbc_FileType.tp_dealloc ((PyObject *)file); - file = NULL; - } - - Py_DECREF (largs); - Py_DECREF (lkwlist); - return (PyObject *)file; -} + { + PyObject *result = NULL; + PyObject *largs = NULL; + PyObject *lkwlist = NULL; + char *uri; + int mode = 0; + File *file = NULL; + smbc_creat_fn fn_creat; + + do /*once*/ + { + if (!PyArg_ParseTuple (args, "s|i", &uri, &mode)) + break; + largs = Py_BuildValue("()"); + if (PyErr_Occurred()) + break; + lkwlist = PyDict_New(); + if (PyErr_Occurred()) + break; + PyDict_SetItemString(lkwlist, "context", (PyObject *)self); + if (PyErr_Occurred()) + break; + file = (File *)smbc_FileType.tp_new(&smbc_FileType, largs, lkwlist); + if (file == NULL) + { + PyErr_NoMemory(); + break; + } /*if*/ + if (smbc_FileType.tp_init((PyObject *)file, largs, lkwlist) < 0) + break; + fn_creat = smbc_getFunctionCreat (self->context); + errno = 0; + file->file = fn_creat(self->context, uri, mode); + if (file->file == NULL) + { + pysmbc_SetFromErrno(); + break; + } /*if*/ + /* all done */ + result = (PyObject *)file; + file = NULL; /* so I don't dispose of it yet */ + } + while (false); + if (file != NULL) + { + smbc_FileType.tp_dealloc((PyObject *)file); + } /*if*/ + Py_XDECREF(largs); + Py_XDECREF(lkwlist); + return + result; + } /*Context_creat*/ static PyObject * Context_unlink (Context *self, PyObject *args) @@ -364,38 +390,59 @@ Context_rename (Context *self, PyObject *args) static PyObject * Context_opendir (Context *self, PyObject *args) -{ - PyObject *largs, *lkwlist; - PyObject *uri; - PyObject *dir; - - debugprintf ("%p -> Context_opendir()\n", self->context); - if (!PyArg_ParseTuple (args, "O", &uri)) - { - debugprintf ("%p <- Context_opendir() EXCEPTION\n", self->context); - return NULL; - } - - largs = Py_BuildValue ("()"); - lkwlist = PyDict_New (); - PyDict_SetItemString (lkwlist, "context", (PyObject *) self); - PyDict_SetItemString (lkwlist, "uri", uri); - dir = smbc_DirType.tp_new (&smbc_DirType, largs, lkwlist); - if (smbc_DirType.tp_init (dir, largs, lkwlist) < 0) - { - smbc_DirType.tp_dealloc (dir); - debugprintf ("%p <- Context_opendir() EXCEPTION\n", self->context); - dir = NULL; - } - else - { - debugprintf ("%p <- Context_opendir() = Dir\n", self->context); - } - - Py_DECREF (largs); - Py_DECREF (lkwlist); - return dir; -} + { + PyObject *result = NULL; + PyObject *largs = NULL; + PyObject *lkwlist = NULL; + PyObject *uri; + PyObject *dir = NULL; + + debugprintf ("%p -> Context_opendir()\n", self->context); + do /*once*/ + { + if (!PyArg_ParseTuple(args, "O", &uri)) + { + debugprintf ("%p <- Context_opendir() EXCEPTION\n", self->context); + break; + } /*if*/ + largs = Py_BuildValue("()"); + if (PyErr_Occurred()) + break; + lkwlist = PyDict_New(); + if (PyErr_Occurred()) + break; + PyDict_SetItemString(lkwlist, "context", (PyObject *) self); + if (PyErr_Occurred()) + break; + PyDict_SetItemString(lkwlist, "uri", uri); + if (PyErr_Occurred()) + break; + dir = smbc_DirType.tp_new(&smbc_DirType, largs, lkwlist); + if (dir == NULL) + { + PyErr_NoMemory(); + break; + } /*if*/ + if (smbc_DirType.tp_init(dir, largs, lkwlist) < 0) + { + debugprintf ("%p <- Context_opendir() EXCEPTION\n", self->context); + break; + } /*if*/ + debugprintf ("%p <- Context_opendir() = Dir\n", self->context); + /* all done */ + result = (PyObject *)dir; + dir = NULL; /* so I don't dispose of it yet */ + } + while (false); + if (dir != NULL) + { + smbc_DirType.tp_dealloc(dir); + } /*if*/ + Py_XDECREF(largs); + Py_XDECREF(lkwlist); + return + result; + } /*Context_opendir*/ static PyObject * Context_mkdir (Context *self, PyObject *args) diff --git a/smbc/dir.c b/smbc/dir.c index 9a8db19..1cbc138 100644 --- a/smbc/dir.c +++ b/smbc/dir.c @@ -109,77 +109,111 @@ Dir_dealloc (Dir *self) static PyObject * Dir_getdents (Dir *self) -{ - PyObject *listobj; - SMBCCTX *ctx; - char dirbuf[1024]; - smbc_getdents_fn fn; - int dirlen; - - debugprintf ("-> Dir_getdents()\n"); - ctx = self->context->context; - listobj = PyList_New (0); - fn = smbc_getFunctionGetdents (ctx); - errno = 0; - while ((dirlen = (*fn) (ctx, self->dir, - (struct smbc_dirent *) dirbuf, - sizeof (dirbuf))) != 0) - { - struct smbc_dirent *dirp; - - debugprintf ("dirlen = %d\n", dirlen); - if (dirlen < 0) - { - pysmbc_SetFromErrno(); - Py_DECREF (listobj); - debugprintf ("<- Dir_getdents() EXCEPTION\n"); - return NULL; - } - - dirp = (struct smbc_dirent *) dirbuf; - while (dirlen > 0) - { - PyObject *dent; - PyObject *largs = Py_BuildValue ("()"); - PyObject *lkwlist; - int len = dirp->dirlen; - int ret; + { + PyObject *result = NULL; + PyObject *listobj = NULL; + SMBCCTX *ctx; - PyObject *name = PyBytes_FromStringAndSize (dirp->name, - strlen (dirp->name)); - PyObject *comment = PyBytes_FromStringAndSize (dirp->comment, - strlen(dirp->comment)); - PyObject *type = PyLong_FromLong (dirp->smbc_type); - lkwlist = PyDict_New (); - PyDict_SetItemString (lkwlist, "name", name); - PyDict_SetItemString (lkwlist, "comment", comment); - PyDict_SetItemString (lkwlist, "smbc_type", type); - Py_DECREF (name); - Py_DECREF (comment); - Py_DECREF (type); - dent = smbc_DirentType.tp_new (&smbc_DirentType, largs, lkwlist); - ret = smbc_DirentType.tp_init (dent, largs, lkwlist); - Py_DECREF (largs); - Py_DECREF (lkwlist); - if(ret < 0){ - PyErr_SetString (PyExc_RuntimeError, - "Cannot initialize smbc_DirentType"); - Py_DECREF (listobj); - Py_DECREF (dent); - return NULL; + debugprintf ("-> Dir_getdents()\n"); + ctx = self->context->context; + do /*once*/ + { + listobj = PyList_New(0); + if (PyErr_Occurred()) + break; + const smbc_getdents_fn fn_getdents = smbc_getFunctionGetdents(ctx); + errno = 0; + for (;;) + { + char dirbuf[1024]; + int dirlen = fn_getdents(ctx, self->dir, (struct smbc_dirent *)dirbuf, sizeof dirbuf); + struct smbc_dirent *dirp; + if (dirlen <= 0) + { + if (dirlen < 0) + { + pysmbc_SetFromErrno(); + debugprintf ("<- Dir_getdents() EXCEPTION\n"); + } /*if*/ + break; + } /*if*/ + debugprintf ("dirlen = %d\n", dirlen); + dirp = (struct smbc_dirent *)dirbuf; + for (;;) + { + PyObject *dent = NULL; + PyObject *largs = NULL; + PyObject *lkwlist = NULL; + PyObject *name = NULL; + PyObject *comment = NULL; + PyObject *type = NULL; + do /*once*/ + { + largs = Py_BuildValue("()"); + if (PyErr_Occurred()) + break; + name = PyBytes_FromString(dirp->name); + if (PyErr_Occurred()) + break; + comment = PyBytes_FromString(dirp->comment); + if (PyErr_Occurred()) + break; + type = PyLong_FromLong(dirp->smbc_type); + if (PyErr_Occurred()) + break; + lkwlist = PyDict_New(); + if (PyErr_Occurred()) + break; + PyDict_SetItemString(lkwlist, "name", name); + if (PyErr_Occurred()) + break; + PyDict_SetItemString(lkwlist, "comment", comment); + if (PyErr_Occurred()) + break; + PyDict_SetItemString(lkwlist, "smbc_type", type); + if (PyErr_Occurred()) + break; + dent = smbc_DirentType.tp_new(&smbc_DirentType, largs, lkwlist); + if (PyErr_Occurred()) + break; + if (smbc_DirentType.tp_init(dent, largs, lkwlist) < 0) + { + PyErr_SetString(PyExc_RuntimeError, "Cannot initialize smbc_DirentType"); + break; + } /*if*/ + PyList_Append(listobj, dent); + if (PyErr_Occurred()) + break; + } + while (false); + Py_XDECREF(dent); + Py_XDECREF(largs); + Py_XDECREF(lkwlist); + Py_XDECREF(name); + Py_XDECREF(comment); + Py_XDECREF(type); + if (PyErr_Occurred()) + break; + const int len = dirp->dirlen; + dirp = (struct smbc_dirent *)(((char *)dirp) + len); + dirlen -= len; + if (dirlen == 0) + break; + } /*for*/ + if (PyErr_Occurred()) + break; + } /*for*/ + if (PyErr_Occurred()) + break; + /* all done */ + result = listobj; + listobj = NULL; /* so I don't dispose of it yet */ + debugprintf ("<- Dir_getdents() = list\n"); } - - PyList_Append (listobj, dent); - Py_DECREF (dent); - - dirp = (struct smbc_dirent *) (((char *) dirp) + len); - dirlen -= len; - } - } - - debugprintf ("<- Dir_getdents() = list\n"); - return listobj; -} + while (false); + Py_XDECREF(listobj); + return result; + } /*Dir_getdents*/ PyMethodDef Dir_methods[] = { diff --git a/smbc/smbcmodule.h b/smbc/smbcmodule.h index 0d7a0e2..752a9c0 100644 --- a/smbc/smbcmodule.h +++ b/smbc/smbcmodule.h @@ -24,6 +24,7 @@ #ifndef HAVE_SMBCMODULE_H #define HAVE_SMBCMODULE_H +#include #include /* GCC attributes */ -- 2.22.0