Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <367f52c9-f224-c538-ff74-10ef88dd1f2c@gmx.com>
Date:   Wed, 7 Sep 2022 08:40:28 +0000
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.2.1
Subject: Re: [PATCH] ntfs3: fix junction point resolution
To:     Daniel Pinto <danielpinto52@gmail.com>,
        Konstantin Komarov <almaz.alexandrovich@paragon-software.com>,
        ntfs3@lists.linux.dev, linux-kernel@vger.kernel.org
References: <7839ac04-f93c-a897-70eb-2505d72f936c@gmail.com>
From:   "Artem S. Tashkinov" <aros@gmx.com>
In-Reply-To: <7839ac04-f93c-a897-70eb-2505d72f936c@gmail.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: quoted-printable
Precedence: bulk


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=3D214833
> Signed-off-by: Daniel Pinto <danielpinto52@gmail.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 =3D target_len;
> +	char *link_path_buffer =3D NULL, *link_path;
> +	char *translated =3D NULL;
> +	char *target_start;
> +	int copy_len;
> +
> +	link_path_buffer =3D kmalloc(PATH_MAX, GFP_NOFS);
> +	if (!link_path_buffer) {
> +		err =3D -ENOMEM;
> +		goto out;
> +	}
> +	/* Get link path, relative to mount point */
> +	link_path =3D dentry_path_raw(link_de, link_path_buffer, PATH_MAX);
> +	if (IS_ERR(link_path)) {
> +		ntfs_err(sb, "Error getting link path");
> +		err =3D -EINVAL;
> +		goto out;
> +	}
> +
> +	translated =3D kmalloc(PATH_MAX, GFP_NOFS);
> +	if (!translated) {
> +		err =3D -ENOMEM;
> +		goto out;
> +	}
> +
> +	/* Make translated path a relative path to mount point */
> +	strcpy(translated, "./");
> +	++link_path;	/* Skip leading / */
> +	for (tl_len =3D sizeof("./") - 1; *link_path; ++link_path) {
> +		if (*link_path =3D=3D '/') {
> +			if (PATH_MAX - tl_len < sizeof("../")) {
> +				ntfs_err(sb, "Link path %s has too many components",
> +					 link_path);
> +				err =3D -EINVAL;
> +				goto out;
> +			}
> +			strcpy(translated + tl_len, "../");
> +			tl_len +=3D sizeof("../") - 1;
> +		}
> +	}
> +
> +	/* Skip drive letter */
> +	target_start =3D target;
> +	while (*target_start && *target_start !=3D ':')
> +		++target_start;
> +
> +	if (!*target_start) {
> +		ntfs_err(sb, "Link target (%s) missing drive separator", target);
> +		err =3D -EINVAL;
> +		goto out;
> +	}
> +
> +	/* Skip drive separator and leading /, if exists */
> +	target_start +=3D 1 + (target_start[1] =3D=3D '/');
> +	copy_len =3D target_len - (target_start - target);
> +
> +	if (PATH_MAX - tl_len <=3D copy_len) {
> +		ntfs_err(sb, "Link target %s too large for buffer (%d <=3D %d)",
> +			 target_start, PATH_MAX - tl_len, copy_len);
> +		err =3D -EINVAL;
> +		goto out;
> +	}
> +
> +	/* translated path has a trailing / and target_start does not */
> +	strcpy(translated + tl_len, target_start);
> +	tl_len +=3D copy_len;
> +	if (target_max <=3D tl_len) {
> +		ntfs_err(sb, "Target path %s too large for buffer (%d <=3D %d)",
> +			 translated, target_max, tl_len);
> +		err =3D -EINVAL;
> +		goto out;
> +	}
> +	strcpy(target, translated);
> +	err =3D 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 =3D -EINVAL;
> @@ -1892,6 +1986,11 @@ static noinline int ntfs_readlink_hlp(struct inod=
e *inode, char *buffer,
>
>   	/* Always set last zero. */
>   	buffer[err] =3D 0;
> +
> +	/* If this is a junction, translate the link target. */
> +	if (rp->ReparseTag =3D=3D IO_REPARSE_TAG_MOUNT_POINT)
> +		err =3D 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 =3D ntfs_readlink_hlp(inode, ret, PAGE_SIZE);
> +	err =3D 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.