netdev - Re: [PATCH v2 2/8] fwctl: Basic ioctl dispatch for the character device

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: <20240801130546.GB2809814@nvidia.com>
Date: Thu, 1 Aug 2024 10:05:46 -0300
From: Jason Gunthorpe <jgg@...dia.com>
To: Jonathan Cameron <Jonathan.Cameron@...wei.com>
Cc: Jonathan Corbet <corbet@....net>, Itay Avraham <itayavr@...dia.com>,
	Jakub Kicinski <kuba@...nel.org>, Leon Romanovsky <leon@...nel.org>,
	linux-doc@...r.kernel.org, linux-rdma@...r.kernel.org,
	netdev@...r.kernel.org, Paolo Abeni <pabeni@...hat.com>,
	Saeed Mahameed <saeedm@...dia.com>,
	Tariq Toukan <tariqt@...dia.com>,
	Andy Gospodarek <andrew.gospodarek@...adcom.com>,
	Aron Silverton <aron.silverton@...cle.com>,
	Dan Williams <dan.j.williams@...el.com>,
	David Ahern <dsahern@...nel.org>,
	Christoph Hellwig <hch@...radead.org>, Jiri Pirko <jiri@...dia.com>,
	Leonid Bloch <lbloch@...dia.com>,
	Leon Romanovsky <leonro@...dia.com>, linux-cxl@...r.kernel.org,
	patches@...ts.linux.dev
Subject: Re: [PATCH v2 2/8] fwctl: Basic ioctl dispatch for the character
 device

On Tue, Jul 30, 2024 at 06:28:08PM +0100, Jonathan Cameron wrote:
> > And the basic userspace pattern is:
> > 
> >   struct fwctl_info info = {.size = sizeof(info), ...);
> >   ioctl(fd, FWCTL_INFO, &info);
> > 
> > This works today and generates the 24 byte command.
> > 
> > Tomorrow the kernel adds a new member:
> > 
> > struct fwctl_info {
> > 	__u32 size;
> > 	__u32 flags;
> > 	__u32 out_device_type;
> > 	__u32 device_data_len;
> > 	__aligned_u64 out_device_data;
> > 	__aligned_u64 new_thing;
> > };
> > 
> > Current builds of the userpace use a 24 byte command. A new kernel
> > will see the 24 bytes and behave as before.
> > 
> > When I recompile the userspace with the updated header it will issue a
> > 32 byte command with no source change.
> > 
> > Old kernel will see a 32 byte command with the trailing bytes it
> > doesn't understand as 0 and keep working.
> > 
> > The new kernel will see the new_thing bytes are zero and behave the
> > same as before.
> > 
> > If then the userspace decides to set new_thing the old kernel will
> > stop working. Userspace can use some 'try and fail' approach to try
> > again with new_thing = 0.
> 
> I'm not keen on try and fail interfaces because they become messy
> if this has potentially be extended multiple times. Rest
> of argument is fair enough. Thanks for the explanation.

I'd say try-and-fail is just the universal option, if there is merit
we can put cap bits and other things to positively indicate increased
kernel capability.

We have quite a deep experiance on this topic now in RDMA, and there
we've been doing both options, depending on the situation.

For instance you might introduce a new API that returns FOO and extend
a prior API to optionally accept FOO as well. A cap flag that the new
API exists is useful [1], but it is not for the prior API. The
userspace can just blindly pass FOO to the prior API, and if it
happened to get a non-zero FOO somehow then the kernel must also
support it..

[1] "try and fail" works well here too you can invoke the IOCTL with a
0 size and you get ENOTTY if the IOCTL is not understood, and another
error code if it is.

Jason