Re: [RFCv2 1/9] tcp: authopt: Initial support and key management

On 8/11/21 2:12 PM, Dmitry Safonov wrote:

Hi David,

On 8/11/21 6:15 PM, David Ahern wrote:

quoted

On 8/11/21 8:31 AM, Dmitry Safonov wrote:

quoted

On 8/11/21 9:29 AM, Leonard Crestez wrote:

quoted

On 8/10/21 11:41 PM, Dmitry Safonov wrote:

[..]

quoted

quoted

quoted

quoted

I'm pretty sure it's not a good choice to write partly tcp_authopt.
And a user has no way to check what's the correct len on this kernel.
Instead of len = min_t(unsigned int, len, sizeof(info)), it should be
if (len != sizeof(info))
     return -EINVAL;

Purpose is to allow sockopts to grow as md5 has grown.

md5 has not grown. See above.

MD5 uapi has - e.g., 8917a777be3ba and  6b102db50cdde. We want similar
capabilities for growth with this API.

So, you mean adding a new setsockopt when the struct has to be extended?
Like TCP_AUTHOPT_EXT?

uh, no. That was needed because of failures with the original
implementation wrt checking that all unused bits are 0. If checking is
not done from day 1, that field can never be used in the future.

My point here was only that MD5 uapi was extended.

My second point is more relevant to Leonard as a very recent example of
how to build an extendable struct.

quoted

Look at how TCP_ZEROCOPY_RECEIVE has grown over releases as an example
of how to properly handle this.

Exactly.

: switch (len) {
:		case offsetofend(...)
:		case offsetofend(...)

And than also:
:		if (unlikely(len > sizeof(zc))) {
:			err = check_zeroed_user(optval + sizeof(zc),
:						len - sizeof(zc));

Does it sound similar to what I've written in my ABI review?
And what the LWN article has in it.
Please, look again at the patch I replied to.

Thanks,
         Dmitry