Re: [net-next v2] tcp: Explicitly mark reserved field in tcp_zerocopy_receive args.

On 2/8/21 7:53 PM, Jakub Kicinski wrote:

On Mon, 8 Feb 2021 19:24:05 -0700 David Ahern wrote:

quoted

On 2/8/21 11:41 AM, Jakub Kicinski wrote:

quoted

On Sun, 7 Feb 2021 10:26:54 +0200 Leon Romanovsky wrote:  

quoted

There is a check that len is not larger than zs and users can't give
large buffer.

I would say that is pretty safe to write "if (zc.reserved)".  

Which check? There's a check which truncates (writes back to user space
len = min(len, sizeof(zc)). Application can still pass garbage beyond
sizeof(zc) and syscall may start failing in the future if sizeof(zc)
changes.

That would be the case for new userspace on old kernel. Extending the
check to the end of the struct would guarantee new userspace can not ask
for something that the running kernel does not understand.

Indeed, so we're agreeing that check_zeroed_user() is needed before
original optlen from user space gets truncated?

I thought so, but maybe not. To think through this ...

If current kernel understands a struct of size N, it can only copy that
amount from user to kernel. Anything beyond is ignored in these
multiplexed uAPIs, and that is where the new userspace on old kernel falls.

Known value checks can only be done up to size N. In this case, the
reserved field is at the end of the known struct size, so checking just
the field is fine. Going beyond the reserved field has implications for
extensions to the API which should be handled when those extensions are
added.

So, in short I think the "if (zc.reserved)" is correct as Leon noted.