All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jakub Kicinski <kuba@kernel.org>
To: David Ahern <dsahern@gmail.com>
Cc: Leon Romanovsky <leon@kernel.org>,
	Arjun Roy <arjunroy.kdev@gmail.com>,
	davem@davemloft.net, netdev@vger.kernel.org, arjunroy@google.com,
	edumazet@google.com, soheil@google.com
Subject: Re: [net-next v2] tcp: Explicitly mark reserved field in tcp_zerocopy_receive args.
Date: Tue, 9 Feb 2021 08:59:09 -0800	[thread overview]
Message-ID: <20210209085909.32d27f0d@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com> (raw)
In-Reply-To: <af35d535-8d58-3cf3-60e3-1764e409308b@gmail.com>

On Mon, 8 Feb 2021 20:20:29 -0700 David Ahern wrote:
> On 2/8/21 7:53 PM, Jakub Kicinski wrote:
> > On Mon, 8 Feb 2021 19:24:05 -0700 David Ahern wrote:  
> >> 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.

Let me try one last time.

There is no check in the kernels that len <= N. User can pass any
length _already_. check_zeroed_user() forces the values beyond the
structure length to be known (0) rather than anything. It can only 
avoid breakages in the future.

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

If it's correct to check some arbitrary part of the buffer is zeroed 
it should be correct to check the entire tail is zeroed.

  parent reply	other threads:[~2021-02-09 17:00 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-02-06 20:36 [net-next v2] tcp: Explicitly mark reserved field in tcp_zerocopy_receive args Arjun Roy
2021-02-06 23:28 ` Jakub Kicinski
2021-02-07  8:26   ` Leon Romanovsky
2021-02-08 18:41     ` Jakub Kicinski
2021-02-09  2:24       ` David Ahern
2021-02-09  2:53         ` Jakub Kicinski
2021-02-09  3:20           ` David Ahern
2021-02-09  6:29             ` Leon Romanovsky
2021-02-09 16:59             ` Jakub Kicinski [this message]
2021-02-09 23:46               ` Arjun Roy
2021-02-10  4:35                 ` David Ahern
2021-02-10 19:23                   ` Arjun Roy
2021-02-09  6:15       ` Leon Romanovsky
2021-02-09 16:59         ` Jakub Kicinski
2021-02-09 19:01           ` Leon Romanovsky
2021-02-07 17:49 ` David Ahern
2021-02-07 17:53   ` David Ahern

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210209085909.32d27f0d@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com \
    --to=kuba@kernel.org \
    --cc=arjunroy.kdev@gmail.com \
    --cc=arjunroy@google.com \
    --cc=davem@davemloft.net \
    --cc=dsahern@gmail.com \
    --cc=edumazet@google.com \
    --cc=leon@kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=soheil@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.