Re: [PATCH] posix-clock: drop code duplication using compat_ptr_ioctl

[Cyrill Gorcunov]

On Mon, Jan 20, 2025 at 11:41:26PM +0100, Thomas Weißschuh wrote:
> > 
> > > +#ifdef CONFIG_COMPAT
> > > +long ptp_compat_ioctl(struct posix_clock_context *pccontext, unsigned int cmd,
> > > +		      unsigned long arg)
> > > +{
> > > +	switch (cmd) {
> > > +	case PTP_ENABLE_PPS:
> > > +	case PTP_ENABLE_PPS2:
> > > +		/* These take in scalar arg, do not convert */
> > > +		break;
> > > +	default:
> > > +		arg = (unsigned long)compat_ptr(arg);
> > 
> > Here^^^

Hi Thomas!

> 
> The key is to only call compat_ptr() on *pointers*.
> Scalars have to be passed through unmodified.
> For ptp_ioctl(), PTP_ENABLE_PPS and PTP_ENABLE_PPS2 take such scalars,
> which is why those two *can not* use compat_ptr().
> compat_ptr_ioctl() however passes all arguments through compat_ptr().

Yeah, and the PTP_ENABLE_PPS/PTP_ENABLE_PPS2 consider `arg` as 0/1 flip-flop
so compat_ptr won't screw it. So I personally would rather stick with a more
simple code (taking into account that ptp is the only real underlied device
so far sitting in code for so long).

> 
> Admittedly it's quite unlikely anybody would pass a value where it would
> make a difference in practice. But if we fix this now, it might as well
> be correct.

Sure, I see your point. Thanks for comments!

	Cyrill