On Monday 15 June 2015 22:26:51 Bamvor Zhang Jian wrote:
On 15 Jun 03:33 "Arnd Bergmann" arnd@arndb.de wrote：
case PPSETTIME64:+#endif /* PPSETTIME64 */
case PPSETTIME32:if (!IS_ENABLED(CONFIG_64BIT) || is_compat_task()) {if (compat_get_timeval(&par_timeout,compat_ptr(arg)))
return -EFAULT;} else {if (copy_from_user(&par_timeout, argp,sizeof(par_timeout)))return -EFAULT; }The logic that you add here seems wrong to me: The structure format really should not depend on whether you have a compat task or not, but only on whether you use PPSETTIME32 or PPSETTIME64.
I am not sure if this code is consistent with my ideas. The idea is
PP[SG]ETTIME32 show that it is 32 bit(compat on 64bit or naive 32 bit), but we do not know that whether it is y2038. So, I need a way to ensure it's y2038 safe.
The problem for y2038 safety in this driver is not that the 32-bit time_t is insufficient because the tv_sec member here is only used to pass a timeout value that is at most a few seconds rather than an absolute time.
Instead, we need to modify the driver so it can work with new user space that has set time_t to 64-bit and passes an updated structure layout.
In particular, we want both 32-bit and 64-bit tasks to use the same structures. With your current approach, I don't see how a 32-bit task could ever pass a 64-bit time_t value here.
Yes. This is what I'm puzzled. After study your patch series, I don't found any solutions. Maybe I should read your patches again carefully,
I think the trick is to define the structures separately for the use within the driver and for the user space interface. In the uapi header, you must keep using the definition based on 'timeval' because user existing space uses that structure with the PPSETTIME command. Within the driver, you can define two local structures that are fixed-size and then handle both in the ioctl function separately.
Arnd