linux-kernel - Re: [PATCH] ntfs3: fix junction point resolution

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <367f52c9-f224-c538-ff74-10ef88dd1f2c@gmx.com>
Date:   Wed, 7 Sep 2022 08:40:28 +0000
From:   "Artem S. Tashkinov" <aros@....com>
To:     Daniel Pinto <danielpinto52@...il.com>,
        Konstantin Komarov <almaz.alexandrovich@...agon-software.com>,
        ntfs3@...ts.linux.dev, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] ntfs3: fix junction point resolution



On 9/7/22 01:45, Daniel Pinto wrote:
> The ntfs3 file system driver does not convert the target path of
> junction points to a proper Linux path. As junction points targets
> are always absolute paths (they start with a drive letter), all
> junctions will result in broken links.
>
> Translate the targets of junction points to relative paths so they
> point to directories inside the mounted volume. Note that Windows
> allows junction points to reference directories in another drive.
> However, as there is no way to know which drive the junctions refer
> to, we assume they always target the same file system they are in.
>
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=214833
> Signed-off-by: Daniel Pinto <danielpinto52@...il.com>
> ---
>   fs/ntfs3/inode.c | 103 ++++++++++++++++++++++++++++++++++++++++++++++-
>   1 file changed, 101 insertions(+), 2 deletions(-)
>
> diff --git a/fs/ntfs3/inode.c b/fs/ntfs3/inode.c
> index 51363d4e8636..f7e8876f56d3 100644
> --- a/fs/ntfs3/inode.c
> +++ b/fs/ntfs3/inode.c
> @@ -1749,7 +1749,101 @@ void ntfs_evict_inode(struct inode *inode)
>   	ni_clear(ntfs_i(inode));
>   }
>
> -static noinline int ntfs_readlink_hlp(struct inode *inode, char *buffer,
> +/*
> + * ntfs_translate_junction
> + *
> + * Translate a Windows junction target to the Linux equivalent.
> + * On junctions, targets are always absolute (they include the drive
> + * letter). We have no way of knowing if the target is for the current
> + * mounted device or not so we just assume it is.
> + */
> +static int ntfs_translate_junction(const struct super_block *sb,
> +				   const struct dentry *link_de, char *target,
> +				   int target_len, int target_max)
> +{
> +	int tl_len, err = target_len;
> +	char *link_path_buffer = NULL, *link_path;
> +	char *translated = NULL;
> +	char *target_start;
> +	int copy_len;
> +
> +	link_path_buffer = kmalloc(PATH_MAX, GFP_NOFS);
> +	if (!link_path_buffer) {
> +		err = -ENOMEM;
> +		goto out;
> +	}
> +	/* Get link path, relative to mount point */
> +	link_path = dentry_path_raw(link_de, link_path_buffer, PATH_MAX);
> +	if (IS_ERR(link_path)) {
> +		ntfs_err(sb, "Error getting link path");
> +		err = -EINVAL;
> +		goto out;
> +	}
> +
> +	translated = kmalloc(PATH_MAX, GFP_NOFS);
> +	if (!translated) {
> +		err = -ENOMEM;
> +		goto out;
> +	}
> +
> +	/* Make translated path a relative path to mount point */
> +	strcpy(translated, "./");
> +	++link_path;	/* Skip leading / */
> +	for (tl_len = sizeof("./") - 1; *link_path; ++link_path) {
> +		if (*link_path == '/') {
> +			if (PATH_MAX - tl_len < sizeof("../")) {
> +				ntfs_err(sb, "Link path %s has too many components",
> +					 link_path);
> +				err = -EINVAL;
> +				goto out;
> +			}
> +			strcpy(translated + tl_len, "../");
> +			tl_len += sizeof("../") - 1;
> +		}
> +	}
> +
> +	/* Skip drive letter */
> +	target_start = target;
> +	while (*target_start && *target_start != ':')
> +		++target_start;
> +
> +	if (!*target_start) {
> +		ntfs_err(sb, "Link target (%s) missing drive separator", target);
> +		err = -EINVAL;
> +		goto out;
> +	}
> +
> +	/* Skip drive separator and leading /, if exists */
> +	target_start += 1 + (target_start[1] == '/');
> +	copy_len = target_len - (target_start - target);
> +
> +	if (PATH_MAX - tl_len <= copy_len) {
> +		ntfs_err(sb, "Link target %s too large for buffer (%d <= %d)",
> +			 target_start, PATH_MAX - tl_len, copy_len);
> +		err = -EINVAL;
> +		goto out;
> +	}
> +
> +	/* translated path has a trailing / and target_start does not */
> +	strcpy(translated + tl_len, target_start);
> +	tl_len += copy_len;
> +	if (target_max <= tl_len) {
> +		ntfs_err(sb, "Target path %s too large for buffer (%d <= %d)",
> +			 translated, target_max, tl_len);
> +		err = -EINVAL;
> +		goto out;
> +	}
> +	strcpy(target, translated);
> +	err = tl_len;
> +
> +out:
> +	kfree(link_path_buffer);
> +	kfree(translated);
> +	return err;
> +}
> +
> +static noinline int ntfs_readlink_hlp(const struct dentry *link_de,
> +				      struct inode *inode, char *buffer,
>   				      int buflen)
>   {
>   	int i, err = -EINVAL;
> @@ -1892,6 +1986,11 @@ static noinline int ntfs_readlink_hlp(struct inode *inode, char *buffer,
>
>   	/* Always set last zero. */
>   	buffer[err] = 0;
> +
> +	/* If this is a junction, translate the link target. */
> +	if (rp->ReparseTag == IO_REPARSE_TAG_MOUNT_POINT)
> +		err = ntfs_translate_junction(sb, link_de, buffer, err, buflen);
> +
>   out:
>   	kfree(to_free);
>   	return err;
> @@ -1910,7 +2009,7 @@ static const char *ntfs_get_link(struct dentry *de, struct inode *inode,
>   	if (!ret)
>   		return ERR_PTR(-ENOMEM);
>
> -	err = ntfs_readlink_hlp(inode, ret, PAGE_SIZE);
> +	err = ntfs_readlink_hlp(de, inode, ret, PAGE_SIZE);
>   	if (err < 0) {
>   		kfree(ret);
>   		return ERR_PTR(err);
>

The patch works here, thanks a ton. Tested-by me.