On Tue, Oct 22, 2019 at 6:30 AM Al Viro viro@zeniv.linux.org.uk wrote:
On Thu, Oct 17, 2019 at 04:33:09PM +0200, Arnd Bergmann wrote:
However, looking at this again after your comment I found a rather more serious bug in my new RTC_IRQP_SET handling: Any 64-bit machine can now bypass the permission check for RTC_IRQP_SET by calling RTC_IRQP_SET32 instead.
You've lost the check on RTC_EPOCH_SET as well.
Right, originally my plan was to keep the epoch handling local to rtc-vr41xx.c as explained in the patch description. The driver is specific to a particular very obsolete MIPS machine that was apparently only ever used with 32-bit kernels.
I guess it can't hurt to treat it the same as RTC_IRQP_SET32 if you prefer. Folding in this change now and adapting the changelog text:
--- a/drivers/rtc/dev.c +++ b/drivers/rtc/dev.c @@ -402,6 +402,7 @@ static long rtc_dev_ioctl(struct file *file, #ifdef CONFIG_COMPAT #define RTC_IRQP_SET32 _IOW('p', 0x0c, __u32) #define RTC_IRQP_READ32 _IOR('p', 0x0b, __u32) +#define RTC_EPOCH_SET32 _IOW('p', 0x0e, __u32)
static long rtc_dev_compat_ioctl(struct file *file, unsigned int cmd, unsigned long arg) @@ -416,6 +417,10 @@ static long rtc_dev_compat_ioctl(struct file *file, case RTC_IRQP_SET32: /* arg is a plain integer, not pointer */ return rtc_dev_ioctl(file, RTC_IRQP_SET, arg); + + case RTC_EPOCH_SET32: + /* arg is a plain integer, not pointer */ + return rtc_dev_ioctl(file, RTC_EPOCH_SET, arg); }
return rtc_dev_ioctl(file, cmd, (unsigned long)uarg); diff --git a/drivers/rtc/rtc-vr41xx.c b/drivers/rtc/rtc-vr41xx.c index 79f27de545af..c3671043ace7 100644 --- a/drivers/rtc/rtc-vr41xx.c +++ b/drivers/rtc/rtc-vr41xx.c @@ -69,7 +69,6 @@ static void __iomem *rtc2_base;
/* 32-bit compat for ioctls that nobody else uses */ #define RTC_EPOCH_READ32 _IOR('p', 0x0d, __u32) -#define RTC_EPOCH_SET32 _IOW('p', 0x0e, __u32)
static unsigned long epoch = 1970; /* Jan 1 1970 00:00:00 */
@@ -187,7 +186,6 @@ static int vr41xx_rtc_ioctl(struct device *dev, unsigned int cmd, unsigned long #ifdef CONFIG_64BIT case RTC_EPOCH_READ32: return put_user(epoch, (unsigned int __user *)arg); - case RTC_EPOCH_SET32: #endif case RTC_EPOCH_SET: /* Doesn't support before 1900 */
Another potential issue is drivers/input/misc/hp_sdc_rtc.c, provided that the hardware in question might possibly exist on hppa64 boxen - CONFIG_GSC defaults to y and it's not 32bit-only, so that thing is at least selectable on 64bit kernels.
I decided long ago not to care: that code has never compiled after it was originally merged into the kernel in 2005:
static int hp_sdc_rtc_ioctl(struct inode *inode, struct file *file, unsigned int cmd, unsigned long arg) { #if 1 return -EINVAL; #else ... RTC_IRQP_SET, RTC_EPOCH_SET, ... ... #endif }
I don't see any chance that this code is revived. If anyone wanted to make it work, the right approach would be to use the rtc framework and rewrite the code first.
I could send a patch to remove the dead code though if that helps.
Arnd