Linux-api Archive on lore.kernel.org
 help / color / Atom feed
From: David Laight <David.Laight@ACULAB.COM>
To: 'Christoph Hellwig' <hch@lst.de>,
	"x86@kernel.org" <x86@kernel.org>, "Jan Kara" <jack@suse.com>
Cc: "linux-arm-kernel@lists.infradead.org" 
	<linux-arm-kernel@lists.infradead.org>,
	"linux-fsdevel@vger.kernel.org" <linux-fsdevel@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"linux-api@vger.kernel.org" <linux-api@vger.kernel.org>,
	"linux-arch@vger.kernel.org" <linux-arch@vger.kernel.org>,
	Jan Kara <jack@suse.cz>
Subject: RE: [PATCH 3/3] quota: simplify the quotactl compat handling
Date: Fri, 7 Aug 2020 09:07:06 +0000
Message-ID: <97285a880c434a4ea9072ff1b9c238e7@AcuMS.aculab.com> (raw)
In-Reply-To: <20200731122202.213333-4-hch@lst.de>

From: Christoph Hellwig
> Sent: 31 July 2020 13:22
>
> Fold the misaligned u64 workarounds into the main quotactl flow instead
> of implementing a separate compat syscall handler.
> 
...
> diff --git a/fs/quota/compat.h b/fs/quota/compat.h
> new file mode 100644
> index 00000000000000..ef7d1e12d650b3
> --- /dev/null
> +++ b/fs/quota/compat.h
> @@ -0,0 +1,34 @@
> +// SPDX-License-Identifier: GPL-2.0
> +#include <linux/compat.h>
> +
> +struct compat_if_dqblk {
> +	compat_u64			dqb_bhardlimit;
> +	compat_u64			dqb_bsoftlimit;
> +	compat_u64			dqb_curspace;
> +	compat_u64			dqb_ihardlimit;
> +	compat_u64			dqb_isoftlimit;
> +	compat_u64			dqb_curinodes;
> +	compat_u64			dqb_btime;
> +	compat_u64			dqb_itime;
> +	compat_uint_t			dqb_valid;
> +};
> +
> +struct compat_fs_qfilestat {
> +	compat_u64			dqb_bhardlimit;
> +	compat_u64			qfs_nblks;
> +	compat_uint_t			qfs_nextents;
> +};
> +
> +struct compat_fs_quota_stat {
> +	__s8				qs_version;
> +	__u16				qs_flags;
> +	__s8				qs_pad;
> +	struct compat_fs_qfilestat	qs_uquota;
> +	struct compat_fs_qfilestat	qs_gquota;
> +	compat_uint_t			qs_incoredqs;
> +	compat_int_t			qs_btimelimit;
> +	compat_int_t			qs_itimelimit;
> +	compat_int_t			qs_rtbtimelimit;
> +	__u16				qs_bwarnlimit;
> +	__u16				qs_iwarnlimit;
> +};
> diff --git a/fs/quota/quota.c b/fs/quota/quota.c
> index 5444d3c4d93f37..e1e9d05a14c3e4 100644
> --- a/fs/quota/quota.c
> +++ b/fs/quota/quota.c
> @@ -19,6 +19,7 @@
>  #include <linux/types.h>
>  #include <linux/writeback.h>
>  #include <linux/nospec.h>
> +#include "compat.h"
> 
>  static int check_quotactl_permission(struct super_block *sb, int type, int cmd,
>  				     qid_t id)
> @@ -211,8 +212,18 @@ static int quota_getquota(struct super_block *sb, int type, qid_t id,
>  	if (ret)
>  		return ret;
>  	copy_to_if_dqblk(&idq, &fdq);
> -	if (copy_to_user(addr, &idq, sizeof(idq)))
> -		return -EFAULT;
> +
> +	if (compat_need_64bit_alignment_fixup()) {
> +		struct compat_if_dqblk __user *compat_dqblk = addr;
> +
> +		if (copy_to_user(compat_dqblk, &idq, sizeof(*compat_dqblk)))
> +			return -EFAULT;
> +		if (put_user(idq.dqb_valid, &compat_dqblk->dqb_valid))
> +			return -EFAULT;

Isn't this always copying the same value again?
I don't think Linux has any 64 bit systems with a 32bit compat
layer that have 64bit 'int'.
Since the only difference in the structures is the 'end padding'
isn't it enough to just copy the size of the 'compat' structure
in a compat system call?
It might even be that gcc will optimise the condition away
when the structure sizes match.

The same is true for a lot of the rest of this file.

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)


  reply index

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-07-31 12:21 improve compat handling for the i386 u64 alignment quirk v2 Christoph Hellwig
2020-07-31 12:22 ` [PATCH 1/3] compat: lift compat_s64 and compat_u64 to <asm-generic/compat.h> Christoph Hellwig
2020-07-31 12:22 ` [PATCH 2/3] compat: add a compat_need_64bit_alignment_fixup() helper Christoph Hellwig
2020-07-31 12:22 ` [PATCH 3/3] quota: simplify the quotactl compat handling Christoph Hellwig
2020-08-07  9:07   ` David Laight [this message]
2020-08-07  9:38   ` David Laight
2020-08-06 13:50 ` improve compat handling for the i386 u64 alignment quirk v2 Ingo Molnar
2020-09-17  7:41 improve compat handling for the i386 u64 alignment quirk v3 Christoph Hellwig
2020-09-17  7:41 ` [PATCH 3/3] quota: simplify the quotactl compat handling Christoph Hellwig

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=97285a880c434a4ea9072ff1b9c238e7@AcuMS.aculab.com \
    --to=david.laight@aculab.com \
    --cc=hch@lst.de \
    --cc=jack@suse.com \
    --cc=jack@suse.cz \
    --cc=linux-api@vger.kernel.org \
    --cc=linux-arch@vger.kernel.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=x86@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Linux-api Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-api/0 linux-api/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-api linux-api/ https://lore.kernel.org/linux-api \
		linux-api@vger.kernel.org
	public-inbox-index linux-api

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-api


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git