Thread (28 messages) 28 messages, 6 authors, 2019-01-08

RE: [PATCH net 1/2] ptp: check that rsv field is zero in struct ptp_sys_offset_extended

From: "Keller, Jacob E" <jacob.e.keller@intel.com>
Date: 2019-01-08 20:45:57

-----Original Message-----
From: netdev-owner@vger.kernel.org [mailto:netdev-owner@vger.kernel.org] On
Behalf Of Eugene Syromiatnikov
Sent: Tuesday, January 08, 2019 6:38 AM
To: Richard Cochran <richardcochran@gmail.com>
Cc: David Miller <davem@davemloft.net>; mlichvar@redhat.com;
netdev@vger.kernel.org; Keller, Jacob E [off-list ref];
mtosatti@redhat.com
Subject: Re: [PATCH net 1/2] ptp: check that rsv field is zero in struct
ptp_sys_offset_extended

On Mon, Jan 07, 2019 at 09:19:23PM -0800, Richard Cochran wrote:
quoted
On Mon, Jan 07, 2019 at 08:29:38AM -0800, David Miller wrote:
quoted
From: Eugene Syromiatnikov <redacted>
Date: Mon, 7 Jan 2019 16:22:29 +0100
quoted
Otherwise it is impossible to use it for something else, as it will break
userspace that puts garbage there.

The same check should be done in other structures, but the fact that
data in reserved fields is ignored is already part of the kernel ABI.

Signed-off-by: Eugene Syromiatnikov <redacted>
I think the opportunity to enforce this has passed and you will break
userspace by doing this.
Does this seriously mean that the 'rsv' field in

	struct ptp_extts_request {
		unsigned int index;  /* Which channel to configure. */
		unsigned int flags;  /* Bit field for PTP_xxx flags. */
		unsigned int rsv[2]; /* Reserved for future use. */
	};

can never be extended with some semantics?
Yes[*], since there's no check for garbage in both unused flags bits and rsv
values. The same for ptp_perout_request, ptp_sys_offset, ptp_pin_desc in
PTP_PIN_SETFUNC, and to some extent for ptp_sys_offset_precise, ptp_clock_caps,
and ptp_pin_desc in PTP_PIN_GETFUNC (all newly added data has to be
non-zero there). See also [1][2].

It can be worked around by adding new ioctl commands that operate on the
same structures, but also perform proper checks, though.
So if/when we want to use these fields, this is what we'd have to do...

Is it worth doing that sooner rather than later? i.e. so that the new ioctl command is available for userspace to update to?

Thanks,
Jake
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help