Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753988AbbHGChm (ORCPT ); Thu, 6 Aug 2015 22:37:42 -0400 Received: from mx1.redhat.com ([209.132.183.28]:37798 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751541AbbHGChj (ORCPT ); Thu, 6 Aug 2015 22:37:39 -0400 From: Paul Moore To: Richard Guy Briggs Cc: linux-audit@redhat.com, linux-kernel@vger.kernel.org, sgrubb@redhat.com, eparis@redhat.com Subject: Re: [PATCH V5] audit: use macros for unset inode and device values Date: Thu, 06 Aug 2015 14:49:15 -0400 Message-ID: <3585472.C48MyYSY3g@sifl> Organization: Red Hat User-Agent: KMail/4.14.10 (Linux/4.1.2-gentoo; KDE/4.14.10; x86_64; ; ) In-Reply-To: <2c12323ffb2c61ff83876a9adcba2c2d252dafd9.1438832670.git.rgb@redhat.com> References: <2c12323ffb2c61ff83876a9adcba2c2d252dafd9.1438832670.git.rgb@redhat.com> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 4790 Lines: 132 On Wednesday, August 05, 2015 11:48:20 PM Richard Guy Briggs wrote: > Clean up a number of places were casted magic numbers are used to represent > unset inode and device numbers in preparation for the audit by executable > path patch set. > > Signed-off-by: Richard Guy Briggs > --- > v6: Change dev macro cast from unsigned int to dev_t. > > v5: Move macros from include/uapi/linux/audit.h to include/linux/audit.h > Use "unsigned int" rather than bare "unsigned". > > include/linux/audit.h | 3 +++ > kernel/audit.c | 2 +- > kernel/audit_watch.c | 8 ++++---- > kernel/auditsc.c | 6 +++--- > 4 files changed, 11 insertions(+), 8 deletions(-) Great, thank you. ... and since we were just talking about modifications while I apply the patches, here are two examples of changes I'm "OK" making: * There was a minor conflict in audit_log_name() that caused that AUDIT_INO_UNSET to be dropped so I restored it. This is just par for the course when you're applying patches to a tree; I don't think twice about these changes so long as the merge is trivially understood. * The AUDIT_{INO,DEV}_UNSET macros needed to be wrapped in parentheses to keep ./scripts/checkpatch happy. I consider that trivial and not a functional change so I made that change after noting it in the description metadata. > diff --git a/include/linux/audit.h b/include/linux/audit.h > index c2e7e3a..1514412 100644 > --- a/include/linux/audit.h > +++ b/include/linux/audit.h > @@ -27,6 +27,9 @@ > #include > #include > > +#define AUDIT_INO_UNSET (unsigned long)-1 > +#define AUDIT_DEV_UNSET (dev_t)-1 > + > struct audit_sig_info { > uid_t uid; > pid_t pid; > diff --git a/kernel/audit.c b/kernel/audit.c > index 1c13e42..d546003 100644 > --- a/kernel/audit.c > +++ b/kernel/audit.c > @@ -1761,7 +1761,7 @@ void audit_log_name(struct audit_context *context, > struct audit_names *n, } else > audit_log_format(ab, " name=(null)"); > > - if (n->ino != (unsigned long)-1) > + if (n->ino != AUDIT_INO_UNSET) > audit_log_format(ab, " inode=%lu" > " dev=%02x:%02x mode=%#ho" > " ouid=%u ogid=%u rdev=%02x:%02x", > diff --git a/kernel/audit_watch.c b/kernel/audit_watch.c > index 8f123d7..c668bfc 100644 > --- a/kernel/audit_watch.c > +++ b/kernel/audit_watch.c > @@ -138,7 +138,7 @@ char *audit_watch_path(struct audit_watch *watch) > > int audit_watch_compare(struct audit_watch *watch, unsigned long ino, dev_t > dev) { > - return (watch->ino != (unsigned long)-1) && > + return (watch->ino != AUDIT_INO_UNSET) && > (watch->ino == ino) && > (watch->dev == dev); > } > @@ -179,8 +179,8 @@ static struct audit_watch *audit_init_watch(char *path) > INIT_LIST_HEAD(&watch->rules); > atomic_set(&watch->count, 1); > watch->path = path; > - watch->dev = (dev_t)-1; > - watch->ino = (unsigned long)-1; > + watch->dev = AUDIT_DEV_UNSET; > + watch->ino = AUDIT_INO_UNSET; > > return watch; > } > @@ -493,7 +493,7 @@ static int audit_watch_handle_event(struct > fsnotify_group *group, if (mask & (FS_CREATE|FS_MOVED_TO) && inode) > audit_update_watch(parent, dname, inode->i_sb->s_dev, inode->i_ino, 0); > else if (mask & (FS_DELETE|FS_MOVED_FROM)) > - audit_update_watch(parent, dname, (dev_t)-1, (unsigned long)-1, 1); > + audit_update_watch(parent, dname, AUDIT_DEV_UNSET, AUDIT_INO_UNSET, 1); > else if (mask & (FS_DELETE_SELF|FS_UNMOUNT|FS_MOVE_SELF)) > audit_remove_parent_watches(parent); > > diff --git a/kernel/auditsc.c b/kernel/auditsc.c > index 9fb9d1c..701ea5c 100644 > --- a/kernel/auditsc.c > +++ b/kernel/auditsc.c > @@ -180,7 +180,7 @@ static int audit_match_filetype(struct audit_context > *ctx, int val) return 0; > > list_for_each_entry(n, &ctx->names_list, list) { > - if ((n->ino != -1) && > + if ((n->ino != AUDIT_INO_UNSET) && > ((n->mode & S_IFMT) == mode)) > return 1; > } > @@ -1683,7 +1683,7 @@ static struct audit_names *audit_alloc_name(struct > audit_context *context, aname->should_free = true; > } > > - aname->ino = (unsigned long)-1; > + aname->ino = AUDIT_INO_UNSET; > aname->type = type; > list_add_tail(&aname->list, &context->names_list); > > @@ -1925,7 +1925,7 @@ void __audit_inode_child(const struct inode *parent, > if (inode) > audit_copy_inode(found_child, dentry, inode); > else > - found_child->ino = (unsigned long)-1; > + found_child->ino = AUDIT_INO_UNSET; > } > EXPORT_SYMBOL_GPL(__audit_inode_child); -- paul moore security @ redhat -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/