On Mon, Aug 27, 2018 at 10:03:09PM +0200, Arnd Bergmann wrote:
+#ifdef CONFIG_COMPAT +#define RTC_IRQP_READ32 _IOR('p', 0x0b, compat_ulong_t) +#define RTC_IRQP_SET32 _IOW('p', 0x0c, compat_ulong_t) +#define RTC_EPOCH_READ32 _IOR('p', 0x0d, compat_ulong_t) +#define RTC_EPOCH_SET32 _IOW('p', 0x0e, compat_ulong_t)
+static long rtc_dev_compat_ioctl(struct file *file, unsigned cmd, unsigned long arg) +{
- unsigned long __user *valp;
- compat_ulong_t __user *argp = compat_ptr(arg);
- unsigned long val;
- int ret;
- switch (cmd) {
- /* pointer to 'long' needs translation. */
- case RTC_IRQP_READ32:
- case RTC_EPOCH_READ32: {
valp = compat_alloc_user_space(sizeof(*valp));if (valp == NULL)return -EFAULT;ret = rtc_dev_ioctl(file, (cmd == RTC_IRQP_READ32) ?RTC_IRQP_READ : RTC_EPOCH_READ,(unsigned long)valp);if (ret)return ret;if (get_user(val, valp) || put_user(val, argp))return -EFAULT;
No. With the side of Hell, No. This is bloody ridiculous - take a look at rtc_dev_ioctl(). Seriously. Relevant bits: struct rtc_device *rtc = file->private_data; const struct rtc_class_ops *ops = rtc->ops; struct rtc_time tm; struct rtc_wkalrm alarm; void __user *uarg = (void __user *) arg;
err = mutex_lock_interruptible(&rtc->ops_lock); if (err) return err;
err = put_user(rtc->irq_freq, (unsigned long __user *)uarg);
mutex_unlock(&rtc->ops_lock); return err;
That's RTC_IRQP_READ. IOW, all that song and dance starting with compat_alloc_user_space() is just a way to get to rtc->irq_freq value.
RTC_EPOCH_READ is in the bowels of individual implementations, but it's otherwise very similar; add a method returning the damn thing (e.g. rtc->ops->get_epoch(rtc)) and lift its call into rtc_dev_ioctl(), then this nonsense will go away as well.
return 0;- }
- /* arguments are compatible, command number is not */
- case RTC_IRQP_SET32:
return rtc_dev_ioctl(file, RTC_IRQP_SET, arg);
ITYM cmd = RTC_IRQP_SET; break;
- case RTC_EPOCH_SET32:
... and cmd = RTC_EPOCH_SET; break; here.
return rtc_dev_ioctl(file, RTC_EPOCH_SET, arg);- }
- /*
* all other rtc ioctls are compatible, or only* exist on architectures without compat mode*/- return rtc_dev_ioctl(file, cmd, (unsigned long)argp);
+}
compat_alloc_user_space() is usually papering over bogosities like that; if you are moving something out to individual drivers, it's always a red flag - most of the time it'll turn out to be pointless.