linux-kernel - Re: [PATCH -V14 05/11] vfs: Support null pathname in readlink

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]

Date:	Thu, 8 Jul 2010 01:27:06 +1000
From:	Nick Piggin <npiggin@...e.de>
To:	"Aneesh Kumar K.V" <aneesh.kumar@...ux.vnet.ibm.com>
Cc:	hch@...radead.org, viro@...iv.linux.org.uk, adilger@....com,
	corbet@....net, serue@...ibm.com, neilb@...e.de,
	hooanon05@...oo.co.jp, bfields@...ldses.org,
	linux-fsdevel@...r.kernel.org, sfrench@...ibm.com,
	philippe.deniel@....FR, linux-kernel@...r.kernel.org
Subject: Re: [PATCH -V14 05/11] vfs: Support null pathname in readlink

On Tue, Jun 15, 2010 at 10:42:55PM +0530, Aneesh Kumar K.V wrote:
> From: NeilBrown <neilb@...e.de>
> 
> This enables to use readlink to get the link target name
> from a file descriptor point to the link. This can be used
> with open_by_handle syscall that returns a file descriptor for a link.
> We can then use this file descriptor to get the target name.
> 
> Signed-off-by: NeilBrown <neilb@...e.de>
> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@...ux.vnet.ibm.com>
> ---
>  fs/stat.c |   30 ++++++++++++++++++++++--------
>  1 files changed, 22 insertions(+), 8 deletions(-)
> 
> diff --git a/fs/stat.c b/fs/stat.c
> index c4ecd52..49b95a7 100644
> --- a/fs/stat.c
> +++ b/fs/stat.c
> @@ -284,26 +284,40 @@ SYSCALL_DEFINE2(newfstat, unsigned int, fd, struct stat __user *, statbuf)
>  SYSCALL_DEFINE4(readlinkat, int, dfd, const char __user *, pathname,
>  		char __user *, buf, int, bufsiz)
>  {
> -	struct path path;
> -	int error;
> +	int error = 0;
> +	struct path path, *pp;
> +	struct file *file = NULL;
>  
>  	if (bufsiz <= 0)
>  		return -EINVAL;
>  
> -	error = user_path_at(dfd, pathname, 0, &path);
> +	if (pathname == NULL && dfd != AT_FDCWD) {
> +		file = fget(dfd);
> +
> +		if (file)
> +			pp = &file->f_path;
> +		else
> +			error = -EBADF;
> +	} else {
> +		error = user_path_at(dfd, pathname, 0, &path);
> +		pp = &path;
> +	}

This (and all the others) is really ugly overloading of syscall
arguments IMO, and the changelog is seriously lacking for such changes.

This also changes the the syscall API of existing calls; from reading
the path at NULL, to switching to a completely different syscall.
Perhaps you're assuming nobody relies on SIGSEGV / mmapped NULL address
there, but even then you surely need to document the changed semantics
somewhere (and document the new syscall semantics properly).
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/