We have same limitation for lockspace paths as well, maybe we need to add this validation as part of the path converter to cover all path usages.
Previously if a path was too long, we truncated the path silently. This
probably fail when sanlock try to access non-existing path with "No such
path or directory" error. If you are unlucky, this could try to access
the wrong path, failing with bogus error, returning wrong data, or
worse, writing a resource into the wrong location, destroying existing
data.
Now we fail immediately with:
ValueError: Path is too long: '/too/long/path/...'
Signed-off-by: Nir Soffer <nsoffer@redhat.com>
---
python/sanlock.c | 17 +++++++++++++++++
tests/python_test.py | 7 -------
2 files changed, 17 insertions(+), 7 deletions(-)
diff --git a/python/sanlock.c b/python/sanlock.c
index ca3713d..7c50aba 100644
--- a/python/sanlock.c
+++ b/python/sanlock.c
@@ -143,10 +143,21 @@ pystring_as_cstring(PyObject *obj)
#else
return PyUnicode_AsUTF8(obj);
#endif
}
+static int
+validate_path(PyObject *path)
+{
+ if (PyBytes_Size(path) > SANLK_PATH_LEN - 1) {
+ set_error(PyExc_ValueError, "Path is too long: %s", path);
+ return 0;
+ }
+
+ return 1;
+}
+
static int
parse_single_disk(PyObject* disk, struct sanlk_disk* res_disk)
{
int rv = 0;
PyObject *path = NULL;
@@ -161,10 +172,13 @@ parse_single_disk(PyObject* disk, struct sanlk_disk* res_disk)
/* Override the error since it confusing in this context. */
set_error(PyExc_ValueError, "Invalid disk %s", disk);
goto finally;
}
+ if (!validate_path(path))
+ goto finally;
+
strncpy(res_disk->path, PyBytes_AsString(path), SANLK_PATH_LEN - 1);
res_disk->offset = offset;
rv = 1;
finally:
@@ -654,10 +668,13 @@ py_read_resource(PyObject *self __unused, PyObject *args, PyObject *keywds)
if (!PyArg_ParseTupleAndKeywords(args, keywds, "O&|kli", kwlist,
pypath_converter, &path, &(res->disks[0].offset), &align, §or)) {
goto finally;
}
+ if (!validate_path(path))
+ goto finally;
+
/* prepare the resource disk path */
strncpy(res->disks[0].path, PyBytes_AsString(path), SANLK_PATH_LEN - 1);
/* set alignment/sector flags */
if (add_align_flag(align, &res->flags) == -1)
diff --git a/tests/python_test.py b/tests/python_test.py
index 5624f60..47788e4 100644
--- a/tests/python_test.py
+++ b/tests/python_test.py
@@ -550,11 +550,10 @@ def test_write_resource_parse_args(no_sanlock_daemon, name, filename, encoding):
with raises_sanlock_errno():
sanlock.write_resource(b"ls_name", name, disks)
-@pytest.mark.xfail(reason="path truncated silently")
def test_write_resource_path_length(no_sanlock_daemon):
with pytest.raises(ValueError):
path = "x" * constants.SANLK_PATH_LEN
sanlock.write_resource(b"ls_name", b"res_name", [(path, 0)])
@@ -573,11 +572,10 @@ def test_release_resource_parse_args(no_sanlock_daemon, name, filename, encoding
with raises_sanlock_errno():
sanlock.release(b"ls_name", name, disks)
-@pytest.mark.xfail(reason="path truncated silently")
def test_release_resource_path_length(no_sanlock_daemon):
with pytest.raises(ValueError):
path = "x" * constants.SANLK_PATH_LEN
sanlock.release(b"ls_name", b"res_name", [(path, 0)])
@@ -596,11 +594,10 @@ def test_read_resource_owners_parse_args(no_sanlock_daemon, name, filename, enco
with raises_sanlock_errno():
sanlock.read_resource_owners(b"ls_name", name, disks)
-@pytest.mark.xfail(reason="path truncated silently")
def test_read_resource_owners_path_length(no_sanlock_daemon):
with pytest.raises(ValueError):
path = "x" * constants.SANLK_PATH_LEN
sanlock.read_resource_owners(b"ls_name", b"res_name", [(path, 0)])
@@ -658,11 +655,10 @@ def test_init_resource_parse_args(no_sanlock_daemon, name, filename, encoding):
sanlock.init_resource(b"ls_name", name, disks)
with raises_sanlock_errno(errno.ENOENT):
sanlock.init_resource(name, b"res_name", disks)
-@pytest.mark.xfail(reason="path truncated silently")
def test_init_resource_path_length(no_sanlock_daemon):
with pytest.raises(ValueError):
path = "x" * constants.SANLK_PATH_LEN
sanlock.init_resource(b"ls_name", b"res_name", [(path, 0)])
@@ -689,11 +685,10 @@ def test_read_resource_parse_args(no_sanlock_daemon, filename, encoding):
path = util.generate_path("/tmp/", filename, encoding)
with raises_sanlock_errno():
sanlock.read_resource(path)
-@pytest.mark.xfail(reason="path truncated silently")
def test_read_resource_path_length(no_sanlock_daemon):
with pytest.raises(ValueError):
path = "x" * constants.SANLK_PATH_LEN
sanlock.read_resource(path)
@@ -713,11 +708,10 @@ def test_request_parse_args(no_sanlock_daemon, name, filename, encoding):
with raises_sanlock_errno():
sanlock.request(name, b"res_name", disks)
-@pytest.mark.xfail(reason="path truncated silently")
def test_request_path_length(no_sanlock_daemon):
with pytest.raises(ValueError):
path = "x" * constants.SANLK_PATH_LEN
sanlock.request(b"ls_name", b"res_name", [(path, 0)])
@@ -737,11 +731,10 @@ def test_acquire_parse_args(no_sanlock_daemon, name, filename, encoding):
with raises_sanlock_errno():
sanlock.acquire(name, b"res_name", disks, pid=os.getpid())
-@pytest.mark.xfail(reason="path truncated silently")
def test_acquire_path_length(no_sanlock_daemon):
with pytest.raises(ValueError):
path = "x" * constants.SANLK_PATH_LEN
sanlock.acquire(b"ls_name", b"res_name", [(path, 0)], pid=os.getpid())
--
2.17.2