Re: [PATCH] selinux: Use pr_fmt to prefix "SELinux: "

From: Paul Moore
Date: Mon Jul 02 2018 - 16:51:44 EST


On Wed, Jun 20, 2018 at 2:39 AM Joe Perches <joe@xxxxxxxxxxx> 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@xxxxxxxxxxx>
> ---
> 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