All of lore.kernel.org
 help / color / mirror / Atom feed
From: Breno Leitao <leitao@debian.org>
To: dccp@vger.kernel.org
Subject: Re: [PATCH net-next v6] net: ioctl: Use kernel memory on protocol ioctl callbacks
Date: Thu, 08 Jun 2023 08:43:13 +0000	[thread overview]
Message-ID: <ZIGUofpP4k24qfQs@gmail.com> (raw)
In-Reply-To: <20230606180045.827659-1-leitao@debian.org>

Hello Kuniyuki,
On Wed, Jun 07, 2023 at 10:31:42AM -0700, Kuniyuki Iwashima wrote:
> > +/* This is the most common ioctl prep function, where the result (4 bytes) is
> > + * copied back to userspace if the ioctl() returns successfully. No input is
> > + * copied from userspace as input argument.
> > + */
> > +static int sock_ioctl_out(struct sock *sk, unsigned int cmd, void __user *arg)
> > +{
> > +	int ret, karg = 0;
> > +
> > +	ret = sk->sk_prot->ioctl(sk, cmd, &karg);
> 
> We need READ_ONCE(sk->sk_prot) as IPv4 conversion or ULP chnage could
> occur at the same time.

Thanks for the heads-up. I would like to pick you brain and understand
a bit more about READ_ONCE() and what is the situation that READ_ONCE()
will solve.

Is the situation related to when sock_ioctl_out() start to execute, and
"sk->sk_prot" changes in a different thread? If that is the case, the
arguments (cmd and arg) will be from the "previous" instance.

Also, grepping for "sk->sk_prot->", I see more than a bunch of calls
that do not use READ_ONCE() barrier. Why is this case different?

Thank you

WARNING: multiple messages have this Message-ID (diff)
From: Breno Leitao <leitao@debian.org>
To: Kuniyuki Iwashima <kuniyu@amazon.com>
Cc: alex.aring@gmail.com, andrea.righi@canonical.com,
	asml.silence@gmail.com, ast@kernel.org, axboe@kernel.dk,
	courmisch@gmail.com, davem@davemloft.net, dccp@vger.kernel.org,
	dsahern@kernel.org, edumazet@google.com, gnault@redhat.com,
	hbh25y@gmail.com, joannelkoong@gmail.com, kernelxing@tencent.com,
	kuba@kernel.org, leit@fb.com, linux-kernel@vger.kernel.org,
	linux-sctp@vger.kernel.org, linux-wpan@vger.kernel.org,
	lucien.xin@gmail.com, marcelo.leitner@gmail.com,
	martin.lau@kernel.org, martineau@kernel.org,
	matthieu.baerts@tessares.net, miquel.raynal@bootlin.com,
	mptcp@lists.linux.dev, netdev@vger.kernel.org, pabeni@redhat.com,
	stefan@datenfreihafen.org, willemdebruijn.kernel@gmail.com,
	wojciech.drewek@intel.com
Subject: Re: [PATCH net-next v6] net: ioctl: Use kernel memory on protocol ioctl callbacks
Date: Thu, 8 Jun 2023 01:43:13 -0700	[thread overview]
Message-ID: <ZIGUofpP4k24qfQs@gmail.com> (raw)
In-Reply-To: <20230607173142.86395-1-kuniyu@amazon.com>

Hello Kuniyuki,
On Wed, Jun 07, 2023 at 10:31:42AM -0700, Kuniyuki Iwashima wrote:
> > +/* This is the most common ioctl prep function, where the result (4 bytes) is
> > + * copied back to userspace if the ioctl() returns successfully. No input is
> > + * copied from userspace as input argument.
> > + */
> > +static int sock_ioctl_out(struct sock *sk, unsigned int cmd, void __user *arg)
> > +{
> > +	int ret, karg = 0;
> > +
> > +	ret = sk->sk_prot->ioctl(sk, cmd, &karg);
> 
> We need READ_ONCE(sk->sk_prot) as IPv4 conversion or ULP chnage could
> occur at the same time.

Thanks for the heads-up. I would like to pick you brain and understand
a bit more about READ_ONCE() and what is the situation that READ_ONCE()
will solve.

Is the situation related to when sock_ioctl_out() start to execute, and
"sk->sk_prot" changes in a different thread? If that is the case, the
arguments (cmd and arg) will be from the "previous" instance.

Also, grepping for "sk->sk_prot->", I see more than a bunch of calls
that do not use READ_ONCE() barrier. Why is this case different?

Thank you

  parent reply	other threads:[~2023-06-08  8:43 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-06-06 18:00 [PATCH net-next v6] net: ioctl: Use kernel memory on protocol ioctl callbacks Breno Leitao
2023-06-06 18:00 ` Breno Leitao
2023-06-06 19:09 ` net: ioctl: Use kernel memory on protocol ioctl callbacks: Tests Results MPTCP CI
2023-06-07 12:15 ` [PATCH net-next v6] net: ioctl: Use kernel memory on protocol ioctl callbacks Willem de Bruijn
2023-06-07 12:15   ` Willem de Bruijn
2023-06-07 13:25 ` net: ioctl: Use kernel memory on protocol ioctl callbacks: Tests Results MPTCP CI
2023-06-07 14:45 ` [PATCH net-next v6] net: ioctl: Use kernel memory on protocol ioctl callbacks David Ahern
2023-06-07 14:45   ` David Ahern
2023-06-07 15:57 ` net: ioctl: Use kernel memory on protocol ioctl callbacks: Tests Results MPTCP CI
2023-06-07 17:31 ` [PATCH net-next v6] net: ioctl: Use kernel memory on protocol ioctl callbacks Kuniyuki Iwashima
2023-06-07 17:31   ` Kuniyuki Iwashima
2023-06-08  7:39 ` Ido Schimmel
2023-06-08  7:39   ` Ido Schimmel
2023-06-08  8:43 ` Breno Leitao [this message]
2023-06-08  8:43   ` Breno Leitao
2023-06-08 13:57 ` Paolo Abeni
2023-06-08 13:57   ` Paolo Abeni
2023-06-08 14:35 ` Breno Leitao
2023-06-08 14:35   ` Breno Leitao

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=ZIGUofpP4k24qfQs@gmail.com \
    --to=leitao@debian.org \
    --cc=dccp@vger.kernel.org \
    /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.