Re: [Y2038] [PATCH 2/3] xfs: disallow broken ioctls without compat-32-bit-time
On Tue, Jan 07, 2020 at 10:16:14AM -0800, Darrick J. Wong wrote: > Yeah. Fixing that (and maybe adding an ioctl to set the FS UUID online) > were on my list for 5.6 but clearly I have to defer everything until 5.7 > because we've just run out of time. > > Uh... I started looking into unifying the ext4 and xfs defrag ioctl, but > gagged when I realized that the ext4 ioctl also handles the data copy > inside the kernel. I think that's the sort of behavior we should /not/ > allow into the new ioctl, though that also means that the required > changes for ext4/e4defrag will be non-trivial. Well, we should eventually end up with a common defrag tool (e.g. in util-linux). We might as well start of with the xfs_fsr codebase for that or whatever suits us best. > The btrfs defrag ioctl also contains thresholding information and > optional knobs to enable compression, which makes me wonder if we should > focus narrowly on swapext being "swap these extents but only if the > source file hasn't changed" and not necessarily defrag? That sounds like the most useful common API. > ...in which case I wonder, can people (ab)use this interface for atomic > file updates? Create an O_TMPFILE, reflink the source file into the > temp file, make your updates to the tempfile, and then swapext the > donor's file data back into the source file, but only if the source file > hasn't changed? Sure. ___ Y2038 mailing list Y2038@lists.linaro.org https://lists.linaro.org/mailman/listinfo/y2038
Re: [Y2038] [PATCH 2/3] xfs: disallow broken ioctls without compat-32-bit-time
On Thu, Jan 02, 2020 at 10:07:49AM -0800, Darrick J. Wong wrote: > > Sorry I missed that comment earlier. I've had a fresh look now, but > > I think we still need to deprecate XFS_IOC_SWAPEXT and add a > > v5 version of it, since the comparison will fail as soon as the range > > of the inode timestamps is extended beyond 2038, otherwise the > > comparison will always be false, or require comparing the truncated > > time values which would add yet another representation. > > I prefer we replace the old SWAPEXT with a new version to get rid of > struct xfs_bstat. Though a SWAPEXT_V5 probably only needs to contain > the *stat fields that swapext actually needs to check that the file > hasn't been changed, which would be ino/gen/btime/ctime. > > (Maybe I'd add an offset/length too...) And most importantly we need to lift it to the VFS instead of all the crazy fs specific interfaces at the moment. ___ Y2038 mailing list Y2038@lists.linaro.org https://lists.linaro.org/mailman/listinfo/y2038
Re: [Y2038] [PATCH v3 2/2] xfs: quota: move to time64_t interfaces
Looks good, Reviewed-by: Christoph Hellwig ___ Y2038 mailing list Y2038@lists.linaro.org https://lists.linaro.org/mailman/listinfo/y2038
Re: [Y2038] [PATCH 2/3] xfs: disallow broken ioctls without compat-32-bit-time
On Thu, Jan 02, 2020 at 09:34:48PM +0100, Arnd Bergmann wrote: > I tried adding the helper now but ran into a stupid problem: the best > place to put it would be linux/time32.h, but then I have to include > linux/compat.h from there, which in turn pulls in tons of other > headers in any file using linux/time.h. > > I considered making it a macro instead, but that's also really ugly. > > I now think we should just defer this change until after v5.6, once I > have separated linux/time.h from linux/time32.h. > In the meantime I'll resend the other two patches that I know we > need in v5.6 in order to get there, so Darrick can apply them to his > tree. Sounds good. ___ Y2038 mailing list Y2038@lists.linaro.org https://lists.linaro.org/mailman/listinfo/y2038
Re: [Y2038] [PATCH 2/3] xfs: disallow broken ioctls without compat-32-bit-time
On Wed, Dec 18, 2019 at 05:39:29PM +0100, Arnd Bergmann wrote:
> +/* disallow y2038-unsafe ioctls with CONFIG_COMPAT_32BIT_TIME=n */
> +static bool xfs_have_compat_bstat_time32(unsigned int cmd)
> +{
> + if (IS_ENABLED(CONFIG_COMPAT_32BIT_TIME))
> + return true;
> +
> + if (IS_ENABLED(CONFIG_64BIT) && !in_compat_syscall())
> + return true;
> +
> + if (cmd == XFS_IOC_FSBULKSTAT_SINGLE ||
> + cmd == XFS_IOC_FSBULKSTAT ||
> + cmd == XFS_IOC_SWAPEXT)
> + return false;
> +
> + return true;
I think the check for the individual command belongs into the callers,
which laves us with:
static inline bool have_time32(void)
{
return IS_ENABLED(CONFIG_COMPAT_32BIT_TIME) ||
(IS_ENABLED(CONFIG_64BIT) && !in_compat_syscall());
}
and that looks like it should be in a generic helper somewhere.
> STATIC int
> xfs_ioc_fsbulkstat(
> xfs_mount_t *mp,
> @@ -637,6 +655,9 @@ xfs_ioc_fsbulkstat(
> if (!capable(CAP_SYS_ADMIN))
> return -EPERM;
>
> + if (!xfs_have_compat_bstat_time32(cmd))
> + return -EINVAL;
Here we can simply check for cmd != XFS_IOC_FSINUMBERS before the call.
> if (XFS_FORCED_SHUTDOWN(mp))
> return -EIO;
>
> @@ -1815,6 +1836,11 @@ xfs_ioc_swapext(
> struct fd f, tmp;
> int error = 0;
>
> + if (!xfs_have_compat_bstat_time32(XFS_IOC_SWAPEXT)) {
> + error = -EINVAL;
> + goto out;
> + }
And for this one we just have one cmd anyway. But I actually still
disagree with the old_time check for this one entirely, as voiced on
one of the last iterations. For swapext the time stamp really is
only used as a generation counter, so overflows are entirely harmless.
___
Y2038 mailing list
Y2038@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/y2038
Re: [Y2038] [PATCH 1/3] xfs: rename compat_time_t to old_time32_t
On Wed, Dec 18, 2019 at 05:39:28PM +0100, Arnd Bergmann wrote: > The compat_time_t type has been removed everywhere else, > as most users rely on old_time32_t for both native and > compat mode handling of 32-bit time_t. > > Remove the last one in xfs. > > Reviewed-by: Darrick J. Wong > Signed-off-by: Arnd Bergmann Looks good, Reviewed-by: Christoph Hellwig ___ Y2038 mailing list Y2038@lists.linaro.org https://lists.linaro.org/mailman/listinfo/y2038
Re: [Y2038] [PATCH 21/24] compat_ioctl: block: move blkdev_compat_ioctl() into ioctl.c
> +#ifdef CONFIG_COMPAT
> +static int compat_put_ushort(unsigned long arg, unsigned short val)
> +{
> + return put_user(val, (unsigned short __user *)compat_ptr(arg));
> +}
> +
> +static int compat_put_int(unsigned long arg, int val)
> +{
> + return put_user(val, (compat_int_t __user *)compat_ptr(arg));
> +}
> +
> +static int compat_put_uint(unsigned long arg, unsigned int val)
> +{
> + return put_user(val, (compat_uint_t __user *)compat_ptr(arg));
> +}
> +
> +static int compat_put_long(unsigned long arg, long val)
> +{
> + return put_user(val, (compat_long_t __user *)compat_ptr(arg));
> +}
> +
> +static int compat_put_ulong(unsigned long arg, compat_ulong_t val)
> +{
> + return put_user(val, (compat_ulong_t __user *)compat_ptr(arg));
> +}
> +
> +static int compat_put_u64(unsigned long arg, u64 val)
> +{
> + return put_user(val, (compat_u64 __user *)compat_ptr(arg));
> +}
> +#endif
Can we lift these helpers to compat.h instead?
___
Y2038 mailing list
Y2038@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/y2038
Re: [Y2038] [PATCH 20/24] compat_ioctl: move HDIO ioctl handling into drivers/ide
> +static int put_user_long(long val, unsigned long arg)
> +{
> +#ifdef CONFIG_COMPAT
> + if (in_compat_syscall())
> + return put_user(val, (compat_long_t __user *)compat_ptr(arg));
> +#endif
> + return put_user(val, (long __user *)arg);
> +}
We had this
#ifdef CONFIG_COMPAT
if (in_compat_syscall())
...
...
#endif
patter quite frequently. Can we define a in_compat_syscall stub
and make sure compat_ptr and the compat_* types are available available
to clean this up a bit?
> - if (NULL == (void *) arg) {
> + if (NULL == argp) {
if (!argp) {
?
___
Y2038 mailing list
Y2038@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/y2038
Re: [Y2038] [PATCH 15/24] compat_ioctl: scsi: move ioctl handling into drivers
On Thu, Dec 12, 2019 at 01:28:08AM +0100, Paolo Bonzini wrote: > I think it's because the only ioctl for virtio-blk is SG_IO. It makes > sense to lump it in with scsi, but I wouldn't mind getting rid of > CONFIG_VIRTIO_BLK_SCSI altogether. CONFIG_VIRTIO_BLK_SCSI has been broken for about two years, as it never set the QUEUE_FLAG_SCSI_PASSTHROUGH flag after that was introduced. I actually have a patch that I plan to send to remove this support as it was a really idea to start with (speaking as the person who had that idea back in the day). ___ Y2038 mailing list Y2038@lists.linaro.org https://lists.linaro.org/mailman/listinfo/y2038
Re: [Y2038] [PATCH 02/24] compat: scsi: sg: fix v3 compat read/write interface
On Wed, Dec 11, 2019 at 09:42:36PM +0100, Arnd Bergmann wrote:
> In the v5.4 merge window, a cleanup patch from Al Viro conflicted
> with my rework of the compat handling for sg.c read(). Linus Torvalds
> did a correct merge but pointed out that the resulting code is still
> unsatisfactory.
>
> I later noticed that the sg_new_read() function still gets the compat
> mode wrong, when the 'count' argument is large enough to pass a
> compat_sg_io_hdr object, but not a nativ sg_io_hdr.
>
> To address both of these, move the definition of compat_sg_io_hdr
> into a scsi/sg.h to make it visible to sg.c and rewrite the logic
> for reading req_pack_id as well as the size check to a simpler
> version that gets the expected results.
>
> Fixes: c35a5cfb4150 ("scsi: sg: sg_read(): simplify reading ->pack_id of
> userland sg_io_hdr_t")
> Fixes: 98aaaec4a150 ("compat_ioctl: reimplement SG_IO handling")
> Signed-off-by: Arnd Bergmann
> ---
> block/scsi_ioctl.c | 29 +--
> drivers/scsi/sg.c | 125 +
> include/scsi/sg.h | 30 +++
> 3 files changed, 89 insertions(+), 95 deletions(-)
>
> diff --git a/block/scsi_ioctl.c b/block/scsi_ioctl.c
> index 650bade5ea5a..b61dbf4d8443 100644
> --- a/block/scsi_ioctl.c
> +++ b/block/scsi_ioctl.c
> @@ -20,6 +20,7 @@
> #include
> #include
> #include
> +#include
>
> struct blk_cmd_filter {
> unsigned long read_ok[BLK_SCSI_CMD_PER_LONG];
> @@ -550,34 +551,6 @@ static inline int blk_send_start_stop(struct
> request_queue *q,
> return __blk_send_generic(q, bd_disk, GPCMD_START_STOP_UNIT, data);
> }
>
> -#ifdef CONFIG_COMPAT
> -struct compat_sg_io_hdr {
> - compat_int_t interface_id; /* [i] 'S' for SCSI generic (required)
> */
> - compat_int_t dxfer_direction; /* [i] data transfer direction */
> - unsigned char cmd_len; /* [i] SCSI command length ( <= 16
> bytes) */
> - unsigned char mx_sb_len;/* [i] max length to write to sbp */
> - unsigned short iovec_count; /* [i] 0 implies no scatter gather */
> - compat_uint_t dxfer_len;/* [i] byte count of data transfer */
> - compat_uint_t dxferp; /* [i], [*io] points to data transfer
> memory
> - or scatter gather list */
> - compat_uptr_t cmdp; /* [i], [*i] points to command to
> perform */
> - compat_uptr_t sbp; /* [i], [*o] points to sense_buffer
> memory */
> - compat_uint_t timeout; /* [i] MAX_UINT->no timeout (unit:
> millisec) */
> - compat_uint_t flags;/* [i] 0 -> default, see SG_FLAG... */
> - compat_int_t pack_id; /* [i->o] unused internally (normally)
> */
> - compat_uptr_t usr_ptr; /* [i->o] unused internally */
> - unsigned char status; /* [o] scsi status */
> - unsigned char masked_status;/* [o] shifted, masked scsi status */
> - unsigned char msg_status; /* [o] messaging level data (optional)
> */
> - unsigned char sb_len_wr;/* [o] byte count actually written to
> sbp */
> - unsigned short host_status; /* [o] errors from host adapter */
> - unsigned short driver_status; /* [o] errors from software driver */
> - compat_int_t resid; /* [o] dxfer_len - actual_transferred */
> - compat_uint_t duration; /* [o] time taken by cmd (unit:
> millisec) */
> - compat_uint_t info; /* [o] auxiliary information */
> -};
> -#endif
> -
> int put_sg_io_hdr(const struct sg_io_hdr *hdr, void __user *argp)
> {
> #ifdef CONFIG_COMPAT
> diff --git a/drivers/scsi/sg.c b/drivers/scsi/sg.c
> index 160748ad9c0f..985546aac236 100644
> --- a/drivers/scsi/sg.c
> +++ b/drivers/scsi/sg.c
> @@ -198,6 +198,7 @@ static void sg_device_destroy(struct kref *kref);
>
> #define SZ_SG_HEADER sizeof(struct sg_header)
> #define SZ_SG_IO_HDR sizeof(sg_io_hdr_t)
> +#define SZ_COMPAT_SG_IO_HDR sizeof(struct compat_sg_io_hdr)
I'd rather not add more defines like this. The raw sizeof is
much more readable and obvious.
>
> + if (count < SZ_SG_HEADER)
> + goto unknown_id;
> +
> + /* negative reply_len means v3 format, otherwise v1/v2 */
> + if (get_user(reply_len, _hdr->reply_len))
> + return -EFAULT;
> + if (reply_len >= 0)
> + return get_user(*pack_id, _hdr->pack_id);
> +
> + if (in_compat_syscall() && count >= SZ_COMPAT_SG_IO_HDR) {
> + struct compat_sg_io_hdr __user *hp = buf;
> + return get_user(*pack_id, >pack_id);
> + }
> +
> + if (count >= SZ_SG_IO_HDR) {
> + struct sg_io_hdr __user *hp = buf;
> + return get_user(*pack_id, >pack_id);
> + }
> +
> +unknown_id:
> + /* no valid header was passed, so ignore the pack_id */
> + *pack_id = -1;
> + return 0;
> +}
I find the structure here a little confusing, as it doesn't follow
the normal
Re: [Y2038] [RFC 4/5] xfs: extend inode format for 40-bit timestamps
Amir just send another patch dealing with the time stamps. I'd suggest you chime into the discussion in that thread. ___ Y2038 mailing list Y2038@lists.linaro.org https://lists.linaro.org/mailman/listinfo/y2038
Re: [Y2038] [RFC 1/5] xfs: [variant A] avoid time_t in user api
On Tue, Nov 12, 2019 at 01:09:06PM +0100, Arnd Bergmann wrote: > However, as long as two observations are true, a much simpler solution > can be used: > > 1. xfsprogs is the only user space project that has a copy of this header We can't guarantee that. > 2. xfsprogs already has a replacement for all three affected ioctl commands, >based on the xfs_bulkstat structure to pass 64-bit timestamps >regardless of the architecture XFS_IOC_BULKSTAT replaces XFS_IOC_FSBULKSTAT directly, and can replace XFS_IOC_FSBULKSTAT_SINGLE indirectly, so that is easy. Most users actually use the new one now through libfrog, although I found a user of the direct ioctl in the xfs_io tool, which could easily be fixed as well. XFS_IOC_SWAPEXT does not have a direct replacement, but the timestamp is only used to verify that the file did not change vs the previous stat. So not being able to represent > 2038 times is not a real problem anyway. At some point we should probably look into a file system independent defrag ioctl anyway, at which point we can deprecate XFS_IOC_SWAPEXT. > Based on those assumptions, changing xfs_bstime to use __kernel_long_t > instead of time_t in both the kernel and in xfsprogs preserves the current > ABI for any libc definition of time_t and solves the problem of passing > 64-bit timestamps to 32-bit user space. As said above their are not entirely true, but I still think this patch is the right thing to do, if only to get the time_t out of the ABI.. Reviewed-by: Christoph Hellwig ___ Y2038 mailing list Y2038@lists.linaro.org https://lists.linaro.org/mailman/listinfo/y2038
Re: [Y2038] [GIT PULL] asm-generic: syscall table script for arch/sh
> The conversion does not include the old 64-bit sh5 architecture, which > has never shipped and not even compiled in a long time. Maybe it is time to drop the code for it then? ___ Y2038 mailing list Y2038@lists.linaro.org https://lists.linaro.org/mailman/listinfo/y2038
Re: [Y2038] [PATCH 1/8] sh: dreamcast: rtc: push down rtc class ops into driver
On Fri, Dec 07, 2018 at 02:48:17PM +0100, Arnd Bergmann wrote: > The dreamcast RTC support has an extra level of indirection to > provide either the old read_persistent_clock/update_persistent_clock > interface or the rtc-generic device for hctosys/systohc. s/dreamcast/sh/ in the above sentence? Also repeated in the other patches. ___ Y2038 mailing list Y2038@lists.linaro.org https://lists.linaro.org/mailman/listinfo/y2038
Re: [Y2038] [PATCH 0/3] System call table generation support
On Thu, Sep 20, 2018 at 08:48:37PM +, Paul Burton wrote: > > Speaking of nanoMIPS, what is your plan for the syscall ABI there? > > I can see two ways of approaching it: > > > > a) keep all the MIPSisms in the data structures, and just use a subset of > > o32 that drops all the obsolete entry points > > b) start over and stay as close as possible to the generic ABI, using the > > asm-generic versions of both the syscall table and the uapi header > > files instead of the traditional version. > > We've taken option b in our current downstream kernel & that's what I > hope we'll get upstream too. There's no expectation that we'll ever need > to mix pre-nanoMIPS & nanoMIPS ISAs or their associated ABIs across the > kernel/user boundary so it's felt like a great opportunity to clean up & > standardise. > > Getting nanoMIPS/p32 support submitted upstream is on my to-do list, but > there's a bunch of prep work to get in first & of course that to-do list > is forever growing. Hopefully in the next couple of cycles. p32 is just the ABI name for nanoMIPS or yet another MIPS ABI? Either way, І think if there is yet another ABI even on an existing port we should always aim for the asm-generic syscall table indeed. Especially for mips where o32 has a rather awkward ABI only explained by odd decisions more than 20 years ago. ___ Y2038 mailing list Y2038@lists.linaro.org https://lists.linaro.org/mailman/listinfo/y2038
Re: [Y2038] [PATCH] y2038: provide aliases for compat helpers
On Tue, Aug 21, 2018 at 10:33:00PM +0200, Arnd Bergmann wrote: > As part of the system call rework for 64-bit time_t, we are restructuring > the way that compat syscalls deal with 32-bit time_t, reusing the > implementation for 32-bit architectures. Christoph Hellwig suggested > a rename of the associated types and interfaces to avoid the confusing > usage of the 'compat' prefix for 32-bit architectures. > > To prepare for doing that in linux-4.20, this adds a set of macros > that lets us convert subsystems separately to the new names and > avoid some of the nastier merge conflicts. Looks fine to me: Acked-by: Christoph Hellwig ___ Y2038 mailing list Y2038@lists.linaro.org https://lists.linaro.org/mailman/listinfo/y2038
Re: [Y2038] [PATCH] [RFC] y2038: globally rename compat_time to old_time32
On Mon, Jul 16, 2018 at 12:11:53PM +0200, Arnd Bergmann wrote: > I don't think it makes a big difference whether we change it now or later, > but if Christoph feels that it addresses his concern about the compat_ > namespace being reused during the transition, doing it earlier would > enable us to finish the remaining syscalls. Yes, I've been looking at your new series and the reuse of compat_* is really confusing ___ Y2038 mailing list Y2038@lists.linaro.org https://lists.linaro.org/mailman/listinfo/y2038
Re: [Y2038] [PATCH v2 06/17] y2038: Change sys_utimensat() to use __kernel_timespec
On Mon, Jul 16, 2018 at 06:10:52PM +0200, Arnd Bergmann wrote: > When 32-bit architectures get changed to support 64-bit time_t, > utimensat() needs to use the new __kernel_timespec structure as its > argument. > > The older utime(), utimes() and futimesat() system calls don't need a > corresponding change as they are no longer used on C libraries that have > 64-bit time support. > > As we do for the other syscalls that have timespec arguments, we reuse > the 'compat' syscall entry points to implement the traditional four > interfaces, and only leave the new utimensat() as a native handler, > so that the same code gets used on both 32-bit and 64-bit kernels > on each syscall. I wonder about the direction here: wouldn't it be easier to just leave th existing syscall names as-is and introduce a new utimesat64 which uses the new timespec? We can then drop the old legacy utimesat for new architectures added after the cutover. ___ Y2038 mailing list Y2038@lists.linaro.org https://lists.linaro.org/mailman/listinfo/y2038
Re: [Y2038] [PATCH v2 05/17] asm-generic: Remove empty asm/unistd.h
On Mon, Jul 16, 2018 at 06:10:51PM +0200, Arnd Bergmann wrote:
> Nothing is left in asm/unistd.h except for the redirect to
> uapi/asm/unistd.h, so removing the file simply leads to that one being
> used directly. The linux/export.h inclusion is a leftover from commit
> e1b5bb6d1236 ("consolidate cond_syscall and SYSCALL_ALIAS declarations")
> and should not be used anyway.
Looks good,
Reviewed-by: Christoph Hellwig
___
Y2038 mailing list
Y2038@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/y2038
Re: [Y2038] [PATCH v2 04/17] asm-generic: Remove unneeded __ARCH_WANT_SYS_LLSEEK macro
On Mon, Jul 16, 2018 at 06:10:50PM +0200, Arnd Bergmann wrote: > The sys_llseek sytem call is needed on all 32-bit architectures and > none of the 64-bit ones, so we can remove the __ARCH_WANT_SYS_LLSEEK guard > and simplify the include/asm-generic/unistd.h header further. > > Since 32-bit tasks can run either natively or in compat mode on 64-bit > architectures, we have to check for both !CONFIG_64BIT and CONFIG_COMPAT. > > There are a few 64-bit architectures that also reference sys_llseek > in their 64-bit ABI (e.g. sparc), but I verified that those all > select CONFIG_COMPAT, so the #if check is still correct here. It's > a bit odd to include it in the syscall table though, as it's the > same as sys_lseek() on 64-bit, but with strange calling conventions. Looks good, Reviewed-by: Christoph Hellwig ___ Y2038 mailing list Y2038@lists.linaro.org https://lists.linaro.org/mailman/listinfo/y2038
Re: [Y2038] [PATCH v2 02/17] y2038: Remove newstat family from default syscall set
On Mon, Jul 16, 2018 at 06:10:48PM +0200, Arnd Bergmann wrote: > We have four generations of stat() syscalls: > - the oldstat syscalls that are only used on the older architectures > - the newstat family that is used on all 64-bit architectures but > lacked support for large files on 32-bit architectures. > - the stat64 family that is used mostly on 32-bit architectures to > replace newstat > - statx() to replace all of the above, adding 64-bit timestamps among > other things. > > We already compile stat64 only on those architectures that need it, > but newstat is always built, including on those that don't reference > it. This adds a new __ARCH_WANT_NEW_STAT symbol along the lines of > __ARCH_WANT_OLD_STAT and __ARCH_WANT_STAT64 to control compilation of > newstat. All architectures that need it use an explict define, the > others now get a little bit smaller, and future architecture (including > 64-bit targets) won't ever see it. > > Acked-by: Geert Uytterhoeven > Signed-off-by: Arnd Bergmann Do I read this right that you only want to provide statx by default? It is a little different from the traditional stat calls, so I'd like to know this is actually ok from libc folks first. ___ Y2038 mailing list Y2038@lists.linaro.org https://lists.linaro.org/mailman/listinfo/y2038
Re: [Y2038] [PATCH v2 01/17] y2038: compat: Move common compat types to asm-generic/compat.h
On Mon, Jul 16, 2018 at 06:10:47PM +0200, Arnd Bergmann wrote: > While converting compat system call handlers to work on 32-bit > architectures, I found a number of types used in those handlers > that are identical between all architectures. > > Let's move all the identical ones into asm-generic/compat.h to avoid > having to add even more identical definitions of those types. > > For unknown reasons, mips defines __compat_gid32_t, __compat_uid32_t > and compat_caddr_t as signed, while all others have them unsigned. > This seems to be a mistake, but I'm leaving it alone here. The other > types all differ by size or alignment on at least on architecture. > > compat_aio_context_t is currently defined in linux/compat.h but > also needed for compat_sys_io_getevents(), so let's move it into > the same place. > > While we still have not decided whether the 32-bit time handling > will always use the compat syscalls, or in which form, I think this > is a useful cleanup that we can merge regardless. Looks good: Reviewed-by: Christoph Hellwig ___ Y2038 mailing list Y2038@lists.linaro.org https://lists.linaro.org/mailman/listinfo/y2038
Re: [Y2038] [PATCH v2 3/7] riscv: Include asm-generic/compat.h
On Fri, Jul 06, 2018 at 01:42:46PM +0200, Arnd Bergmann wrote: > We can also rename all the compat syscalls that are now shared > with 32-bit, e.g. using sys_waitid_time32() instead of > compat_sys_waitid(), and that would be consistent with the > new _time64() naming that we are introducing for some of them. Yes, please. You'll need to touch the syscall tables anyway to refer to some new name, so it really isn't that much more work. > Completely separating them from the compat code > would add further complexity though, as some of the > system calls take another argument that is different > between 32-bit and 64-bit kernels, in particular > pselect6, ppoll, io_pgetevents, recvmmsg, and waitid. Why would that create further complexity? IFF those calls need compat work other than the time structures you will need additional variants of them anyway. If the only compat handling is the time structures they will stay the same independent of the name. > >Arnd ---end quoted text--- ___ Y2038 mailing list Y2038@lists.linaro.org https://lists.linaro.org/mailman/listinfo/y2038
Re: [Y2038] [PATCH v3 7/7] timex: change syscalls to use struct __kernel_timex
On Fri, Jul 06, 2018 at 10:42:47PM -0700, Deepa Dinamani wrote:
> struct timex is not y2038 safe.
> Switch all the syscall apis to use y2038 safe __kernel_timex.
So you switch existing syscalls to use a different structure.
If this actually happens to be safe it needs a big explanation
in the commit log.
> -#ifdef CONFIG_COMPAT
> -
> COMPAT_SYSCALL_DEFINE2(clock_adjtime, clockid_t, which_clock,
> struct compat_timex __user *, utp)
> {
> @@ -1187,10 +1183,6 @@ COMPAT_SYSCALL_DEFINE2(clock_adjtime, clockid_t,
> which_clock,
> return err;
> }
>
> -#endif
And this unconditionally defines clock_adjtime, but doesn't actually
seem to add callers, which looks rather odd. Same for other bits
in the patch.
___
Y2038 mailing list
Y2038@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/y2038
Re: [Y2038] [PATCH v3 6/7] timex: use __kernel_timex internally
On Fri, Jul 06, 2018 at 10:42:46PM -0700, Deepa Dinamani wrote: > struct timex is not y2038 safe. > Replace all uses of timex with y2038 safe __kernel_timex. > > Note that struct __kernel_timex is an ABI interface definition. If it actually is an ABI interface it should probably have a different name. ___ Y2038 mailing list Y2038@lists.linaro.org https://lists.linaro.org/mailman/listinfo/y2038
Re: [Y2038] [PATCH v3 5/7] time: Add struct __kernel_timex
I don't think this patches makes sense without the next one,
which actually uses the structure.
> +/* CONFIG_64BIT_TIME enables new 64 bit time_t syscalls in the compat path
> + * and 32-bit emulation.
> + */
Wrong comment style, also the 'compat path is the 32 (or 31 in case of
s390) bit emulation, so the comment seems rather confusing.
> +#ifndef CONFIG_64BIT_TIME
> +#define __kernel_timex timex
> +#endif
using #defines for structs has all kinds of ill effects. Why can't
we aways use __kernel_timex for the in-kernel usage?
> +#ifndef __kernel_timex
> +struct __kernel_timex {
> + unsigned int modes; /* mode selector */
> + int :32;/* pad */
Why do we need padding for a purely in-kernel structure?
Also the anonymous member syntax is rather odd and I don't remeber
us using it anywhere else. Why here?
___
Y2038 mailing list
Y2038@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/y2038
Re: [Y2038] [PATCH v3 1/7] arm64: Make basic compat_* types always available
On Fri, Jul 06, 2018 at 10:42:41PM -0700, Deepa Dinamani wrote: > As we repurpose more compat syscalls to be used in non CONFIG_COMPAT > usecases as part of solving y2038, we need to make these basic types > available unconditionally. NAK. Compat code is for foreign ABIs. If this code is going to be used elsewhere it should not be named compat_ and not be in compat.h. ___ Y2038 mailing list Y2038@lists.linaro.org https://lists.linaro.org/mailman/listinfo/y2038
Re: [Y2038] [PATCH v3 3/7] riscv: Delete asm/compat.h
On Fri, Jul 06, 2018 at 10:42:43PM -0700, Deepa Dinamani wrote: > riscv does not enable CONFIG_COMPAT in default configurations: > defconfig, allmodconfig and allnoconfig. > Remove the asm/compat.h as it does not seem to add any value to > the architecture without CONFIG_COMPAT. Looks good, Reviewed-by: Christoph Hellwig > > Now that time compat syscalls are being reused in non CONFIG_COMPAT > modes, asm-generic/compat.h provides definitions for riscv 32 bit > mode. > > Alternative would be to make compat_time.h to be conditional on > CONFIG_COMPAT_32BIT_TIME. But, since riscv does not does not need > asm/compat.h, delete it instead. Except that all this isn't relavant for this patch (and a horribly bad idea, but Í'll reply to it separately). ___ Y2038 mailing list Y2038@lists.linaro.org https://lists.linaro.org/mailman/listinfo/y2038
Re: [Y2038] [PATCH v2 3/7] riscv: Include asm-generic/compat.h
On Thu, Jul 05, 2018 at 02:36:00PM -0700, Deepa Dinamani wrote: > defconfig, allmodconfig and nomodconfig. > And hence does not inlude definitions for compat data types. > > Now that time syscalls are being reused in non CONFIG_COMPAT > modes, include asm-generic definitions for riscv. > > Alternative would be to make compat_time.h to be conditional on > CONFIG_COMPAT_32BIT_TIME. But, since riscv is already has an > asm/compat.h include the generic version instead. Two comments here: First I think the current riscv compat.h is completely bogus. As you mentioned riscv does not actually have a compat mode, so having a compat.h makes no sensse at all, and the COMPAT_UTS_MACHINE override which is the only thing implemented is included in that statement. Second I think abusing compat.h for old syscall compatibility of any form is a really bad idea. I think you need to split that part out, and preferably not using compat in the name, but something like old-time.h or time32.h for the name. ___ Y2038 mailing list Y2038@lists.linaro.org https://lists.linaro.org/mailman/listinfo/y2038
Re: [Y2038] [PATCH 31/31] aio: implement io_pgetevents
On Wed, Jan 10, 2018 at 04:48:05PM +0100, Arnd Bergmann wrote: > Ok, fair enough. Given that the old version gets obsoleted by this > one, it shouldn't get much uglier than it already is here when you > start out with a regular timespec. We just need to be aware of possible > clashes when we get the patches merged at the same time. I still hope to get this into 4.15. If it gets delayed and we have a coherent 64-bit time_t abi around already I can reconsider the decision. ___ Y2038 mailing list Y2038@lists.linaro.org https://lists.linaro.org/mailman/listinfo/y2038
Re: [Y2038] [PATCH 31/31] aio: implement io_pgetevents
On Wed, Jan 10, 2018 at 12:03:24PM +0100, Arnd Bergmann wrote:
> I'd suggest passing a variant of timespec with two 64-bit members.
> Deepa has posted patches for this structure in the past and was planning
> to do a new version (with minor changes from review) soon, but we
> can just well use it in your patch if that gets merged first.
>
> If we merge io_pgetevents quickly (before the bulk of the y2038 syscall
> conversion), I'd say we should use
>
> struct __kernel_timespec64 {
> __s64 tv_sec;
> __s64 tv_nsec;
> };
>
> The tv_nsec type is unfortunately much trickier than it should be:
> C99 requires it to be 'long', so user space needs to define the 64-bit
> 'struct timespec' with internal padding in the right places depending
> on endianess, and the kernel has to be careful about either zeroing
> the upper half or checking it for being zeroed by user space depending
> on whether we come from a 32-bit or 64-bit task, but I'm fairly sure
> we have that part worked out by now.
Eww. Being the ginea pig is never good, and in this the fact that
the aio syscalls aren't in glibc is just going to make things worse
in chances of diverging from the future 'standard'.
I'm tempted to say I'd rather deal with the new 64-bit time_t version
later, especially as that should only affect 32-bit ports.
___
Y2038 mailing list
Y2038@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/y2038
Re: [Y2038] [PATCH 31/31] aio: implement io_pgetevents
On Tue, Jan 09, 2018 at 11:16:16PM +0100, Arnd Bergmann wrote: > Hmm, these two new syscall entry points turn into four when we add in > support for 64-bit time_t, as we'd have to support all combinations of 32/64 > bit aio_context_t and time_t. At least they'll also replace plain old io_getevents :) > Would it be better to start this interface out by defining it using a 64-bit > timeout structure? The downside would be that the user space syscall > wrappers have to start out with a conversion, if we don't do it, then > the opposite conversion would have to get added later. Which structure do you want? In the end applications using libaio or even the syscalls directly (like seastar) are a special bread, so they could probably just deal with whatever structure we want to pass. ___ Y2038 mailing list Y2038@lists.linaro.org https://lists.linaro.org/mailman/listinfo/y2038
