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: <bb07ee50-a686-420e-abe8-0fce852086a1@t-8ch.de>
Date: Tue, 21 Jan 2025 13:48:23 +0100
From: Thomas Weißschuh <linux@...ssschuh.net>
To: Cyrill Gorcunov <gorcunov@...il.com>
Cc: LKML <linux-kernel@...r.kernel.org>, 
	Anna-Maria Behnsen <anna-maria@...utronix.de>, Frederic Weisbecker <frederic@...nel.org>, 
	Thomas Gleixner <tglx@...utronix.de>
Subject: Re: [PATCH] posix-clock: drop code duplication using compat_ptr_ioctl

Hi Cyrill!

On 2025-01-21 09:48:28+0300, Cyrill Gorcunov wrote:
> 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^^^

> > 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).

It is valid to pass any value to these ioctls, not only booleans.
On s390 the value 0x80000000 aka BIT(31) would interpreted as "true" by
a native 32bit kernel and "false" by a 64bit kernel in compat mode.

It's indeed an edge case.
Personally I prefer the correct solution.

> > 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!

Let's see what the maintainers prefer :-)


Thomas

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ