linux-kernel - Re: [Y2038] [PATCH 0/3] introduce new evdev interface type

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: <3471947.HA9TBa0js5@wuerfel>
Date:	Thu, 03 Dec 2015 13:54:47 +0100
From:	Arnd Bergmann <arnd@...db.de>
To:	y2038@...ts.linaro.org
Cc:	Pingbo Wen <pingbo.wen@...aro.org>, linux-api@...r.kernel.org,
	Dmitry Torokhov <dmitry.torokhov@...il.com>,
	linux-kernel@...r.kernel.org, aksgarg1989@...il.com,
	linux-input@...r.kernel.org
Subject: Re: [Y2038] [PATCH 0/3] introduce new evdev interface type

On Thursday 03 December 2015 20:49:06 Pingbo Wen wrote:
> > 在 2015年12月1日，18:47，Arnd Bergmann <arnd@...db.de> 写道：
> > On Tuesday 01 December 2015 16:34:00 Pingbo Wen wrote:
> >> We can force kernel using monotonic time in EV_IF_LEGACY interface, and
> >> making input_event independent from time_t(after evdev has converted to
> >> input_value, it’s easy to do that), but that also imply userspace
> >> must change their code to fit this change. If changing userspace code is
> >> a mandatory option, why not to force them do a complete conversion?
> > 
> > Most user space programs won't care, as they don't even look at the tv_sec
> > portion, and the goal is to avoid having to change them.
> > 
> > There is still an open question to how exactly we want to get user space
> > to change.
> > 
> > We could do some compile-time trick by having a union in struct input_event
> > and mark the existing timeval part as deprecated, so we flag any use of the
> > 32-bit tv_sec member, such as:
> > 
> > struct input_event {
> > #if !defined(__KERNEL__) && __TIME_T_BITS == __BITS_PER_LONG
> >        struct timeval time;
> 
> > #else
> > 	struct {
> > 		union {
> > 			__u32 tv_sec __attribute__((deprecated));
> > 			__u32 tv_sec_monotonic;
> > 		};
> > 		__s32 tv_usec;
> > 	} time;
> > #endif
> >        __u16 type;
> >        __u16 code;
> >        __s32 value;
> > };
> 
> I have one question here, if userspace use this structure, all helper functions
> of timeval will not work. And userspace need to write extra helper function for
> this fake timeval. This just create an another urgly time structure.

Correct, this is a useful side-effect of the change: any user space access to
the event->time member that assumes it's a timeval will cause a compile-time
warning or error (depending on the access), which helps us identify the
broken code and fix it to use monotonic times as well as access the right
struct members.

> And this method also forces most of old binaries to compile with new libc, adjust
> their codes with new fake time structure.
> 
> Besides, I get an idea to combine your structure with input_composite_event:
> 
> union {
> 	struct {
> 		__s32 tv_usec;
> 		__s32 tv_sec;
> 	};
> 	__s64 time;
> } time;
> 
> I prefer to use a single s64 timestamp, if our goal is to remove timeval from kernel.

We can't really remove this instance of timeval anyway, so adding an __s64 member
here is not all that helpful. We should use __s64 nanoseconds for new interfaces
like this, but I see no reason to change the one we have here.

	Arnd
--
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/