linux-kernel - Re: [PATCH] selinux: Use pr

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAHC9VhS_UmWfv0aFGLfaqw-fLSmoGiDEcXGFZ3bvwNgp2PF=zg@mail.gmail.com>
Date:   Mon, 2 Jul 2018 16:51:25 -0400
From:   Paul Moore <paul@...l-moore.com>
To:     joe@...ches.com
Cc:     Stephen Smalley <sds@...ho.nsa.gov>,
        Eric Paris <eparis@...isplace.org>, peter.enderborg@...y.com,
        selinux@...ho.nsa.gov, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] selinux: Use pr_fmt to prefix "SELinux: "

On Wed, Jun 20, 2018 at 2:39 AM Joe Perches <joe@...ches.com> wrote:
> pr_fmt can be used with the pr_<level> macros to prefix
> arbitrary content to logging messages.
>
> So add '#define pr_fmt(fmt) "SELinux: " fmt' to selinux files
> that use pr_<level> and remove embedded "SELinux: " prefixes
> from the format strings.
>
> Miscellanea:
>
> o Coalesce formats and realign arguments
> o Add missing space to a coalesced format
> o Remove "SELinux: " from SEL_MOUNT_FAIL_MSG as that is directly
>   used only in pr_warn
> o Add missing terminating \n to some formats
> o Consistently use single space after logging prefixes
>
> Signed-off-by: Joe Perches <joe@...ches.com>
> ---
>  security/selinux/avc.c            |   7 ++-
>  security/selinux/hooks.c          | 124 +++++++++++++++++---------------------
>  security/selinux/netif.c          |   7 ++-
>  security/selinux/netlink.c        |   7 ++-
>  security/selinux/netnode.c        |   4 +-
>  security/selinux/netport.c        |   4 +-
>  security/selinux/selinuxfs.c      |  33 +++++-----
>  security/selinux/ss/avtab.c       |  61 +++++++++----------
>  security/selinux/ss/conditional.c |  18 +++---
>  security/selinux/ss/ebitmap.c     |  20 +++---
>  security/selinux/ss/policydb.c    | 123 +++++++++++++++++--------------------
>  security/selinux/ss/services.c    | 110 +++++++++++++--------------------
>  security/selinux/ss/sidtab.c      |  12 ++--
>  13 files changed, 247 insertions(+), 283 deletions(-)

...

> @@ -1644,13 +1640,12 @@ static int inode_doinit_with_dentry(struct inode *inode, struct dentry *opt_dent
>
>                                 if (rc == -EINVAL) {
>                                         if (printk_ratelimit())
> -                                               pr_notice("SELinux: inode=%lu on dev=%s was found to have an invalid "
> -                                                       "context=%s.  This indicates you may need to relabel the inode or the "
> -                                                       "filesystem in question.\n", ino, dev, context);
> +                                               pr_notice("inode=%lu on dev=%s was found to have an invalid context=%s.  This indicates you may need to relabel the inode or the filesystem in question.\n",
> +                                                         ino, dev, context);

Please split up lines like this.  I realize that there isn't much room
left, but this message wraps a silly amount in my 80-char terminal; if
you need to wrap, please limit it to a word or two.

To stop the argument before it starts, I don't care what checkpatch.pl
says about splitting printk format strings like this.  Split the line
please.

> @@ -3124,9 +3117,9 @@ static int selinux_validate_for_sb_reconfigure(struct fs_context *fc)
>                 rc = security_context_str_to_sid(&selinux_state, mount_options[i],
>                                                  &sid, GFP_KERNEL);
>                 if (rc) {
> -                       pr_warn("SELinux: security_context_str_to_sid"
> -                               "(%s) failed for (dev %s, type %s) errno=%d\n",
> -                               mount_options[i], sb->s_id, sb->s_type->name, rc);
> +                       pr_warn("security_context_str_to_sid(%s) failed for (dev %s, type %s) errno=%d\n",
> +                               mount_options[i],
> +                               sb->s_id, sb->s_type->name, rc);
>                         goto inval;
>                 }

What tree did you base your patch on?  Please base SELinux patches
either on the SELinux tree or Linus' tree.  The SELinux tree can be
found at the links below:

* git://git.kernel.org/pub/scm/linux/kernel/git/pcmoore/selinux.git
* https://git.kernel.org/pub/scm/linux/kernel/git/pcmoore/selinux.git

--
paul moore
www.paul-moore.com