On Mon, Nov 11, 2024 at 05:06:43PM -0500, Paul Moore wrote:
This is completely untested, I didn't even compile it, but what about
something like the following? We do add an extra strlen(), but that is
going to be faster than the alloc/copy.
diff --git a/kernel/auditfilter.c b/kernel/auditfilter.c
index 470041c49a44..c30c2ee9fb77 100644
--- a/kernel/auditfilter.c
+++ b/kernel/auditfilter.c
@@ -1320,10 +1320,13 @@ int audit_compare_dname_path(const struct qstr *dname, const char *path, int par
return 1;
parentlen = parentlen == AUDIT_NAME_FULL ? parent_len(path) : parentlen;
- if (pathlen - parentlen != dlen)
- return 1;
-
p = path + parentlen;
+ pathlen = strlen(p);
Huh? We have
pathlen = strlen(path);
several lines prior to this. So unless parentlen + path manages to exceed
strlen(path) (in which case your strlen() is really asking for trouble),
this is simply
pathlen -= parentlen;
What am I missing here?
And while we are at it,
parentlen = parentlen == AUDIT_NAME_FULL ? parent_len(path) : parentlen;
is a bloody awful way to spell
if (parentlen == AUDIT_NAME_FULL)
parentlen = parent_len(path);
What's more, parent_len(path) starts with *yet* *another* strlen(path),
followed by really awful crap - we trim the trailing slashes (if any),
then search for the last slash before that... is that really worth
the chance to skip that strncmp()?
+ if (p[pathlen - 1] == '/')
+ pathlen--;
+
+ if (pathlen != dlen)
+ return 1;
return strncmp(p, dname->name, dlen);
... which really should've been memcmp(), at that.