* Re: [PATCH v2 0/5] Overhaul the audit filename handling
From: Guenter Roeck @ 2015-01-22 16:58 UTC (permalink / raw)
To: Paul Moore; +Cc: linux-fsdevel, linux-audit, rgb, sd, linux-kernel, viro
In-Reply-To: <3252765.lrITUkIS9l@sifl>
On 01/22/2015 08:22 AM, Paul Moore wrote:
> On Wednesday, January 21, 2015 09:36:34 PM Guenter Roeck wrote:
>> On 01/21/2015 08:59 PM, Paul Moore wrote:
>>> This patchset has some important changes from the previous revision,
>>> namely a fix from Al Viro (included in 2/5) that resolves a boot panic
>>> on some systems as well as some smaller, less noteworthy fixes found
>>> in the linux-next announcement thread from January 20th (refcount bump
>>> in __audit_reusename() and a inode type in __audit_inode()).
>>>
>>> This patchset still needs some additional testing to verify that the
>>> audit code still functions properly (the minor fixes mentioned above)
>>> and there is an additional patch from Al that should be included as
>>> well, but I wanted to post this and push the series to the audit next
>>> branch quickly since a number of folks were affected by the boot panic.
>>>
>>> ---
>>>
>>> Paul Moore (5):
>>> fs: rework getname_kernel to handle up to PATH_MAX sized filenames
>>> fs: create proper filename objects using getname_kernel()
>>> audit: enable filename recording via getname_kernel()
>>> audit: fix filename matching in __audit_inode() and
>>> __audit_inode_child()
>>> audit: replace getname()/putname() hacks with reference counters
>>
>> Hi Paul,
>>
>> What is the baseline for this patch set ? Obviously -next won't work,
>> and it does not apply to mainline either.
>
> This patchset currently lives, along with one other unrelated patch, in the
> audit next branch:
>
> * git://git.infradead.org/users/pcmoore/audit
>
> I'm currently testing these in combination with the patch Al posted last
> night. Assuming all goes well I'll drop them from the audit next branch and
> toss all six patches (these plus Al's) into another branch in case Al wants to
> pull them for the VFS tree.
>
The version in the audit next tree works with my qemu microblaze test,
so feel free to add
Tested-by: Guenter Roeck <linux@roeck-us.net>
Guenter
^ permalink raw reply
* Re: [PATCH v2 1/5] fs: rework getname_kernel to handle up to PATH_MAX sized filenames
From: Guenter Roeck @ 2015-01-22 16:56 UTC (permalink / raw)
To: Richard Guy Briggs, Paul Moore
Cc: linux-fsdevel, linux-audit, sd, linux-kernel, viro
In-Reply-To: <20150122155316.GH18752@madcap2.tricolour.ca>
On 01/22/2015 07:53 AM, Richard Guy Briggs wrote:
> On 15/01/21, Paul Moore wrote:
>> In preparation for expanded use in the kernel, make getname_kernel()
>> more useful by allowing it to handle any legal filename length.
>>
>> Thanks to Guenter Roeck for his suggestion to substitute memcpy() for
>> strlcpy().
>>
>> CC: linux@roeck-us.net
>> CC: viro@zeniv.linux.org.uk
>> CC: linux-fsdevel@vger.kernel.org
>> Signed-off-by: Paul Moore <pmoore@redhat.com>
>
> Noted memcpy() difference (I assume to avoid races).
>
It is more efficient to use memcpy if the string length is known.
Guenter
^ permalink raw reply
* Re: [PATCH v2 5/5] audit: replace getname()/putname() hacks with reference counters
From: Paul Moore @ 2015-01-22 16:24 UTC (permalink / raw)
To: Richard Guy Briggs
Cc: linux-fsdevel, linux-audit, sd, linux-kernel, linux, viro
In-Reply-To: <20150122160942.GK18752@madcap2.tricolour.ca>
On Thursday, January 22, 2015 11:09:42 AM Richard Guy Briggs wrote:
> On 15/01/22, Paul Moore wrote:
> > In order to ensure that filenames are not released before the audit
> > subsystem is done with the strings there are a number of hacks built
> > into the fs and audit subsystems around getname() and putname(). To
> > say these hacks are "ugly" would be kind.
> >
> > This patch removes the filename hackery in favor of a more
> > conventional reference count based approach. The diffstat below tells
> > most of the story; lots of audit/fs specific code is replaced with a
> > traditional reference count based approach that is easily understood,
> > even by those not familiar with the audit and/or fs subsystems.
> >
> > CC: viro@zeniv.linux.org.uk
> > CC: linux-fsdevel@vger.kernel.org
> > Signed-off-by: Paul Moore <pmoore@redhat.com>
>
> Noted change of bumping refcnt before passing back pointer to struct
> filename.
>
> Reviewed-by: Richard Guy Briggs <rgb@redhat.com>
Thanks for taking another look at the patch set.
--
paul moore
security @ redhat
^ permalink raw reply
* Re: [PATCH v2 0/5] Overhaul the audit filename handling
From: Paul Moore @ 2015-01-22 16:23 UTC (permalink / raw)
To: Al Viro; +Cc: Guenter Roeck, linux-fsdevel, linux-audit, rgb, sd, linux-kernel
In-Reply-To: <20150122075429.GV29656@ZenIV.linux.org.uk>
On Thursday, January 22, 2015 07:54:29 AM Al Viro wrote:
> On Wed, Jan 21, 2015 at 09:36:34PM -0800, Guenter Roeck wrote:
> > On 01/21/2015 08:59 PM, Paul Moore wrote:
> > >This patchset has some important changes from the previous revision,
> > >namely a fix from Al Viro (included in 2/5) that resolves a boot panic
> > >on some systems as well as some smaller, less noteworthy fixes found
> > >in the linux-next announcement thread from January 20th (refcount bump
> > >in __audit_reusename() and a inode type in __audit_inode()).
> > >
> > >This patchset still needs some additional testing to verify that the
> > >audit code still functions properly (the minor fixes mentioned above)
> > >and there is an additional patch from Al that should be included as
> > >well, but I wanted to post this and push the series to the audit next
> > >branch quickly since a number of folks were affected by the boot panic.
> > >
> > >---
> > >
> > >Paul Moore (5):
> > > fs: rework getname_kernel to handle up to PATH_MAX sized filenames
> > > fs: create proper filename objects using getname_kernel()
> > > audit: enable filename recording via getname_kernel()
> > > audit: fix filename matching in __audit_inode() and
> > > __audit_inode_child()
> > > audit: replace getname()/putname() hacks with reference counters
> >
> > Hi Paul,
> >
> > What is the baseline for this patch set ? Obviously -next won't work,
> > and it does not apply to mainline either.
>
> FWIW, I've ported that on top of vfs.git#for-next; result is in
> vfs.git#experimental. Paul, are you OK with that one?
Okay, hang on let me test that ...
--
paul moore
security @ redhat
^ permalink raw reply
* Re: [PATCH v2 0/5] Overhaul the audit filename handling
From: Paul Moore @ 2015-01-22 16:22 UTC (permalink / raw)
To: Guenter Roeck; +Cc: linux-fsdevel, linux-audit, rgb, sd, linux-kernel, viro
In-Reply-To: <54C08C62.8010609@roeck-us.net>
On Wednesday, January 21, 2015 09:36:34 PM Guenter Roeck wrote:
> On 01/21/2015 08:59 PM, Paul Moore wrote:
> > This patchset has some important changes from the previous revision,
> > namely a fix from Al Viro (included in 2/5) that resolves a boot panic
> > on some systems as well as some smaller, less noteworthy fixes found
> > in the linux-next announcement thread from January 20th (refcount bump
> > in __audit_reusename() and a inode type in __audit_inode()).
> >
> > This patchset still needs some additional testing to verify that the
> > audit code still functions properly (the minor fixes mentioned above)
> > and there is an additional patch from Al that should be included as
> > well, but I wanted to post this and push the series to the audit next
> > branch quickly since a number of folks were affected by the boot panic.
> >
> > ---
> >
> > Paul Moore (5):
> > fs: rework getname_kernel to handle up to PATH_MAX sized filenames
> > fs: create proper filename objects using getname_kernel()
> > audit: enable filename recording via getname_kernel()
> > audit: fix filename matching in __audit_inode() and
> > __audit_inode_child()
> > audit: replace getname()/putname() hacks with reference counters
>
> Hi Paul,
>
> What is the baseline for this patch set ? Obviously -next won't work,
> and it does not apply to mainline either.
This patchset currently lives, along with one other unrelated patch, in the
audit next branch:
* git://git.infradead.org/users/pcmoore/audit
I'm currently testing these in combination with the patch Al posted last
night. Assuming all goes well I'll drop them from the audit next branch and
toss all six patches (these plus Al's) into another branch in case Al wants to
pull them for the VFS tree.
--
paul moore
security @ redhat
^ permalink raw reply
* Re: [PATCH v2 5/5] audit: replace getname()/putname() hacks with reference counters
From: Richard Guy Briggs @ 2015-01-22 16:09 UTC (permalink / raw)
To: Paul Moore; +Cc: linux-fsdevel, linux-audit, sd, linux-kernel, linux, viro
In-Reply-To: <20150122050023.1347.6866.stgit@localhost>
On 15/01/22, Paul Moore wrote:
> In order to ensure that filenames are not released before the audit
> subsystem is done with the strings there are a number of hacks built
> into the fs and audit subsystems around getname() and putname(). To
> say these hacks are "ugly" would be kind.
>
> This patch removes the filename hackery in favor of a more
> conventional reference count based approach. The diffstat below tells
> most of the story; lots of audit/fs specific code is replaced with a
> traditional reference count based approach that is easily understood,
> even by those not familiar with the audit and/or fs subsystems.
>
> CC: viro@zeniv.linux.org.uk
> CC: linux-fsdevel@vger.kernel.org
> Signed-off-by: Paul Moore <pmoore@redhat.com>
Noted change of bumping refcnt before passing back pointer to struct
filename.
Reviewed-by: Richard Guy Briggs <rgb@redhat.com>
> ---
> fs/namei.c | 29 +++++++-------
> include/linux/audit.h | 3 -
> include/linux/fs.h | 9 +---
> kernel/audit.h | 17 +-------
> kernel/auditsc.c | 105 +++++--------------------------------------------
> 5 files changed, 29 insertions(+), 134 deletions(-)
>
> diff --git a/fs/namei.c b/fs/namei.c
> index e18a2b5..f73e757 100644
> --- a/fs/namei.c
> +++ b/fs/namei.c
> @@ -117,15 +117,6 @@
> * POSIX.1 2.4: an empty pathname is invalid (ENOENT).
> * PATH_MAX includes the nul terminator --RR.
> */
> -void final_putname(struct filename *name)
> -{
> - if (name->separate) {
> - __putname(name->name);
> - kfree(name);
> - } else {
> - __putname(name);
> - }
> -}
>
> #define EMBEDDED_NAME_MAX (PATH_MAX - sizeof(struct filename))
>
> @@ -144,6 +135,7 @@ getname_flags(const char __user *filename, int flags, int *empty)
> result = __getname();
> if (unlikely(!result))
> return ERR_PTR(-ENOMEM);
> + result->refcnt = 1;
>
> /*
> * First, try to embed the struct filename inside the names_cache
> @@ -178,6 +170,7 @@ recopy:
> }
> result->name = kname;
> result->separate = true;
> + result->refcnt = 1;
> max = PATH_MAX;
> goto recopy;
> }
> @@ -201,7 +194,7 @@ recopy:
> return result;
>
> error:
> - final_putname(result);
> + putname(result);
> return err;
> }
>
> @@ -242,19 +235,25 @@ getname_kernel(const char * filename)
> memcpy((char *)result->name, filename, len);
> result->uptr = NULL;
> result->aname = NULL;
> + result->refcnt = 1;
> audit_getname(result);
>
> return result;
> }
>
> -#ifdef CONFIG_AUDITSYSCALL
> void putname(struct filename *name)
> {
> - if (unlikely(!audit_dummy_context()))
> - return audit_putname(name);
> - final_putname(name);
> + BUG_ON(name->refcnt <= 0);
> +
> + if (--name->refcnt > 0)
> + return;
> +
> + if (name->separate) {
> + __putname(name->name);
> + kfree(name);
> + } else
> + __putname(name);
> }
> -#endif
>
> static int check_acl(struct inode *inode, int mask)
> {
> diff --git a/include/linux/audit.h b/include/linux/audit.h
> index b481779..5f2ad5f 100644
> --- a/include/linux/audit.h
> +++ b/include/linux/audit.h
> @@ -127,7 +127,6 @@ extern void __audit_syscall_entry(int major, unsigned long a0, unsigned long a1,
> extern void __audit_syscall_exit(int ret_success, long ret_value);
> extern struct filename *__audit_reusename(const __user char *uptr);
> extern void __audit_getname(struct filename *name);
> -extern void audit_putname(struct filename *name);
>
> #define AUDIT_INODE_PARENT 1 /* dentry represents the parent */
> #define AUDIT_INODE_HIDDEN 2 /* audit record should be hidden */
> @@ -346,8 +345,6 @@ static inline struct filename *audit_reusename(const __user char *name)
> }
> static inline void audit_getname(struct filename *name)
> { }
> -static inline void audit_putname(struct filename *name)
> -{ }
> static inline void __audit_inode(struct filename *name,
> const struct dentry *dentry,
> unsigned int flags)
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index e11d60c..df7a047 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -2017,6 +2017,7 @@ struct filename {
> const char *name; /* pointer to actual string */
> const __user char *uptr; /* original userland pointer */
> struct audit_names *aname;
> + int refcnt;
> bool separate; /* should "name" be freed? */
> };
>
> @@ -2036,6 +2037,7 @@ extern int filp_close(struct file *, fl_owner_t id);
>
> extern struct filename *getname(const char __user *);
> extern struct filename *getname_kernel(const char *);
> +extern void putname(struct filename *name);
>
> enum {
> FILE_CREATED = 1,
> @@ -2056,15 +2058,8 @@ extern void __init vfs_caches_init(unsigned long);
>
> extern struct kmem_cache *names_cachep;
>
> -extern void final_putname(struct filename *name);
> -
> #define __getname() kmem_cache_alloc(names_cachep, GFP_KERNEL)
> #define __putname(name) kmem_cache_free(names_cachep, (void *)(name))
> -#ifndef CONFIG_AUDITSYSCALL
> -#define putname(name) final_putname(name)
> -#else
> -extern void putname(struct filename *name);
> -#endif
>
> #ifdef CONFIG_BLOCK
> extern int register_blkdev(unsigned int, const char *);
> diff --git a/kernel/audit.h b/kernel/audit.h
> index 3cdffad..1caa0d3 100644
> --- a/kernel/audit.h
> +++ b/kernel/audit.h
> @@ -24,12 +24,6 @@
> #include <linux/skbuff.h>
> #include <uapi/linux/mqueue.h>
>
> -/* 0 = no checking
> - 1 = put_count checking
> - 2 = verbose put_count checking
> -*/
> -#define AUDIT_DEBUG 0
> -
> /* AUDIT_NAMES is the number of slots we reserve in the audit_context
> * for saving names from getname(). If we get more names we will allocate
> * a name dynamically and also add those to the list anchored by names_list. */
> @@ -74,9 +68,8 @@ struct audit_cap_data {
> };
> };
>
> -/* When fs/namei.c:getname() is called, we store the pointer in name and
> - * we don't let putname() free it (instead we free all of the saved
> - * pointers at syscall exit time).
> +/* When fs/namei.c:getname() is called, we store the pointer in name and bump
> + * the refcnt in the associated filename struct.
> *
> * Further, in fs/namei.c:path_lookup() we store the inode and device.
> */
> @@ -86,7 +79,6 @@ struct audit_names {
> struct filename *name;
> int name_len; /* number of chars to log */
> bool hidden; /* don't log this record */
> - bool name_put; /* call __putname()? */
>
> unsigned long ino;
> dev_t dev;
> @@ -208,11 +200,6 @@ struct audit_context {
> };
> int fds[2];
> struct audit_proctitle proctitle;
> -
> -#if AUDIT_DEBUG
> - int put_count;
> - int ino_count;
> -#endif
> };
>
> extern u32 audit_ever_enabled;
> diff --git a/kernel/auditsc.c b/kernel/auditsc.c
> index c54b5f0..0a93b71 100644
> --- a/kernel/auditsc.c
> +++ b/kernel/auditsc.c
> @@ -866,33 +866,10 @@ static inline void audit_free_names(struct audit_context *context)
> {
> struct audit_names *n, *next;
>
> -#if AUDIT_DEBUG == 2
> - if (context->put_count + context->ino_count != context->name_count) {
> - int i = 0;
> -
> - pr_err("%s:%d(:%d): major=%d in_syscall=%d"
> - " name_count=%d put_count=%d ino_count=%d"
> - " [NOT freeing]\n", __FILE__, __LINE__,
> - context->serial, context->major, context->in_syscall,
> - context->name_count, context->put_count,
> - context->ino_count);
> - list_for_each_entry(n, &context->names_list, list) {
> - pr_err("names[%d] = %p = %s\n", i++, n->name,
> - n->name->name ?: "(null)");
> - }
> - dump_stack();
> - return;
> - }
> -#endif
> -#if AUDIT_DEBUG
> - context->put_count = 0;
> - context->ino_count = 0;
> -#endif
> -
> list_for_each_entry_safe(n, next, &context->names_list, list) {
> list_del(&n->list);
> - if (n->name && n->name_put)
> - final_putname(n->name);
> + if (n->name)
> + putname(n->name);
> if (n->should_free)
> kfree(n);
> }
> @@ -1711,9 +1688,6 @@ static struct audit_names *audit_alloc_name(struct audit_context *context,
> list_add_tail(&aname->list, &context->names_list);
>
> context->name_count++;
> -#if AUDIT_DEBUG
> - context->ino_count++;
> -#endif
> return aname;
> }
>
> @@ -1734,8 +1708,10 @@ __audit_reusename(const __user char *uptr)
> list_for_each_entry(n, &context->names_list, list) {
> if (!n->name)
> continue;
> - if (n->name->uptr == uptr)
> + if (n->name->uptr == uptr) {
> + n->name->refcnt++;
> return n->name;
> + }
> }
> return NULL;
> }
> @@ -1752,19 +1728,8 @@ void __audit_getname(struct filename *name)
> struct audit_context *context = current->audit_context;
> struct audit_names *n;
>
> - if (!context->in_syscall) {
> -#if AUDIT_DEBUG == 2
> - pr_err("%s:%d(:%d): ignoring getname(%p)\n",
> - __FILE__, __LINE__, context->serial, name);
> - dump_stack();
> -#endif
> + if (!context->in_syscall)
> return;
> - }
> -
> -#if AUDIT_DEBUG
> - /* The filename _must_ have a populated ->name */
> - BUG_ON(!name->name);
> -#endif
>
> n = audit_alloc_name(context, AUDIT_TYPE_UNKNOWN);
> if (!n)
> @@ -1772,56 +1737,13 @@ void __audit_getname(struct filename *name)
>
> n->name = name;
> n->name_len = AUDIT_NAME_FULL;
> - n->name_put = true;
> name->aname = n;
> + name->refcnt++;
>
> if (!context->pwd.dentry)
> get_fs_pwd(current->fs, &context->pwd);
> }
>
> -/* audit_putname - intercept a putname request
> - * @name: name to intercept and delay for putname
> - *
> - * If we have stored the name from getname in the audit context,
> - * then we delay the putname until syscall exit.
> - * Called from include/linux/fs.h:putname().
> - */
> -void audit_putname(struct filename *name)
> -{
> - struct audit_context *context = current->audit_context;
> -
> - BUG_ON(!context);
> - if (!name->aname || !context->in_syscall) {
> -#if AUDIT_DEBUG == 2
> - pr_err("%s:%d(:%d): final_putname(%p)\n",
> - __FILE__, __LINE__, context->serial, name);
> - if (context->name_count) {
> - struct audit_names *n;
> - int i = 0;
> -
> - list_for_each_entry(n, &context->names_list, list)
> - pr_err("name[%d] = %p = %s\n", i++, n->name,
> - n->name->name ?: "(null)");
> - }
> -#endif
> - final_putname(name);
> - }
> -#if AUDIT_DEBUG
> - else {
> - ++context->put_count;
> - if (context->put_count > context->name_count) {
> - pr_err("%s:%d(:%d): major=%d in_syscall=%d putname(%p)"
> - " name_count=%d put_count=%d\n",
> - __FILE__, __LINE__,
> - context->serial, context->major,
> - context->in_syscall, name->name,
> - context->name_count, context->put_count);
> - dump_stack();
> - }
> - }
> -#endif
> -}
> -
> /**
> * __audit_inode - store the inode and device from a lookup
> * @name: name being audited
> @@ -1842,11 +1764,6 @@ void __audit_inode(struct filename *name, const struct dentry *dentry,
> if (!name)
> goto out_alloc;
>
> -#if AUDIT_DEBUG
> - /* The struct filename _must_ have a populated ->name */
> - BUG_ON(!name->name);
> -#endif
> -
> /*
> * If we have a pointer to an audit_names entry already, then we can
> * just use it directly if the type is correct.
> @@ -1893,9 +1810,10 @@ out_alloc:
> n = audit_alloc_name(context, AUDIT_TYPE_UNKNOWN);
> if (!n)
> return;
> - if (name)
> - /* no need to set ->name_put as the original will cleanup */
> + if (name) {
> n->name = name;
> + name->refcnt++;
> + }
>
> out:
> if (parent) {
> @@ -1995,8 +1913,7 @@ void __audit_inode_child(const struct inode *parent,
> if (found_parent) {
> found_child->name = found_parent->name;
> found_child->name_len = AUDIT_NAME_FULL;
> - /* don't call __putname() */
> - found_child->name_put = false;
> + found_child->name->refcnt++;
> }
> }
>
>
- RGB
--
Richard Guy Briggs <rbriggs@redhat.com>
Senior Software Engineer, Kernel Security, AMER ENG Base Operating Systems, Red Hat
Remote, Ottawa, Canada
Voice: +1.647.777.2635, Internal: (81) 32635, Alt: +1.613.693.0684x3545
^ permalink raw reply
* Re: [PATCH v2 4/5] audit: fix filename matching in __audit_inode() and __audit_inode_child()
From: Richard Guy Briggs @ 2015-01-22 16:04 UTC (permalink / raw)
To: Paul Moore; +Cc: linux-fsdevel, linux-audit, sd, linux-kernel, linux, viro
In-Reply-To: <20150122050016.1347.17253.stgit@localhost>
On 15/01/22, Paul Moore wrote:
> In all likelihood there were some subtle, and perhaps not so subtle,
> bugs with filename matching in audit_inode() and audit_inode_child()
> for some time, however, recent changes to the audit filename code have
> definitely broken the filename matching code. The breakage could
> result in duplicate filenames in the audit log and other odd audit
> record entries. This patch fixes the filename matching code and
> restores some sanity to the filename audit records.
>
> CC: viro@zeniv.linux.org.uk
> CC: linux-fsdevel@vger.kernel.org
> Signed-off-by: Paul Moore <pmoore@redhat.com>
Noted change to use the iterator instead of the reference in the inode check.
Reviewed-by: Richard Guy Briggs <rgb@redhat.com>
> ---
> kernel/auditsc.c | 34 +++++++++++++++++++++++++---------
> 1 file changed, 25 insertions(+), 9 deletions(-)
>
> diff --git a/kernel/auditsc.c b/kernel/auditsc.c
> index c967ffc..c54b5f0 100644
> --- a/kernel/auditsc.c
> +++ b/kernel/auditsc.c
> @@ -1846,6 +1846,7 @@ void __audit_inode(struct filename *name, const struct dentry *dentry,
> /* The struct filename _must_ have a populated ->name */
> BUG_ON(!name->name);
> #endif
> +
> /*
> * If we have a pointer to an audit_names entry already, then we can
> * just use it directly if the type is correct.
> @@ -1863,7 +1864,17 @@ void __audit_inode(struct filename *name, const struct dentry *dentry,
> }
>
> list_for_each_entry_reverse(n, &context->names_list, list) {
> - if (!n->name || strcmp(n->name->name, name->name))
> + if (n->ino) {
> + /* valid inode number, use that for the comparison */
> + if (n->ino != inode->i_ino ||
> + n->dev != inode->i_sb->s_dev)
> + continue;
> + } else if (n->name) {
> + /* inode number has not been set, check the name */
> + if (strcmp(n->name->name, name->name))
> + continue;
> + } else
> + /* no inode and no name (?!) ... this is odd ... */
> continue;
>
> /* match the correct record type */
> @@ -1931,11 +1942,16 @@ void __audit_inode_child(const struct inode *parent,
>
> /* look for a parent entry first */
> list_for_each_entry(n, &context->names_list, list) {
> - if (!n->name || n->type != AUDIT_TYPE_PARENT)
> + if (!n->name ||
> + (n->type != AUDIT_TYPE_PARENT &&
> + n->type != AUDIT_TYPE_UNKNOWN))
> continue;
>
> - if (n->ino == parent->i_ino &&
> - !audit_compare_dname_path(dname, n->name->name, n->name_len)) {
> + if (n->ino == parent->i_ino && n->dev == parent->i_sb->s_dev &&
> + !audit_compare_dname_path(dname,
> + n->name->name, n->name_len)) {
> + if (n->type == AUDIT_TYPE_UNKNOWN)
> + n->type = AUDIT_TYPE_PARENT;
> found_parent = n;
> break;
> }
> @@ -1944,11 +1960,8 @@ void __audit_inode_child(const struct inode *parent,
> /* is there a matching child entry? */
> list_for_each_entry(n, &context->names_list, list) {
> /* can only match entries that have a name */
> - if (!n->name || n->type != type)
> - continue;
> -
> - /* if we found a parent, make sure this one is a child of it */
> - if (found_parent && (n->name != found_parent->name))
> + if (!n->name ||
> + (n->type != type && n->type != AUDIT_TYPE_UNKNOWN))
> continue;
>
> if (!strcmp(dname, n->name->name) ||
> @@ -1956,6 +1969,8 @@ void __audit_inode_child(const struct inode *parent,
> found_parent ?
> found_parent->name_len :
> AUDIT_NAME_FULL)) {
> + if (n->type == AUDIT_TYPE_UNKNOWN)
> + n->type = type;
> found_child = n;
> break;
> }
> @@ -1984,6 +1999,7 @@ void __audit_inode_child(const struct inode *parent,
> found_child->name_put = false;
> }
> }
> +
> if (inode)
> audit_copy_inode(found_child, dentry, inode);
> else
>
- RGB
--
Richard Guy Briggs <rbriggs@redhat.com>
Senior Software Engineer, Kernel Security, AMER ENG Base Operating Systems, Red Hat
Remote, Ottawa, Canada
Voice: +1.647.777.2635, Internal: (81) 32635, Alt: +1.613.693.0684x3545
^ permalink raw reply
* Re: [PATCH v2 2/5] fs: create proper filename objects using getname_kernel()
From: Richard Guy Briggs @ 2015-01-22 15:54 UTC (permalink / raw)
To: Paul Moore; +Cc: linux-fsdevel, linux-audit, sd, linux-kernel, linux, viro
In-Reply-To: <20150122050003.1347.4833.stgit@localhost>
On 15/01/22, Paul Moore wrote:
> There are several areas in the kernel that create temporary filename
> objects using the following pattern:
>
> int func(const char *name)
> {
> struct filename *file = { .name = name };
> ...
> return 0;
> }
>
> ... which for the most part works okay, but it causes havoc within the
> audit subsystem as the filename object does not persist beyond the
> lifetime of the function. This patch converts all of these temporary
> filename objects into proper filename objects using getname_kernel()
> and putname() which ensure that the filename object persists until the
> audit subsystem is finished with it.
>
> Also, a special thanks to Al Viro, Guenter Roeck, and Sabrina Dubroca
> for helping resolve a difficult kernel panic on boot related to a
> use-after-free problem in kern_path_create(); the thread can be seen
> at the link below:
>
> * https://lkml.org/lkml/2015/1/20/710
>
> This patch includes code that was either based on, or directly written
> by Al in the above thread.
>
> CC: viro@zeniv.linux.org.uk
> CC: linux@roeck-us.net
> CC: sd@queasysnail.net
> CC: linux-fsdevel@vger.kernel.org
> Signed-off-by: Paul Moore <pmoore@redhat.com>
Noted change to use ERR_CAST() for consistency.
Reviewed-by: Richard Guy Briggs <rgb@redhat.com>
> ---
> fs/exec.c | 11 +++++++-
> fs/namei.c | 80 +++++++++++++++++++++++++++++++++++++++++++++++-------------
> fs/open.c | 11 +++++++-
> 3 files changed, 81 insertions(+), 21 deletions(-)
>
> diff --git a/fs/exec.c b/fs/exec.c
> index a3d33fe..d067771 100644
> --- a/fs/exec.c
> +++ b/fs/exec.c
> @@ -789,8 +789,15 @@ exit:
>
> struct file *open_exec(const char *name)
> {
> - struct filename tmp = { .name = name };
> - return do_open_exec(&tmp);
> + struct file *file;
> + struct filename *tmp;
> +
> + tmp = getname_kernel(name);
> + if (unlikely(IS_ERR(tmp)))
> + return (void *)tmp;
> + file = do_open_exec(tmp);
> + putname(tmp);
> + return file;
> }
> EXPORT_SYMBOL(open_exec);
>
> diff --git a/fs/namei.c b/fs/namei.c
> index 63eaaf6..f793fe4 100644
> --- a/fs/namei.c
> +++ b/fs/namei.c
> @@ -2001,31 +2001,50 @@ static int filename_lookup(int dfd, struct filename *name,
> static int do_path_lookup(int dfd, const char *name,
> unsigned int flags, struct nameidata *nd)
> {
> - struct filename filename = { .name = name };
> + int retval;
> + struct filename *filename;
>
> - return filename_lookup(dfd, &filename, flags, nd);
> + filename = getname_kernel(name);
> + if (unlikely(IS_ERR(filename)))
> + return PTR_ERR(filename);
> + retval = filename_lookup(dfd, filename, flags, nd);
> + putname(filename);
> + return retval;
> }
>
> /* does lookup, returns the object with parent locked */
> struct dentry *kern_path_locked(const char *name, struct path *path)
> {
> + struct filename *filename;
> struct nameidata nd;
> struct dentry *d;
> - int err = do_path_lookup(AT_FDCWD, name, LOOKUP_PARENT, &nd);
> - if (err)
> - return ERR_PTR(err);
> + int err;
> +
> + filename = getname_kernel(name);
> + if (IS_ERR(filename))
> + return ERR_CAST(filename);
> +
> + err = filename_lookup(AT_FDCWD, filename, LOOKUP_PARENT, &nd);
> + if (err) {
> + d = ERR_PTR(err);
> + goto out;
> + }
> if (nd.last_type != LAST_NORM) {
> path_put(&nd.path);
> - return ERR_PTR(-EINVAL);
> + d = ERR_PTR(-EINVAL);
> + goto out;
> }
> mutex_lock_nested(&nd.path.dentry->d_inode->i_mutex, I_MUTEX_PARENT);
> d = __lookup_hash(&nd.last, nd.path.dentry, 0);
> if (IS_ERR(d)) {
> mutex_unlock(&nd.path.dentry->d_inode->i_mutex);
> path_put(&nd.path);
> - return d;
> + goto out;
> }
> *path = nd.path;
> +
> +out:
> + putname(filename);
> return d;
> }
>
> @@ -2368,8 +2387,15 @@ int
> kern_path_mountpoint(int dfd, const char *name, struct path *path,
> unsigned int flags)
> {
> - struct filename s = {.name = name};
> - return filename_mountpoint(dfd, &s, path, flags);
> + int retval;
> + struct filename *s;
> +
> + s = getname_kernel(name);
> + if (unlikely(IS_ERR(s)))
> + return PTR_ERR(s);
> + retval = filename_mountpoint(dfd, s, path, flags);
> + putname(s);
> + return retval;
> }
> EXPORT_SYMBOL(kern_path_mountpoint);
>
> @@ -3259,7 +3285,7 @@ struct file *do_file_open_root(struct dentry *dentry, struct vfsmount *mnt,
> {
> struct nameidata nd;
> struct file *file;
> - struct filename filename = { .name = name };
> + struct filename *filename;
> int flags = op->lookup_flags | LOOKUP_ROOT;
>
> nd.root.mnt = mnt;
> @@ -3268,16 +3294,22 @@ struct file *do_file_open_root(struct dentry *dentry, struct vfsmount *mnt,
> if (d_is_symlink(dentry) && op->intent & LOOKUP_OPEN)
> return ERR_PTR(-ELOOP);
>
> - file = path_openat(-1, &filename, &nd, op, flags | LOOKUP_RCU);
> + filename = getname_kernel(name);
> + if (unlikely(IS_ERR(filename)))
> + return ERR_CAST(filename);
> +
> + file = path_openat(-1, filename, &nd, op, flags | LOOKUP_RCU);
> if (unlikely(file == ERR_PTR(-ECHILD)))
> - file = path_openat(-1, &filename, &nd, op, flags);
> + file = path_openat(-1, filename, &nd, op, flags);
> if (unlikely(file == ERR_PTR(-ESTALE)))
> - file = path_openat(-1, &filename, &nd, op, flags | LOOKUP_REVAL);
> + file = path_openat(-1, filename, &nd, op, flags | LOOKUP_REVAL);
> + putname(filename);
> return file;
> }
>
> -struct dentry *kern_path_create(int dfd, const char *pathname,
> - struct path *path, unsigned int lookup_flags)
> +static struct dentry *filename_create(int dfd, struct filename *name,
> + struct path *path,
> + unsigned int lookup_flags)
> {
> struct dentry *dentry = ERR_PTR(-EEXIST);
> struct nameidata nd;
> @@ -3291,7 +3323,7 @@ struct dentry *kern_path_create(int dfd, const char *pathname,
> */
> lookup_flags &= LOOKUP_REVAL;
>
> - error = do_path_lookup(dfd, pathname, LOOKUP_PARENT|lookup_flags, &nd);
> + error = filename_lookup(dfd, name, LOOKUP_PARENT|lookup_flags, &nd);
> if (error)
> return ERR_PTR(error);
>
> @@ -3345,6 +3377,20 @@ out:
> path_put(&nd.path);
> return dentry;
> }
> +
> +struct dentry *kern_path_create(int dfd, const char *pathname,
> + struct path *path, unsigned int lookup_flags)
> +{
> + struct filename *filename;
> + struct dentry *res;
> +
> + filename = getname_kernel(pathname);
> + if (IS_ERR(filename))
> + return ERR_CAST(filename);
> + res = filename_create(dfd, filename, path, lookup_flags);
> + putname(filename);
> + return res;
> +}
> EXPORT_SYMBOL(kern_path_create);
>
> void done_path_create(struct path *path, struct dentry *dentry)
> @@ -3363,7 +3409,7 @@ struct dentry *user_path_create(int dfd, const char __user *pathname,
> struct dentry *res;
> if (IS_ERR(tmp))
> return ERR_CAST(tmp);
> - res = kern_path_create(dfd, tmp->name, path, lookup_flags);
> + res = filename_create(dfd, tmp, path, lookup_flags);
> putname(tmp);
> return res;
> }
> diff --git a/fs/open.c b/fs/open.c
> index d6fd3ac..33839e1 100644
> --- a/fs/open.c
> +++ b/fs/open.c
> @@ -940,8 +940,15 @@ struct file *file_open_name(struct filename *name, int flags, umode_t mode)
> */
> struct file *filp_open(const char *filename, int flags, umode_t mode)
> {
> - struct filename name = {.name = filename};
> - return file_open_name(&name, flags, mode);
> + struct file *file;
> + struct filename *name;
> +
> + name = getname_kernel(filename);
> + if (unlikely(IS_ERR(name)))
> + return ERR_CAST(name);
> + file = file_open_name(name, flags, mode);
> + putname(name);
> + return file;
> }
> EXPORT_SYMBOL(filp_open);
>
>
- RGB
--
Richard Guy Briggs <rbriggs@redhat.com>
Senior Software Engineer, Kernel Security, AMER ENG Base Operating Systems, Red Hat
Remote, Ottawa, Canada
Voice: +1.647.777.2635, Internal: (81) 32635, Alt: +1.613.693.0684x3545
^ permalink raw reply
* Re: [PATCH v2 1/5] fs: rework getname_kernel to handle up to PATH_MAX sized filenames
From: Richard Guy Briggs @ 2015-01-22 15:53 UTC (permalink / raw)
To: Paul Moore; +Cc: linux-fsdevel, linux-audit, sd, linux-kernel, linux, viro
In-Reply-To: <20150122045956.1347.11508.stgit@localhost>
On 15/01/21, Paul Moore wrote:
> In preparation for expanded use in the kernel, make getname_kernel()
> more useful by allowing it to handle any legal filename length.
>
> Thanks to Guenter Roeck for his suggestion to substitute memcpy() for
> strlcpy().
>
> CC: linux@roeck-us.net
> CC: viro@zeniv.linux.org.uk
> CC: linux-fsdevel@vger.kernel.org
> Signed-off-by: Paul Moore <pmoore@redhat.com>
Noted memcpy() difference (I assume to avoid races).
Reviewed-by: Richard Guy Briggs <rgb@redhat.com>
> ---
> fs/namei.c | 34 ++++++++++++++++++++--------------
> 1 file changed, 20 insertions(+), 14 deletions(-)
>
> diff --git a/fs/namei.c b/fs/namei.c
> index 9eb787e..63eaaf6 100644
> --- a/fs/namei.c
> +++ b/fs/namei.c
> @@ -211,32 +211,38 @@ getname(const char __user * filename)
> return getname_flags(filename, 0, NULL);
> }
>
> -/*
> - * The "getname_kernel()" interface doesn't do pathnames longer
> - * than EMBEDDED_NAME_MAX. Deal with it - you're a kernel user.
> - */
> struct filename *
> getname_kernel(const char * filename)
> {
> struct filename *result;
> - char *kname;
> - int len;
> -
> - len = strlen(filename);
> - if (len >= EMBEDDED_NAME_MAX)
> - return ERR_PTR(-ENAMETOOLONG);
> + int len = strlen(filename) + 1;
>
> result = __getname();
> if (unlikely(!result))
> return ERR_PTR(-ENOMEM);
>
> - kname = (char *)result + sizeof(*result);
> - result->name = kname;
> + if (len <= EMBEDDED_NAME_MAX) {
> + result->name = (char *)(result) + sizeof(*result);
> + result->separate = false;
> + } else if (len <= PATH_MAX) {
> + struct filename *tmp;
> +
> + tmp = kmalloc(sizeof(*tmp), GFP_KERNEL);
> + if (unlikely(!tmp)) {
> + __putname(result);
> + return ERR_PTR(-ENOMEM);
> + }
> + tmp->name = (char *)result;
> + tmp->separate = true;
> + result = tmp;
> + } else {
> + __putname(result);
> + return ERR_PTR(-ENAMETOOLONG);
> + }
> + memcpy((char *)result->name, filename, len);
> result->uptr = NULL;
> result->aname = NULL;
> - result->separate = false;
>
> - strlcpy(kname, filename, EMBEDDED_NAME_MAX);
> return result;
> }
>
>
- RGB
--
Richard Guy Briggs <rbriggs@redhat.com>
Senior Software Engineer, Kernel Security, AMER ENG Base Operating Systems, Red Hat
Remote, Ottawa, Canada
Voice: +1.647.777.2635, Internal: (81) 32635, Alt: +1.613.693.0684x3545
^ permalink raw reply
* Re: [PATCH v2 0/5] Overhaul the audit filename handling
From: Al Viro @ 2015-01-22 7:54 UTC (permalink / raw)
To: Guenter Roeck
Cc: Paul Moore, linux-fsdevel, linux-audit, rgb, sd, linux-kernel
In-Reply-To: <54C08C62.8010609@roeck-us.net>
On Wed, Jan 21, 2015 at 09:36:34PM -0800, Guenter Roeck wrote:
> On 01/21/2015 08:59 PM, Paul Moore wrote:
> >This patchset has some important changes from the previous revision,
> >namely a fix from Al Viro (included in 2/5) that resolves a boot panic
> >on some systems as well as some smaller, less noteworthy fixes found
> >in the linux-next announcement thread from January 20th (refcount bump
> >in __audit_reusename() and a inode type in __audit_inode()).
> >
> >This patchset still needs some additional testing to verify that the
> >audit code still functions properly (the minor fixes mentioned above)
> >and there is an additional patch from Al that should be included as
> >well, but I wanted to post this and push the series to the audit next
> >branch quickly since a number of folks were affected by the boot panic.
> >
> >---
> >
> >Paul Moore (5):
> > fs: rework getname_kernel to handle up to PATH_MAX sized filenames
> > fs: create proper filename objects using getname_kernel()
> > audit: enable filename recording via getname_kernel()
> > audit: fix filename matching in __audit_inode() and __audit_inode_child()
> > audit: replace getname()/putname() hacks with reference counters
> >
> Hi Paul,
>
> What is the baseline for this patch set ? Obviously -next won't work,
> and it does not apply to mainline either.
FWIW, I've ported that on top of vfs.git#for-next; result is in
vfs.git#experimental. Paul, are you OK with that one?
^ permalink raw reply
* Re: [PATCH v2 0/5] Overhaul the audit filename handling
From: Guenter Roeck @ 2015-01-22 5:36 UTC (permalink / raw)
To: Paul Moore, linux-fsdevel, linux-audit; +Cc: rgb, sd, linux-kernel, viro
In-Reply-To: <20150122045303.1347.98054.stgit@localhost>
On 01/21/2015 08:59 PM, Paul Moore wrote:
> This patchset has some important changes from the previous revision,
> namely a fix from Al Viro (included in 2/5) that resolves a boot panic
> on some systems as well as some smaller, less noteworthy fixes found
> in the linux-next announcement thread from January 20th (refcount bump
> in __audit_reusename() and a inode type in __audit_inode()).
>
> This patchset still needs some additional testing to verify that the
> audit code still functions properly (the minor fixes mentioned above)
> and there is an additional patch from Al that should be included as
> well, but I wanted to post this and push the series to the audit next
> branch quickly since a number of folks were affected by the boot panic.
>
> ---
>
> Paul Moore (5):
> fs: rework getname_kernel to handle up to PATH_MAX sized filenames
> fs: create proper filename objects using getname_kernel()
> audit: enable filename recording via getname_kernel()
> audit: fix filename matching in __audit_inode() and __audit_inode_child()
> audit: replace getname()/putname() hacks with reference counters
>
Hi Paul,
What is the baseline for this patch set ? Obviously -next won't work,
and it does not apply to mainline either.
Thanks,
Guenter
^ permalink raw reply
* [PATCH v2 5/5] audit: replace getname()/putname() hacks with reference counters
From: Paul Moore @ 2015-01-22 5:00 UTC (permalink / raw)
To: linux-fsdevel, linux-audit; +Cc: rgb, sd, linux-kernel, linux, viro
In-Reply-To: <20150122045303.1347.98054.stgit@localhost>
In order to ensure that filenames are not released before the audit
subsystem is done with the strings there are a number of hacks built
into the fs and audit subsystems around getname() and putname(). To
say these hacks are "ugly" would be kind.
This patch removes the filename hackery in favor of a more
conventional reference count based approach. The diffstat below tells
most of the story; lots of audit/fs specific code is replaced with a
traditional reference count based approach that is easily understood,
even by those not familiar with the audit and/or fs subsystems.
CC: viro@zeniv.linux.org.uk
CC: linux-fsdevel@vger.kernel.org
Signed-off-by: Paul Moore <pmoore@redhat.com>
---
fs/namei.c | 29 +++++++-------
include/linux/audit.h | 3 -
include/linux/fs.h | 9 +---
kernel/audit.h | 17 +-------
kernel/auditsc.c | 105 +++++--------------------------------------------
5 files changed, 29 insertions(+), 134 deletions(-)
diff --git a/fs/namei.c b/fs/namei.c
index e18a2b5..f73e757 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -117,15 +117,6 @@
* POSIX.1 2.4: an empty pathname is invalid (ENOENT).
* PATH_MAX includes the nul terminator --RR.
*/
-void final_putname(struct filename *name)
-{
- if (name->separate) {
- __putname(name->name);
- kfree(name);
- } else {
- __putname(name);
- }
-}
#define EMBEDDED_NAME_MAX (PATH_MAX - sizeof(struct filename))
@@ -144,6 +135,7 @@ getname_flags(const char __user *filename, int flags, int *empty)
result = __getname();
if (unlikely(!result))
return ERR_PTR(-ENOMEM);
+ result->refcnt = 1;
/*
* First, try to embed the struct filename inside the names_cache
@@ -178,6 +170,7 @@ recopy:
}
result->name = kname;
result->separate = true;
+ result->refcnt = 1;
max = PATH_MAX;
goto recopy;
}
@@ -201,7 +194,7 @@ recopy:
return result;
error:
- final_putname(result);
+ putname(result);
return err;
}
@@ -242,19 +235,25 @@ getname_kernel(const char * filename)
memcpy((char *)result->name, filename, len);
result->uptr = NULL;
result->aname = NULL;
+ result->refcnt = 1;
audit_getname(result);
return result;
}
-#ifdef CONFIG_AUDITSYSCALL
void putname(struct filename *name)
{
- if (unlikely(!audit_dummy_context()))
- return audit_putname(name);
- final_putname(name);
+ BUG_ON(name->refcnt <= 0);
+
+ if (--name->refcnt > 0)
+ return;
+
+ if (name->separate) {
+ __putname(name->name);
+ kfree(name);
+ } else
+ __putname(name);
}
-#endif
static int check_acl(struct inode *inode, int mask)
{
diff --git a/include/linux/audit.h b/include/linux/audit.h
index b481779..5f2ad5f 100644
--- a/include/linux/audit.h
+++ b/include/linux/audit.h
@@ -127,7 +127,6 @@ extern void __audit_syscall_entry(int major, unsigned long a0, unsigned long a1,
extern void __audit_syscall_exit(int ret_success, long ret_value);
extern struct filename *__audit_reusename(const __user char *uptr);
extern void __audit_getname(struct filename *name);
-extern void audit_putname(struct filename *name);
#define AUDIT_INODE_PARENT 1 /* dentry represents the parent */
#define AUDIT_INODE_HIDDEN 2 /* audit record should be hidden */
@@ -346,8 +345,6 @@ static inline struct filename *audit_reusename(const __user char *name)
}
static inline void audit_getname(struct filename *name)
{ }
-static inline void audit_putname(struct filename *name)
-{ }
static inline void __audit_inode(struct filename *name,
const struct dentry *dentry,
unsigned int flags)
diff --git a/include/linux/fs.h b/include/linux/fs.h
index e11d60c..df7a047 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -2017,6 +2017,7 @@ struct filename {
const char *name; /* pointer to actual string */
const __user char *uptr; /* original userland pointer */
struct audit_names *aname;
+ int refcnt;
bool separate; /* should "name" be freed? */
};
@@ -2036,6 +2037,7 @@ extern int filp_close(struct file *, fl_owner_t id);
extern struct filename *getname(const char __user *);
extern struct filename *getname_kernel(const char *);
+extern void putname(struct filename *name);
enum {
FILE_CREATED = 1,
@@ -2056,15 +2058,8 @@ extern void __init vfs_caches_init(unsigned long);
extern struct kmem_cache *names_cachep;
-extern void final_putname(struct filename *name);
-
#define __getname() kmem_cache_alloc(names_cachep, GFP_KERNEL)
#define __putname(name) kmem_cache_free(names_cachep, (void *)(name))
-#ifndef CONFIG_AUDITSYSCALL
-#define putname(name) final_putname(name)
-#else
-extern void putname(struct filename *name);
-#endif
#ifdef CONFIG_BLOCK
extern int register_blkdev(unsigned int, const char *);
diff --git a/kernel/audit.h b/kernel/audit.h
index 3cdffad..1caa0d3 100644
--- a/kernel/audit.h
+++ b/kernel/audit.h
@@ -24,12 +24,6 @@
#include <linux/skbuff.h>
#include <uapi/linux/mqueue.h>
-/* 0 = no checking
- 1 = put_count checking
- 2 = verbose put_count checking
-*/
-#define AUDIT_DEBUG 0
-
/* AUDIT_NAMES is the number of slots we reserve in the audit_context
* for saving names from getname(). If we get more names we will allocate
* a name dynamically and also add those to the list anchored by names_list. */
@@ -74,9 +68,8 @@ struct audit_cap_data {
};
};
-/* When fs/namei.c:getname() is called, we store the pointer in name and
- * we don't let putname() free it (instead we free all of the saved
- * pointers at syscall exit time).
+/* When fs/namei.c:getname() is called, we store the pointer in name and bump
+ * the refcnt in the associated filename struct.
*
* Further, in fs/namei.c:path_lookup() we store the inode and device.
*/
@@ -86,7 +79,6 @@ struct audit_names {
struct filename *name;
int name_len; /* number of chars to log */
bool hidden; /* don't log this record */
- bool name_put; /* call __putname()? */
unsigned long ino;
dev_t dev;
@@ -208,11 +200,6 @@ struct audit_context {
};
int fds[2];
struct audit_proctitle proctitle;
-
-#if AUDIT_DEBUG
- int put_count;
- int ino_count;
-#endif
};
extern u32 audit_ever_enabled;
diff --git a/kernel/auditsc.c b/kernel/auditsc.c
index c54b5f0..0a93b71 100644
--- a/kernel/auditsc.c
+++ b/kernel/auditsc.c
@@ -866,33 +866,10 @@ static inline void audit_free_names(struct audit_context *context)
{
struct audit_names *n, *next;
-#if AUDIT_DEBUG == 2
- if (context->put_count + context->ino_count != context->name_count) {
- int i = 0;
-
- pr_err("%s:%d(:%d): major=%d in_syscall=%d"
- " name_count=%d put_count=%d ino_count=%d"
- " [NOT freeing]\n", __FILE__, __LINE__,
- context->serial, context->major, context->in_syscall,
- context->name_count, context->put_count,
- context->ino_count);
- list_for_each_entry(n, &context->names_list, list) {
- pr_err("names[%d] = %p = %s\n", i++, n->name,
- n->name->name ?: "(null)");
- }
- dump_stack();
- return;
- }
-#endif
-#if AUDIT_DEBUG
- context->put_count = 0;
- context->ino_count = 0;
-#endif
-
list_for_each_entry_safe(n, next, &context->names_list, list) {
list_del(&n->list);
- if (n->name && n->name_put)
- final_putname(n->name);
+ if (n->name)
+ putname(n->name);
if (n->should_free)
kfree(n);
}
@@ -1711,9 +1688,6 @@ static struct audit_names *audit_alloc_name(struct audit_context *context,
list_add_tail(&aname->list, &context->names_list);
context->name_count++;
-#if AUDIT_DEBUG
- context->ino_count++;
-#endif
return aname;
}
@@ -1734,8 +1708,10 @@ __audit_reusename(const __user char *uptr)
list_for_each_entry(n, &context->names_list, list) {
if (!n->name)
continue;
- if (n->name->uptr == uptr)
+ if (n->name->uptr == uptr) {
+ n->name->refcnt++;
return n->name;
+ }
}
return NULL;
}
@@ -1752,19 +1728,8 @@ void __audit_getname(struct filename *name)
struct audit_context *context = current->audit_context;
struct audit_names *n;
- if (!context->in_syscall) {
-#if AUDIT_DEBUG == 2
- pr_err("%s:%d(:%d): ignoring getname(%p)\n",
- __FILE__, __LINE__, context->serial, name);
- dump_stack();
-#endif
+ if (!context->in_syscall)
return;
- }
-
-#if AUDIT_DEBUG
- /* The filename _must_ have a populated ->name */
- BUG_ON(!name->name);
-#endif
n = audit_alloc_name(context, AUDIT_TYPE_UNKNOWN);
if (!n)
@@ -1772,56 +1737,13 @@ void __audit_getname(struct filename *name)
n->name = name;
n->name_len = AUDIT_NAME_FULL;
- n->name_put = true;
name->aname = n;
+ name->refcnt++;
if (!context->pwd.dentry)
get_fs_pwd(current->fs, &context->pwd);
}
-/* audit_putname - intercept a putname request
- * @name: name to intercept and delay for putname
- *
- * If we have stored the name from getname in the audit context,
- * then we delay the putname until syscall exit.
- * Called from include/linux/fs.h:putname().
- */
-void audit_putname(struct filename *name)
-{
- struct audit_context *context = current->audit_context;
-
- BUG_ON(!context);
- if (!name->aname || !context->in_syscall) {
-#if AUDIT_DEBUG == 2
- pr_err("%s:%d(:%d): final_putname(%p)\n",
- __FILE__, __LINE__, context->serial, name);
- if (context->name_count) {
- struct audit_names *n;
- int i = 0;
-
- list_for_each_entry(n, &context->names_list, list)
- pr_err("name[%d] = %p = %s\n", i++, n->name,
- n->name->name ?: "(null)");
- }
-#endif
- final_putname(name);
- }
-#if AUDIT_DEBUG
- else {
- ++context->put_count;
- if (context->put_count > context->name_count) {
- pr_err("%s:%d(:%d): major=%d in_syscall=%d putname(%p)"
- " name_count=%d put_count=%d\n",
- __FILE__, __LINE__,
- context->serial, context->major,
- context->in_syscall, name->name,
- context->name_count, context->put_count);
- dump_stack();
- }
- }
-#endif
-}
-
/**
* __audit_inode - store the inode and device from a lookup
* @name: name being audited
@@ -1842,11 +1764,6 @@ void __audit_inode(struct filename *name, const struct dentry *dentry,
if (!name)
goto out_alloc;
-#if AUDIT_DEBUG
- /* The struct filename _must_ have a populated ->name */
- BUG_ON(!name->name);
-#endif
-
/*
* If we have a pointer to an audit_names entry already, then we can
* just use it directly if the type is correct.
@@ -1893,9 +1810,10 @@ out_alloc:
n = audit_alloc_name(context, AUDIT_TYPE_UNKNOWN);
if (!n)
return;
- if (name)
- /* no need to set ->name_put as the original will cleanup */
+ if (name) {
n->name = name;
+ name->refcnt++;
+ }
out:
if (parent) {
@@ -1995,8 +1913,7 @@ void __audit_inode_child(const struct inode *parent,
if (found_parent) {
found_child->name = found_parent->name;
found_child->name_len = AUDIT_NAME_FULL;
- /* don't call __putname() */
- found_child->name_put = false;
+ found_child->name->refcnt++;
}
}
^ permalink raw reply related
* [PATCH v2 4/5] audit: fix filename matching in __audit_inode() and __audit_inode_child()
From: Paul Moore @ 2015-01-22 5:00 UTC (permalink / raw)
To: linux-fsdevel, linux-audit; +Cc: rgb, sd, linux-kernel, linux, viro
In-Reply-To: <20150122045303.1347.98054.stgit@localhost>
In all likelihood there were some subtle, and perhaps not so subtle,
bugs with filename matching in audit_inode() and audit_inode_child()
for some time, however, recent changes to the audit filename code have
definitely broken the filename matching code. The breakage could
result in duplicate filenames in the audit log and other odd audit
record entries. This patch fixes the filename matching code and
restores some sanity to the filename audit records.
CC: viro@zeniv.linux.org.uk
CC: linux-fsdevel@vger.kernel.org
Signed-off-by: Paul Moore <pmoore@redhat.com>
---
kernel/auditsc.c | 34 +++++++++++++++++++++++++---------
1 file changed, 25 insertions(+), 9 deletions(-)
diff --git a/kernel/auditsc.c b/kernel/auditsc.c
index c967ffc..c54b5f0 100644
--- a/kernel/auditsc.c
+++ b/kernel/auditsc.c
@@ -1846,6 +1846,7 @@ void __audit_inode(struct filename *name, const struct dentry *dentry,
/* The struct filename _must_ have a populated ->name */
BUG_ON(!name->name);
#endif
+
/*
* If we have a pointer to an audit_names entry already, then we can
* just use it directly if the type is correct.
@@ -1863,7 +1864,17 @@ void __audit_inode(struct filename *name, const struct dentry *dentry,
}
list_for_each_entry_reverse(n, &context->names_list, list) {
- if (!n->name || strcmp(n->name->name, name->name))
+ if (n->ino) {
+ /* valid inode number, use that for the comparison */
+ if (n->ino != inode->i_ino ||
+ n->dev != inode->i_sb->s_dev)
+ continue;
+ } else if (n->name) {
+ /* inode number has not been set, check the name */
+ if (strcmp(n->name->name, name->name))
+ continue;
+ } else
+ /* no inode and no name (?!) ... this is odd ... */
continue;
/* match the correct record type */
@@ -1931,11 +1942,16 @@ void __audit_inode_child(const struct inode *parent,
/* look for a parent entry first */
list_for_each_entry(n, &context->names_list, list) {
- if (!n->name || n->type != AUDIT_TYPE_PARENT)
+ if (!n->name ||
+ (n->type != AUDIT_TYPE_PARENT &&
+ n->type != AUDIT_TYPE_UNKNOWN))
continue;
- if (n->ino == parent->i_ino &&
- !audit_compare_dname_path(dname, n->name->name, n->name_len)) {
+ if (n->ino == parent->i_ino && n->dev == parent->i_sb->s_dev &&
+ !audit_compare_dname_path(dname,
+ n->name->name, n->name_len)) {
+ if (n->type == AUDIT_TYPE_UNKNOWN)
+ n->type = AUDIT_TYPE_PARENT;
found_parent = n;
break;
}
@@ -1944,11 +1960,8 @@ void __audit_inode_child(const struct inode *parent,
/* is there a matching child entry? */
list_for_each_entry(n, &context->names_list, list) {
/* can only match entries that have a name */
- if (!n->name || n->type != type)
- continue;
-
- /* if we found a parent, make sure this one is a child of it */
- if (found_parent && (n->name != found_parent->name))
+ if (!n->name ||
+ (n->type != type && n->type != AUDIT_TYPE_UNKNOWN))
continue;
if (!strcmp(dname, n->name->name) ||
@@ -1956,6 +1969,8 @@ void __audit_inode_child(const struct inode *parent,
found_parent ?
found_parent->name_len :
AUDIT_NAME_FULL)) {
+ if (n->type == AUDIT_TYPE_UNKNOWN)
+ n->type = type;
found_child = n;
break;
}
@@ -1984,6 +1999,7 @@ void __audit_inode_child(const struct inode *parent,
found_child->name_put = false;
}
}
+
if (inode)
audit_copy_inode(found_child, dentry, inode);
else
^ permalink raw reply related
* [PATCH v2 3/5] audit: enable filename recording via getname_kernel()
From: Paul Moore @ 2015-01-22 5:00 UTC (permalink / raw)
To: linux-fsdevel, linux-audit; +Cc: rgb, viro, linux-kernel, linux, sd
In-Reply-To: <20150122045303.1347.98054.stgit@localhost>
Enable recording of filenames in getname_kernel() and remove the
kludgy workaround in __audit_inode() now that we have proper filename
logging for kernel users.
CC: viro@zeniv.linux.org.uk
CC: linux-fsdevel@vger.kernel.org
Signed-off-by: Paul Moore <pmoore@redhat.com>
Reviewed-by: Richard Guy Briggs <rgb@redhat.com>
---
fs/namei.c | 1 +
kernel/auditsc.c | 40 +++-------------------------------------
2 files changed, 4 insertions(+), 37 deletions(-)
diff --git a/fs/namei.c b/fs/namei.c
index f793fe4..e18a2b5 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -242,6 +242,7 @@ getname_kernel(const char * filename)
memcpy((char *)result->name, filename, len);
result->uptr = NULL;
result->aname = NULL;
+ audit_getname(result);
return result;
}
diff --git a/kernel/auditsc.c b/kernel/auditsc.c
index 793e9e9..c967ffc 100644
--- a/kernel/auditsc.c
+++ b/kernel/auditsc.c
@@ -1882,44 +1882,10 @@ out_alloc:
n = audit_alloc_name(context, AUDIT_TYPE_UNKNOWN);
if (!n)
return;
- /* unfortunately, while we may have a path name to record with the
- * inode, we can't always rely on the string lasting until the end of
- * the syscall so we need to create our own copy, it may fail due to
- * memory allocation issues, but we do our best */
- if (name) {
- /* we can't use getname_kernel() due to size limits */
- size_t len = strlen(name->name) + 1;
- struct filename *new = __getname();
-
- if (unlikely(!new))
- goto out;
+ if (name)
+ /* no need to set ->name_put as the original will cleanup */
+ n->name = name;
- if (len <= (PATH_MAX - sizeof(*new))) {
- new->name = (char *)(new) + sizeof(*new);
- new->separate = false;
- } else if (len <= PATH_MAX) {
- /* this looks odd, but is due to final_putname() */
- struct filename *new2;
-
- new2 = kmalloc(sizeof(*new2), GFP_KERNEL);
- if (unlikely(!new2)) {
- __putname(new);
- goto out;
- }
- new2->name = (char *)new;
- new2->separate = true;
- new = new2;
- } else {
- /* we should never get here, but let's be safe */
- __putname(new);
- goto out;
- }
- strlcpy((char *)new->name, name->name, len);
- new->uptr = NULL;
- new->aname = n;
- n->name = new;
- n->name_put = true;
- }
out:
if (parent) {
n->name_len = n->name ? parent_len(n->name->name) : AUDIT_NAME_FULL;
^ permalink raw reply related
* [PATCH v2 2/5] fs: create proper filename objects using getname_kernel()
From: Paul Moore @ 2015-01-22 5:00 UTC (permalink / raw)
To: linux-fsdevel, linux-audit; +Cc: rgb, sd, linux-kernel, linux, viro
In-Reply-To: <20150122045303.1347.98054.stgit@localhost>
There are several areas in the kernel that create temporary filename
objects using the following pattern:
int func(const char *name)
{
struct filename *file = { .name = name };
...
return 0;
}
... which for the most part works okay, but it causes havoc within the
audit subsystem as the filename object does not persist beyond the
lifetime of the function. This patch converts all of these temporary
filename objects into proper filename objects using getname_kernel()
and putname() which ensure that the filename object persists until the
audit subsystem is finished with it.
Also, a special thanks to Al Viro, Guenter Roeck, and Sabrina Dubroca
for helping resolve a difficult kernel panic on boot related to a
use-after-free problem in kern_path_create(); the thread can be seen
at the link below:
* https://lkml.org/lkml/2015/1/20/710
This patch includes code that was either based on, or directly written
by Al in the above thread.
CC: viro@zeniv.linux.org.uk
CC: linux@roeck-us.net
CC: sd@queasysnail.net
CC: linux-fsdevel@vger.kernel.org
Signed-off-by: Paul Moore <pmoore@redhat.com>
---
fs/exec.c | 11 +++++++-
fs/namei.c | 80 +++++++++++++++++++++++++++++++++++++++++++++++-------------
fs/open.c | 11 +++++++-
3 files changed, 81 insertions(+), 21 deletions(-)
diff --git a/fs/exec.c b/fs/exec.c
index a3d33fe..d067771 100644
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -789,8 +789,15 @@ exit:
struct file *open_exec(const char *name)
{
- struct filename tmp = { .name = name };
- return do_open_exec(&tmp);
+ struct file *file;
+ struct filename *tmp;
+
+ tmp = getname_kernel(name);
+ if (unlikely(IS_ERR(tmp)))
+ return (void *)tmp;
+ file = do_open_exec(tmp);
+ putname(tmp);
+ return file;
}
EXPORT_SYMBOL(open_exec);
diff --git a/fs/namei.c b/fs/namei.c
index 63eaaf6..f793fe4 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -2001,31 +2001,50 @@ static int filename_lookup(int dfd, struct filename *name,
static int do_path_lookup(int dfd, const char *name,
unsigned int flags, struct nameidata *nd)
{
- struct filename filename = { .name = name };
+ int retval;
+ struct filename *filename;
- return filename_lookup(dfd, &filename, flags, nd);
+ filename = getname_kernel(name);
+ if (unlikely(IS_ERR(filename)))
+ return PTR_ERR(filename);
+ retval = filename_lookup(dfd, filename, flags, nd);
+ putname(filename);
+ return retval;
}
/* does lookup, returns the object with parent locked */
struct dentry *kern_path_locked(const char *name, struct path *path)
{
+ struct filename *filename;
struct nameidata nd;
struct dentry *d;
- int err = do_path_lookup(AT_FDCWD, name, LOOKUP_PARENT, &nd);
- if (err)
- return ERR_PTR(err);
+ int err;
+
+ filename = getname_kernel(name);
+ if (IS_ERR(filename))
+ return ERR_CAST(filename);
+
+ err = filename_lookup(AT_FDCWD, filename, LOOKUP_PARENT, &nd);
+ if (err) {
+ d = ERR_PTR(err);
+ goto out;
+ }
if (nd.last_type != LAST_NORM) {
path_put(&nd.path);
- return ERR_PTR(-EINVAL);
+ d = ERR_PTR(-EINVAL);
+ goto out;
}
mutex_lock_nested(&nd.path.dentry->d_inode->i_mutex, I_MUTEX_PARENT);
d = __lookup_hash(&nd.last, nd.path.dentry, 0);
if (IS_ERR(d)) {
mutex_unlock(&nd.path.dentry->d_inode->i_mutex);
path_put(&nd.path);
- return d;
+ goto out;
}
*path = nd.path;
+
+out:
+ putname(filename);
return d;
}
@@ -2368,8 +2387,15 @@ int
kern_path_mountpoint(int dfd, const char *name, struct path *path,
unsigned int flags)
{
- struct filename s = {.name = name};
- return filename_mountpoint(dfd, &s, path, flags);
+ int retval;
+ struct filename *s;
+
+ s = getname_kernel(name);
+ if (unlikely(IS_ERR(s)))
+ return PTR_ERR(s);
+ retval = filename_mountpoint(dfd, s, path, flags);
+ putname(s);
+ return retval;
}
EXPORT_SYMBOL(kern_path_mountpoint);
@@ -3259,7 +3285,7 @@ struct file *do_file_open_root(struct dentry *dentry, struct vfsmount *mnt,
{
struct nameidata nd;
struct file *file;
- struct filename filename = { .name = name };
+ struct filename *filename;
int flags = op->lookup_flags | LOOKUP_ROOT;
nd.root.mnt = mnt;
@@ -3268,16 +3294,22 @@ struct file *do_file_open_root(struct dentry *dentry, struct vfsmount *mnt,
if (d_is_symlink(dentry) && op->intent & LOOKUP_OPEN)
return ERR_PTR(-ELOOP);
- file = path_openat(-1, &filename, &nd, op, flags | LOOKUP_RCU);
+ filename = getname_kernel(name);
+ if (unlikely(IS_ERR(filename)))
+ return ERR_CAST(filename);
+
+ file = path_openat(-1, filename, &nd, op, flags | LOOKUP_RCU);
if (unlikely(file == ERR_PTR(-ECHILD)))
- file = path_openat(-1, &filename, &nd, op, flags);
+ file = path_openat(-1, filename, &nd, op, flags);
if (unlikely(file == ERR_PTR(-ESTALE)))
- file = path_openat(-1, &filename, &nd, op, flags | LOOKUP_REVAL);
+ file = path_openat(-1, filename, &nd, op, flags | LOOKUP_REVAL);
+ putname(filename);
return file;
}
-struct dentry *kern_path_create(int dfd, const char *pathname,
- struct path *path, unsigned int lookup_flags)
+static struct dentry *filename_create(int dfd, struct filename *name,
+ struct path *path,
+ unsigned int lookup_flags)
{
struct dentry *dentry = ERR_PTR(-EEXIST);
struct nameidata nd;
@@ -3291,7 +3323,7 @@ struct dentry *kern_path_create(int dfd, const char *pathname,
*/
lookup_flags &= LOOKUP_REVAL;
- error = do_path_lookup(dfd, pathname, LOOKUP_PARENT|lookup_flags, &nd);
+ error = filename_lookup(dfd, name, LOOKUP_PARENT|lookup_flags, &nd);
if (error)
return ERR_PTR(error);
@@ -3345,6 +3377,20 @@ out:
path_put(&nd.path);
return dentry;
}
+
+struct dentry *kern_path_create(int dfd, const char *pathname,
+ struct path *path, unsigned int lookup_flags)
+{
+ struct filename *filename;
+ struct dentry *res;
+
+ filename = getname_kernel(pathname);
+ if (IS_ERR(filename))
+ return ERR_CAST(filename);
+ res = filename_create(dfd, filename, path, lookup_flags);
+ putname(filename);
+ return res;
+}
EXPORT_SYMBOL(kern_path_create);
void done_path_create(struct path *path, struct dentry *dentry)
@@ -3363,7 +3409,7 @@ struct dentry *user_path_create(int dfd, const char __user *pathname,
struct dentry *res;
if (IS_ERR(tmp))
return ERR_CAST(tmp);
- res = kern_path_create(dfd, tmp->name, path, lookup_flags);
+ res = filename_create(dfd, tmp, path, lookup_flags);
putname(tmp);
return res;
}
diff --git a/fs/open.c b/fs/open.c
index d6fd3ac..33839e1 100644
--- a/fs/open.c
+++ b/fs/open.c
@@ -940,8 +940,15 @@ struct file *file_open_name(struct filename *name, int flags, umode_t mode)
*/
struct file *filp_open(const char *filename, int flags, umode_t mode)
{
- struct filename name = {.name = filename};
- return file_open_name(&name, flags, mode);
+ struct file *file;
+ struct filename *name;
+
+ name = getname_kernel(filename);
+ if (unlikely(IS_ERR(name)))
+ return ERR_CAST(name);
+ file = file_open_name(name, flags, mode);
+ putname(name);
+ return file;
}
EXPORT_SYMBOL(filp_open);
^ permalink raw reply related
* [PATCH v2 1/5] fs: rework getname_kernel to handle up to PATH_MAX sized filenames
From: Paul Moore @ 2015-01-22 4:59 UTC (permalink / raw)
To: linux-fsdevel, linux-audit; +Cc: rgb, sd, linux-kernel, linux, viro
In-Reply-To: <20150122045303.1347.98054.stgit@localhost>
In preparation for expanded use in the kernel, make getname_kernel()
more useful by allowing it to handle any legal filename length.
Thanks to Guenter Roeck for his suggestion to substitute memcpy() for
strlcpy().
CC: linux@roeck-us.net
CC: viro@zeniv.linux.org.uk
CC: linux-fsdevel@vger.kernel.org
Signed-off-by: Paul Moore <pmoore@redhat.com>
---
fs/namei.c | 34 ++++++++++++++++++++--------------
1 file changed, 20 insertions(+), 14 deletions(-)
diff --git a/fs/namei.c b/fs/namei.c
index 9eb787e..63eaaf6 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -211,32 +211,38 @@ getname(const char __user * filename)
return getname_flags(filename, 0, NULL);
}
-/*
- * The "getname_kernel()" interface doesn't do pathnames longer
- * than EMBEDDED_NAME_MAX. Deal with it - you're a kernel user.
- */
struct filename *
getname_kernel(const char * filename)
{
struct filename *result;
- char *kname;
- int len;
-
- len = strlen(filename);
- if (len >= EMBEDDED_NAME_MAX)
- return ERR_PTR(-ENAMETOOLONG);
+ int len = strlen(filename) + 1;
result = __getname();
if (unlikely(!result))
return ERR_PTR(-ENOMEM);
- kname = (char *)result + sizeof(*result);
- result->name = kname;
+ if (len <= EMBEDDED_NAME_MAX) {
+ result->name = (char *)(result) + sizeof(*result);
+ result->separate = false;
+ } else if (len <= PATH_MAX) {
+ struct filename *tmp;
+
+ tmp = kmalloc(sizeof(*tmp), GFP_KERNEL);
+ if (unlikely(!tmp)) {
+ __putname(result);
+ return ERR_PTR(-ENOMEM);
+ }
+ tmp->name = (char *)result;
+ tmp->separate = true;
+ result = tmp;
+ } else {
+ __putname(result);
+ return ERR_PTR(-ENAMETOOLONG);
+ }
+ memcpy((char *)result->name, filename, len);
result->uptr = NULL;
result->aname = NULL;
- result->separate = false;
- strlcpy(kname, filename, EMBEDDED_NAME_MAX);
return result;
}
^ permalink raw reply related
* [PATCH v2 0/5] Overhaul the audit filename handling
From: Paul Moore @ 2015-01-22 4:59 UTC (permalink / raw)
To: linux-fsdevel, linux-audit; +Cc: rgb, sd, linux-kernel, linux, viro
This patchset has some important changes from the previous revision,
namely a fix from Al Viro (included in 2/5) that resolves a boot panic
on some systems as well as some smaller, less noteworthy fixes found
in the linux-next announcement thread from January 20th (refcount bump
in __audit_reusename() and a inode type in __audit_inode()).
This patchset still needs some additional testing to verify that the
audit code still functions properly (the minor fixes mentioned above)
and there is an additional patch from Al that should be included as
well, but I wanted to post this and push the series to the audit next
branch quickly since a number of folks were affected by the boot panic.
---
Paul Moore (5):
fs: rework getname_kernel to handle up to PATH_MAX sized filenames
fs: create proper filename objects using getname_kernel()
audit: enable filename recording via getname_kernel()
audit: fix filename matching in __audit_inode() and __audit_inode_child()
audit: replace getname()/putname() hacks with reference counters
fs/exec.c | 11 +++
fs/namei.c | 144 ++++++++++++++++++++++++++++-------------
fs/open.c | 11 +++
include/linux/audit.h | 3 -
include/linux/fs.h | 9 +--
kernel/audit.h | 17 +----
kernel/auditsc.c | 171 ++++++++++---------------------------------------
7 files changed, 155 insertions(+), 211 deletions(-)
^ permalink raw reply
* Re: linux-next: Tree for Jan 20 -- Kernel panic - Unable to mount root fs
From: Paul Moore @ 2015-01-22 4:49 UTC (permalink / raw)
To: Al Viro
Cc: Guenter Roeck, Sabrina Dubroca, Stephen Rothwell, linux-next,
linux-kernel, linux-fsdevel, linux-audit, Richard Guy Briggs
In-Reply-To: <20150122041241.GT29656@ZenIV.linux.org.uk>
On Thursday, January 22, 2015 04:12:41 AM Al Viro wrote:
> On Wed, Jan 21, 2015 at 09:28:51PM -0500, Paul Moore wrote:
> > Al, do you mind if I fold your patch below into the existing patches?
>
> No problem, but I'd probably prefer to put this series through vfs.git.
> With the following as the first step:
I just finished fixing up the patchset and giving it a quick sanity test on my
system, some additional testing is still needed to verify that all the audit
stuff is still okay (I did change the __audit_inode() as discussed and bumped
the refcount in __audit_reusename()) but I think as things stand the boot
panic problems should be resolved - thanks again for your help.
I'm going to (re)post what I've currently got for a patchset and call it a
day. Tomorrow I'll add your below patch and finish up my testing, if all goes
well I'll repost the whole set. If you would prefer it to go via the VFS
tree, that's fine with me, as long as it hits Linus' tree at some point I'm
happy.
> Cut down on do_path_lookup() callers
>
> Use filename_lookup() instead. And don't bother with creating new
> struct filename when caller already has one.
>
> Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
> ---
> diff --git a/fs/namei.c b/fs/namei.c
> index bc35b02..73fcf42 100644
> --- a/fs/namei.c
> +++ b/fs/namei.c
> @@ -2046,7 +2046,8 @@ struct dentry *kern_path_locked(const char *name,
> struct path *path) {
> struct nameidata nd;
> struct dentry *d;
> - int err = do_path_lookup(AT_FDCWD, name, LOOKUP_PARENT, &nd);
> + struct filename filename = {.name = name};
> + int err = filename_lookup(AT_FDCWD, &filename, LOOKUP_PARENT, &nd);
> if (err)
> return ERR_PTR(err);
> if (nd.last_type != LAST_NORM) {
> @@ -3290,7 +3291,7 @@ struct file *do_file_open_root(struct dentry *dentry,
> struct vfsmount *mnt, return file;
> }
>
> -struct dentry *kern_path_create(int dfd, const char *pathname,
> +static struct dentry *filename_create(int dfd, struct filename *name,
> struct path *path, unsigned int lookup_flags)
> {
> struct dentry *dentry = ERR_PTR(-EEXIST);
> @@ -3305,7 +3306,7 @@ struct dentry *kern_path_create(int dfd, const char
> *pathname, */
> lookup_flags &= LOOKUP_REVAL;
>
> - error = do_path_lookup(dfd, pathname, LOOKUP_PARENT|lookup_flags, &nd);
> + error = filename_lookup(dfd, name, LOOKUP_PARENT|lookup_flags, &nd);
> if (error)
> return ERR_PTR(error);
>
> @@ -3359,6 +3360,13 @@ out:
> path_put(&nd.path);
> return dentry;
> }
> +
> +struct dentry *kern_path_create(int dfd, const char *pathname,
> + struct path *path, unsigned int lookup_flags)
> +{
> + struct filename filename = {.name = pathname};
> + return filename_create(dfd, &filename, path, lookup_flags);
> +}
> EXPORT_SYMBOL(kern_path_create);
>
> void done_path_create(struct path *path, struct dentry *dentry)
> @@ -3377,7 +3385,7 @@ struct dentry *user_path_create(int dfd, const char
> __user *pathname, struct dentry *res;
> if (IS_ERR(tmp))
> return ERR_CAST(tmp);
> - res = kern_path_create(dfd, tmp->name, path, lookup_flags);
> + res = filename_create(dfd, tmp, path, lookup_flags);
> putname(tmp);
> return res;
> }
--
paul moore
security @ redhat
^ permalink raw reply
* Re: linux-next: Tree for Jan 20 -- Kernel panic - Unable to mount root fs
From: Al Viro @ 2015-01-22 4:12 UTC (permalink / raw)
To: Paul Moore
Cc: Guenter Roeck, Sabrina Dubroca, Stephen Rothwell, linux-next,
linux-kernel, linux-fsdevel, linux-audit, Richard Guy Briggs
In-Reply-To: <1829267.G68Yz0UZrY@sifl>
On Wed, Jan 21, 2015 at 09:28:51PM -0500, Paul Moore wrote:
> Al, do you mind if I fold your patch below into the existing patches?
No problem, but I'd probably prefer to put this series through vfs.git.
With the following as the first step:
Cut down on do_path_lookup() callers
Use filename_lookup() instead. And don't bother with creating new
struct filename when caller already has one.
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
diff --git a/fs/namei.c b/fs/namei.c
index bc35b02..73fcf42 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -2046,7 +2046,8 @@ struct dentry *kern_path_locked(const char *name, struct path *path)
{
struct nameidata nd;
struct dentry *d;
- int err = do_path_lookup(AT_FDCWD, name, LOOKUP_PARENT, &nd);
+ struct filename filename = {.name = name};
+ int err = filename_lookup(AT_FDCWD, &filename, LOOKUP_PARENT, &nd);
if (err)
return ERR_PTR(err);
if (nd.last_type != LAST_NORM) {
@@ -3290,7 +3291,7 @@ struct file *do_file_open_root(struct dentry *dentry, struct vfsmount *mnt,
return file;
}
-struct dentry *kern_path_create(int dfd, const char *pathname,
+static struct dentry *filename_create(int dfd, struct filename *name,
struct path *path, unsigned int lookup_flags)
{
struct dentry *dentry = ERR_PTR(-EEXIST);
@@ -3305,7 +3306,7 @@ struct dentry *kern_path_create(int dfd, const char *pathname,
*/
lookup_flags &= LOOKUP_REVAL;
- error = do_path_lookup(dfd, pathname, LOOKUP_PARENT|lookup_flags, &nd);
+ error = filename_lookup(dfd, name, LOOKUP_PARENT|lookup_flags, &nd);
if (error)
return ERR_PTR(error);
@@ -3359,6 +3360,13 @@ out:
path_put(&nd.path);
return dentry;
}
+
+struct dentry *kern_path_create(int dfd, const char *pathname,
+ struct path *path, unsigned int lookup_flags)
+{
+ struct filename filename = {.name = pathname};
+ return filename_create(dfd, &filename, path, lookup_flags);
+}
EXPORT_SYMBOL(kern_path_create);
void done_path_create(struct path *path, struct dentry *dentry)
@@ -3377,7 +3385,7 @@ struct dentry *user_path_create(int dfd, const char __user *pathname,
struct dentry *res;
if (IS_ERR(tmp))
return ERR_CAST(tmp);
- res = kern_path_create(dfd, tmp->name, path, lookup_flags);
+ res = filename_create(dfd, tmp, path, lookup_flags);
putname(tmp);
return res;
}
^ permalink raw reply related
* Re: linux-next: Tree for Jan 20 -- Kernel panic - Unable to mount root fs
From: Paul Moore @ 2015-01-22 2:28 UTC (permalink / raw)
To: Al Viro
Cc: Guenter Roeck, Sabrina Dubroca, Stephen Rothwell, linux-next,
linux-kernel, linux-fsdevel, linux-audit, Richard Guy Briggs
In-Reply-To: <20150121212833.GS29656@ZenIV.linux.org.uk>
On Wednesday, January 21, 2015 09:28:33 PM Al Viro wrote:
> On Wed, Jan 21, 2015 at 01:03:20PM -0800, Guenter Roeck wrote:
> > failing case:
> >
> > path_lookupat: calling path_init 'usr' flags=40
> > path_init: link_path_walk() returned 0
> > path_lookupat: path_init 'usr' flags=40[50] returned 0
> > walk_component: lookup_fast() returned 1
> > walk_component: lookup_slow() returned 0
> > walk_component: inode= (null), negative=1
> > do_path_lookup(usr, 0x10)
> > path_lookupat: calling path_init 'usr' flags=50
> > path_init: link_path_walk() returned 0
> > path_lookupat: path_init 'usr' flags=50[50] returned 0
> > mkdir[c74012a0,/kkk] => 0 <==== SIC!
>
> Cute. 'k' being 0x6b, aka POISON_FREE... OK, the next question is what's
> been freed under us - I don't believe that it's dentry itself...
> Oh, fuck. OK, I see what happens. Look at kern_path_create(); it does
> LOOKUP_PARENT walk, leaving nd->last pointing to the last component of
> the *COPY* of the name it's just created, walked and freed.
>
> OK... Fortunately, struct nameidata is completely opaque outside of
> fs/namei.c, so we only need to care about a couple of codepaths.
>
> Folks, could you check if the following on top of linux-next fixes the
> problem?
Thanks Al, and Guenter, and Sabrina for helping resolve this; it surely would
have taken me much longer alone.
Al, do you mind if I fold your patch below into the existing patches?
> diff --git a/fs/namei.c b/fs/namei.c
> index 323957f..cda89c3 100644
> --- a/fs/namei.c
> +++ b/fs/namei.c
> @@ -2056,13 +2056,22 @@ static int do_path_lookup(int dfd, const char *name,
> /* does lookup, returns the object with parent locked */
> struct dentry *kern_path_locked(const char *name, struct path *path)
> {
> + struct filename *filename = getname_kernel(name);
> struct nameidata nd;
> struct dentry *d;
> - int err = do_path_lookup(AT_FDCWD, name, LOOKUP_PARENT, &nd);
> - if (err)
> + int err;
> +
> + if (IS_ERR(filename))
> + return ERR_CAST(filename);
> +
> + err = filename_lookup(AT_FDCWD, filename, LOOKUP_PARENT, &nd);
> + if (err) {
> + putname(filename);
> return ERR_PTR(err);
> + }
> if (nd.last_type != LAST_NORM) {
> path_put(&nd.path);
> + putname(filename);
> return ERR_PTR(-EINVAL);
> }
> mutex_lock_nested(&nd.path.dentry->d_inode->i_mutex, I_MUTEX_PARENT);
> @@ -2070,9 +2079,11 @@ struct dentry *kern_path_locked(const char *name,
> struct path *path) if (IS_ERR(d)) {
> mutex_unlock(&nd.path.dentry->d_inode->i_mutex);
> path_put(&nd.path);
> + putname(filename);
> return d;
> }
> *path = nd.path;
> + putname(filename);
> return d;
> }
>
> @@ -3314,7 +3325,7 @@ struct file *do_file_open_root(struct dentry *dentry,
> struct vfsmount *mnt, return file;
> }
>
> -struct dentry *kern_path_create(int dfd, const char *pathname,
> +static struct dentry *filename_create(int dfd, struct filename *name,
> struct path *path, unsigned int lookup_flags)
> {
> struct dentry *dentry = ERR_PTR(-EEXIST);
> @@ -3329,7 +3340,7 @@ struct dentry *kern_path_create(int dfd, const char
> *pathname, */
> lookup_flags &= LOOKUP_REVAL;
>
> - error = do_path_lookup(dfd, pathname, LOOKUP_PARENT|lookup_flags, &nd);
> + error = filename_lookup(dfd, name, LOOKUP_PARENT|lookup_flags, &nd);
> if (error)
> return ERR_PTR(error);
>
> @@ -3383,6 +3394,19 @@ out:
> path_put(&nd.path);
> return dentry;
> }
> +
> +struct dentry *kern_path_create(int dfd, const char *pathname,
> + struct path *path, unsigned int lookup_flags)
> +{
> + struct filename *filename = getname_kernel(pathname);
> + struct dentry *res = ERR_CAST(filename);
> +
> + if (!IS_ERR(filename)) {
> + res = filename_create(dfd, filename, path, lookup_flags);
> + putname(filename);
> + }
> + return res;
> +}
> EXPORT_SYMBOL(kern_path_create);
>
> void done_path_create(struct path *path, struct dentry *dentry)
> @@ -3401,7 +3425,7 @@ struct dentry *user_path_create(int dfd, const char
> __user *pathname, struct dentry *res;
> if (IS_ERR(tmp))
> return ERR_CAST(tmp);
> - res = kern_path_create(dfd, tmp->name, path, lookup_flags);
> + res = filename_create(dfd, tmp, path, lookup_flags);
> putname(tmp);
> return res;
> }
--
paul moore
security @ redhat
^ permalink raw reply
* Re: Detecting loading of libraries
From: hsultan @ 2015-01-22 1:30 UTC (permalink / raw)
To: linux-audit
In-Reply-To: <5dc8468401e6007eaad18a0b7d782927@thefroid.net>
Ok, I now see the file descriptor in a context record for mmap
*sometimes*
1300 - audit(1421886600.839:25623): arch=c000003e syscall=9 success=yes
exit=140564293636096 a0=0 a1=1000 a2=3 a3=22 items=0 ppid=23505
pid=24942 auid=1000 uid=0 gid=0 euid=0 suid=0 fsuid=0 egid=0 sgid=0
fsgid=0 tty=pts5 ses=2 comm="sudo" exe="/usr/bin/sudo" key=(null)
1300 - audit(1421886600.839:25624): arch=c000003e syscall=9 success=yes
exit=140564293636096 a0=0 a1=1000 a2=3 a3=22 items=0 ppid=23505
pid=24942 auid=1000 uid=0 gid=0 euid=0 suid=0 fsuid=0 egid=0 sgid=0
fsgid=0 tty=pts5 ses=2 comm="sudo" exe="/usr/bin/sudo" key=(null)
1300 - audit(1421886600.839:25625): arch=c000003e syscall=9 success=yes
exit=140564239544320 a0=0 a1=2020f0 a2=5 a3=802 items=0 ppid=23505
pid=24942 auid=1000 uid=0 gid=0 euid=0 suid=0 fsuid=0 egid=0 sgid=0
fsgid=0 tty=pts5 ses=2 comm="sudo" exe="/usr/bin/sudo" key=(null)
1323 - audit(1421886600.839:25625): fd=10 flags=0x802
1300 - audit(1421886600.839:25626): arch=c000003e syscall=9 success=yes
exit=140564241645568 a0=7fd7a9b10000 a1=2000 a2=3 a3=812 items=0
ppid=23505 pid=24942 auid=1000 uid=0 gid=0 euid=0 suid=0 fsuid=0 egid=0
sgid=0 fsgid=0 tty=pts5 ses=2 comm="sudo" exe="/usr/bin/sudo" key=(null)
1323 - audit(1421886600.839:25626): fd=10 flags=0x812
1300 - audit(1421886600.839:25627): arch=c000003e syscall=9 success=yes
exit=140564293640192 a0=0 a1=1000 a2=3 a3=22 items=0 ppid=23505
pid=24942 auid=1000 uid=0 gid=0 euid=0 suid=0 fsuid=0 egid=0 sgid=0
fsgid=0 tty=pts5 ses=2 comm="sudo" exe="/usr/bin/sudo" key=(null)
Any idea why the 1323 shows up sometimes and not each time an mmap call
is made ? Is the record generated only on the 1st mmap call for a
library ?
Thanks,
Hassan
On 2015-01-21 16:01, hsultan@thefroid.net wrote:
> Hi,
>
> I'm wondering if there's a good way of detecting the loading of
> libraries by processes (I am specifically NOT talking about the
> uselib
> syscall).
>
> strace shows me apps do open(...)/mmap/mprotect
> I'm currently intercepting mmap calls, however no additional context
> records are given to provide the name of the library, and the file
> descriptor is the 5th parameter, so I can't get that either to match
> it to an open(...)
>
> Is there a way to do this that I'm missing ?
>
> Thanks,
>
> Hassan
>
> --
> Linux-audit mailing list
> Linux-audit@redhat.com
> https://www.redhat.com/mailman/listinfo/linux-audit
^ permalink raw reply
* Detecting loading of libraries
From: hsultan @ 2015-01-22 0:01 UTC (permalink / raw)
To: linux-audit
Hi,
I'm wondering if there's a good way of detecting the loading of
libraries by processes (I am specifically NOT talking about the uselib
syscall).
strace shows me apps do open(...)/mmap/mprotect
I'm currently intercepting mmap calls, however no additional context
records are given to provide the name of the library, and the file
descriptor is the 5th parameter, so I can't get that either to match it
to an open(...)
Is there a way to do this that I'm missing ?
Thanks,
Hassan
^ permalink raw reply
* Re: linux-next: Tree for Jan 20 -- Kernel panic - Unable to mount root fs
From: Paul Walmsley @ 2015-01-21 21:54 UTC (permalink / raw)
To: Al Viro
Cc: Guenter Roeck, Sabrina Dubroca, Paul Moore, Stephen Rothwell,
linux-next, linux-kernel, linux-fsdevel, linux-audit,
Richard Guy Briggs
In-Reply-To: <20150121212833.GS29656@ZenIV.linux.org.uk>
On Wed, 21 Jan 2015, Al Viro wrote:
> Folks, could you check if the following on top of linux-next fixes the problem?
Tested-by: Paul Walmsley <paul@pwsan.com> # Tegra124 Jetson TK1
>
> diff --git a/fs/namei.c b/fs/namei.c
> index 323957f..cda89c3 100644
> --- a/fs/namei.c
> +++ b/fs/namei.c
> @@ -2056,13 +2056,22 @@ static int do_path_lookup(int dfd, const char *name,
> /* does lookup, returns the object with parent locked */
> struct dentry *kern_path_locked(const char *name, struct path *path)
> {
> + struct filename *filename = getname_kernel(name);
> struct nameidata nd;
> struct dentry *d;
> - int err = do_path_lookup(AT_FDCWD, name, LOOKUP_PARENT, &nd);
> - if (err)
> + int err;
> +
> + if (IS_ERR(filename))
> + return ERR_CAST(filename);
> +
> + err = filename_lookup(AT_FDCWD, filename, LOOKUP_PARENT, &nd);
> + if (err) {
> + putname(filename);
> return ERR_PTR(err);
> + }
> if (nd.last_type != LAST_NORM) {
> path_put(&nd.path);
> + putname(filename);
> return ERR_PTR(-EINVAL);
> }
> mutex_lock_nested(&nd.path.dentry->d_inode->i_mutex, I_MUTEX_PARENT);
> @@ -2070,9 +2079,11 @@ struct dentry *kern_path_locked(const char *name, struct path *path)
> if (IS_ERR(d)) {
> mutex_unlock(&nd.path.dentry->d_inode->i_mutex);
> path_put(&nd.path);
> + putname(filename);
> return d;
> }
> *path = nd.path;
> + putname(filename);
> return d;
> }
>
> @@ -3314,7 +3325,7 @@ struct file *do_file_open_root(struct dentry *dentry, struct vfsmount *mnt,
> return file;
> }
>
> -struct dentry *kern_path_create(int dfd, const char *pathname,
> +static struct dentry *filename_create(int dfd, struct filename *name,
> struct path *path, unsigned int lookup_flags)
> {
> struct dentry *dentry = ERR_PTR(-EEXIST);
> @@ -3329,7 +3340,7 @@ struct dentry *kern_path_create(int dfd, const char *pathname,
> */
> lookup_flags &= LOOKUP_REVAL;
>
> - error = do_path_lookup(dfd, pathname, LOOKUP_PARENT|lookup_flags, &nd);
> + error = filename_lookup(dfd, name, LOOKUP_PARENT|lookup_flags, &nd);
> if (error)
> return ERR_PTR(error);
>
> @@ -3383,6 +3394,19 @@ out:
> path_put(&nd.path);
> return dentry;
> }
> +
> +struct dentry *kern_path_create(int dfd, const char *pathname,
> + struct path *path, unsigned int lookup_flags)
> +{
> + struct filename *filename = getname_kernel(pathname);
> + struct dentry *res = ERR_CAST(filename);
> +
> + if (!IS_ERR(filename)) {
> + res = filename_create(dfd, filename, path, lookup_flags);
> + putname(filename);
> + }
> + return res;
> +}
> EXPORT_SYMBOL(kern_path_create);
>
> void done_path_create(struct path *path, struct dentry *dentry)
> @@ -3401,7 +3425,7 @@ struct dentry *user_path_create(int dfd, const char __user *pathname,
> struct dentry *res;
> if (IS_ERR(tmp))
> return ERR_CAST(tmp);
> - res = kern_path_create(dfd, tmp->name, path, lookup_flags);
> + res = filename_create(dfd, tmp, path, lookup_flags);
> putname(tmp);
> return res;
> }
> --
> To unsubscribe from this list: send the line "unsubscribe linux-next" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
- Paul
^ permalink raw reply
* Re: linux-next: Tree for Jan 20 -- Kernel panic - Unable to mount root fs
From: Sabrina Dubroca @ 2015-01-21 21:40 UTC (permalink / raw)
To: Al Viro
Cc: Guenter Roeck, Paul Moore, Stephen Rothwell, linux-next,
linux-kernel, linux-fsdevel, linux-audit, Richard Guy Briggs
In-Reply-To: <20150121212833.GS29656@ZenIV.linux.org.uk>
2015-01-21, 21:28:33 +0000, Al Viro wrote:
> On Wed, Jan 21, 2015 at 01:03:20PM -0800, Guenter Roeck wrote:
> > ok case (putname commented out):
> >
> > user_path_at_empty lookup usr flags 0x0
> > path_lookupat: calling path_init 'usr' flags=40
> > path_init: link_path_walk() returned 0
> > path_lookupat: path_init 'usr' flags=40[50] returned 0
> > walk_component: lookup_fast() returned 1
> > walk_component: lookup_slow() returned 0
> > walk_component: inode= (null), negative=1
> > do_path_lookup(usr, 0x10)
> > path_lookupat: calling path_init 'usr' flags=50
> > path_init: link_path_walk() returned 0
> > path_lookupat: path_init 'usr' flags=50[50] returned 0
> > mkdir[c74012a0,/usr] => 0
> > user_path_at_empty lookup usr flags 0x1
> > path_lookupat: calling path_init 'usr' flags=41
> > path_init: link_path_walk() returned 0
> > path_lookupat: path_init 'usr' flags=41[51] returned 0
> > walk_component: inode=c74004a0, negative=0
> > user_path_at_empty lookup usr flags 0x1
> > path_lookupat: calling path_init 'usr' flags=41
> > path_init: link_path_walk() returned 0
> > path_lookupat: path_init 'usr' flags=41[51] returned 0
> >
> > failing case:
> >
> > path_lookupat: calling path_init 'usr' flags=40
> > path_init: link_path_walk() returned 0
> > path_lookupat: path_init 'usr' flags=40[50] returned 0
> > walk_component: lookup_fast() returned 1
> > walk_component: lookup_slow() returned 0
> > walk_component: inode= (null), negative=1
> > do_path_lookup(usr, 0x10)
> > path_lookupat: calling path_init 'usr' flags=50
> > path_init: link_path_walk() returned 0
> > path_lookupat: path_init 'usr' flags=50[50] returned 0
> > mkdir[c74012a0,/kkk] => 0 <==== SIC!
>
> Cute. 'k' being 0x6b, aka POISON_FREE... OK, the next question is what's
> been freed under us - I don't believe that it's dentry itself...
> Oh, fuck. OK, I see what happens. Look at kern_path_create(); it does
> LOOKUP_PARENT walk, leaving nd->last pointing to the last component of
> the *COPY* of the name it's just created, walked and freed.
>
> OK... Fortunately, struct nameidata is completely opaque outside of fs/namei.c,
> so we only need to care about a couple of codepaths.
>
> Folks, could you check if the following on top of linux-next fixes the problem?
Yes, it works.
--
Sabrina
^ permalink raw reply
* Re: linux-next: Tree for Jan 20 -- Kernel panic - Unable to mount root fs
From: Guenter Roeck @ 2015-01-21 21:38 UTC (permalink / raw)
To: Al Viro
Cc: Sabrina Dubroca, Paul Moore, Stephen Rothwell, linux-next,
linux-kernel, linux-fsdevel, linux-audit, Richard Guy Briggs
In-Reply-To: <20150121212833.GS29656@ZenIV.linux.org.uk>
On Wed, Jan 21, 2015 at 09:28:33PM +0000, Al Viro wrote:
> On Wed, Jan 21, 2015 at 01:03:20PM -0800, Guenter Roeck wrote:
> >
> > failing case:
> >
> > path_lookupat: calling path_init 'usr' flags=40
> > path_init: link_path_walk() returned 0
> > path_lookupat: path_init 'usr' flags=40[50] returned 0
> > walk_component: lookup_fast() returned 1
> > walk_component: lookup_slow() returned 0
> > walk_component: inode= (null), negative=1
> > do_path_lookup(usr, 0x10)
> > path_lookupat: calling path_init 'usr' flags=50
> > path_init: link_path_walk() returned 0
> > path_lookupat: path_init 'usr' flags=50[50] returned 0
> > mkdir[c74012a0,/kkk] => 0 <==== SIC!
>
> Cute. 'k' being 0x6b, aka POISON_FREE... OK, the next question is what's
> been freed under us - I don't believe that it's dentry itself...
> Oh, fuck. OK, I see what happens. Look at kern_path_create(); it does
> LOOKUP_PARENT walk, leaving nd->last pointing to the last component of
> the *COPY* of the name it's just created, walked and freed.
>
Cool.
> OK... Fortunately, struct nameidata is completely opaque outside of fs/namei.c,
> so we only need to care about a couple of codepaths.
>
> Folks, could you check if the following on top of linux-next fixes the problem?
>
> diff --git a/fs/namei.c b/fs/namei.c
> index 323957f..cda89c3 100644
Yes, this patch fixes the problem for me.
Guenter
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox