Re: [PATCH ghak21 V2 4/4] audit: add parent of refused symlink to audit_names

[Paul Moore]

On Mon, Mar 12, 2018 at 2:31 AM, Richard Guy Briggs <rgb@xxxxxxxxxx> wrote:
> Audit link denied events for symlinks were missing the parent PATH
> record.  Add it.  Since the full pathname may not be available,
> reconstruct it from the path in the nameidata supplied.
>
> See: https://github.com/linux-audit/audit-kernel/issues/21
> Signed-off-by: Richard Guy Briggs <rgb@xxxxxxxxxx>
> ---
>  fs/namei.c            |  2 +-
>  include/linux/audit.h |  3 +++
>  kernel/audit.c        | 31 +++++++++++++++++++++++++++++++
>  3 files changed, 35 insertions(+), 1 deletion(-)

See my comment in patch 3/4; it should really be folded into this
patch.  Additional comment inline below ...

> diff --git a/kernel/audit.c b/kernel/audit.c
> index e54deaf..4acf374 100644
> --- a/kernel/audit.c
> +++ b/kernel/audit.c
> @@ -73,6 +73,7 @@
>  #include <linux/freezer.h>
>  #include <linux/pid_namespace.h>
>  #include <net/netns/generic.h>
> +#include <linux/namei.h> /* for LOOKUP_PARENT */
>
>  #include "audit.h"
>
> @@ -2320,6 +2321,36 @@ void audit_log_link_denied(const char *operation)
>         audit_log_end(ab);
>  }
>
> +/*
> + * audit_log_symlink_denied - report a symlink restriction denial
> + * @link: the path that triggered the restriction
> + */
> +void audit_log_symlink_denied(const struct path *link)
> +{
> +       char *pathname;
> +       struct filename *filename;
> +
> +       if (audit_dummy_context())
> +               return;
> +
> +       pathname = kmalloc(PATH_MAX + 1, GFP_KERNEL);
> +       if (!pathname) {
> +               audit_panic("memory allocation error while reporting symlink denied");
> +               return;
> +       }
> +       filename = getname_kernel(d_absolute_path(link, pathname, PATH_MAX + 1));
> +       if (IS_ERR(filename)) {
> +               audit_panic("error getting pathname while reporting symlink denied");
> +               goto out;
> +       }
> +       audit_inode(filename, link->dentry->d_parent, LOOKUP_PARENT);

Since we are already checking audit_dummy_context() above we don't
need to check it again in audit_inode(), you should just call
__audit_inode() directly.  As a reminder, make sure you convert
LOOKUP_PARENT to AUDIT_INODE_PARENT.

> +       audit_log_link_denied("follow_link");
> +       putname(filename);
> +out:
> +       kfree(pathname);
> +       return;
> +}

-- 
paul moore
www.paul-moore.com