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.
