diff mbox

[review,09/19] mnt: Fail collect_mounts when applied to unmounted mounts

Message ID 1428026183-14879-9-git-send-email-ebiederm@xmission.com (mailing list archive)
State New, archived
Headers show

Commit Message

Eric W. Biederman April 3, 2015, 1:56 a.m. UTC
The only users of collect_mounts are in audit_tree.c

In audit_tree_trees and audit_add_tree rule the path passed into
collect_mounts is generated from kern_path passed an audit_tree
pathname which is guaranteed to be an absolute path.   In those cases
collect_mounts is obviously intended to work on mounted paths and
if a race results in paths that are unmounted when collect_mounts
it is reasonable to fail early.

The paths passed into audit_tag_tree don't have the absolute path
check.  But are used to play with fsnotify and otherwise interact with
the audit_trees, so again operating only on mounted paths appears
reasonable.

Avoid having to worry about what happens when we try and audit
unmounted filesystems by restricting collect_mounts to mounts
that appear in the mount tree.

Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>
---
 fs/namespace.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

Comments

Lukasz Pawelczyk April 3, 2015, 8:55 a.m. UTC | #1
On czw, 2015-04-02 at 20:56 -0500, Eric W. Biederman wrote:
> The only users of collect_mounts are in audit_tree.c
> 
> In audit_tree_trees and audit_add_tree rule the path passed into

I think you meant audit_trim_trees.

Also you missed a _ in audit_add_tree_rule.


> collect_mounts is generated from kern_path passed an audit_tree
> pathname which is guaranteed to be an absolute path.   In those cases
> collect_mounts is obviously intended to work on mounted paths and
> if a race results in paths that are unmounted when collect_mounts
> it is reasonable to fail early.
> 
> The paths passed into audit_tag_tree don't have the absolute path
> check.  But are used to play with fsnotify and otherwise interact with
> the audit_trees, so again operating only on mounted paths appears
> reasonable.
> 
> Avoid having to worry about what happens when we try and audit
> unmounted filesystems by restricting collect_mounts to mounts
> that appear in the mount tree.
> 
> Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>
> ---
>  fs/namespace.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/namespace.c b/fs/namespace.c
> index 2b12b7a9455d..acc5583764dc 100644
> --- a/fs/namespace.c
> +++ b/fs/namespace.c
> @@ -1669,8 +1669,11 @@ struct vfsmount *collect_mounts(struct path *path)
>  {
>  	struct mount *tree;
>  	namespace_lock();
> -	tree = copy_tree(real_mount(path->mnt), path->dentry,
> -			 CL_COPY_ALL | CL_PRIVATE);
> +	if (!check_mnt(real_mount(path->mnt)))
> +		tree = ERR_PTR(-EINVAL);
> +	else
> +		tree = copy_tree(real_mount(path->mnt), path->dentry,
> +				 CL_COPY_ALL | CL_PRIVATE);
>  	namespace_unlock();
>  	if (IS_ERR(tree))
>  		return ERR_CAST(tree);
Eric W. Biederman April 9, 2015, 4:39 p.m. UTC | #2
Lukasz Pawelczyk <l.pawelczyk@samsung.com> writes:

> On czw, 2015-04-02 at 20:56 -0500, Eric W. Biederman wrote:
>> The only users of collect_mounts are in audit_tree.c
>> 
>> In audit_tree_trees and audit_add_tree rule the path passed into
>
> I think you meant audit_trim_trees.
>
> Also you missed a _ in audit_add_tree_rule.

I did thank you.

Eric
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/fs/namespace.c b/fs/namespace.c
index 2b12b7a9455d..acc5583764dc 100644
--- a/fs/namespace.c
+++ b/fs/namespace.c
@@ -1669,8 +1669,11 @@  struct vfsmount *collect_mounts(struct path *path)
 {
 	struct mount *tree;
 	namespace_lock();
-	tree = copy_tree(real_mount(path->mnt), path->dentry,
-			 CL_COPY_ALL | CL_PRIVATE);
+	if (!check_mnt(real_mount(path->mnt)))
+		tree = ERR_PTR(-EINVAL);
+	else
+		tree = copy_tree(real_mount(path->mnt), path->dentry,
+				 CL_COPY_ALL | CL_PRIVATE);
 	namespace_unlock();
 	if (IS_ERR(tree))
 		return ERR_CAST(tree);