All of lore.kernel.org
 help / color / mirror / Atom feed
From: Christoph Hellwig <hch@lst.de>
To: Eric Dumazet <eric.dumazet@gmail.com>
Cc: Christoph Hellwig <hch@lst.de>,
	"David S. Miller" <davem@davemloft.net>,
	Jakub Kicinski <kuba@kernel.org>,
	netdev@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 3/3] net: cleanly handle kernel vs user buffers for ->msg_control
Date: Wed, 13 May 2020 18:09:38 +0200	[thread overview]
Message-ID: <20200513160938.GA22381@lst.de> (raw)
In-Reply-To: <c88897b9-7afb-a6f6-08f1-5aaa36631a25@gmail.com>

On Wed, May 13, 2020 at 08:41:57AM -0700, Eric Dumazet wrote:
> > +	 * recv* side when msg_control_is_user is set, msg_control is the kernel
> > +	 * buffer used for all other cases.
> > +	 */
> > +	union {
> > +		void		*msg_control;
> > +		void __user	*msg_control_user;
> > +	};
> > +	bool		msg_control_is_user : 1;
> 
> Adding a field in this structure seems dangerous.
> 
> Some users of 'struct msghdr '  define their own struct on the stack,
> and are unaware of this new mandatory field.
> 
> This bit contains garbage, crashes are likely to happen ?
> 
> Look at IPV6_2292PKTOPTIONS for example.

I though of that, an that is why the field is structured as-is.  The idea
is that the field only matters if:

 (1) we are in the recvmsg and friends path, and
 (2) msg_control is non-zero

I went through the places that initialize msg_control to find any spot
that would need an annotation.  The IPV6_2292PKTOPTIONS sockopt doesn't
need one as it is using the msghdr in sendmsg-like context.

That being said while I did the audit I'd appreciate another look from
people that know the networking code better than me of course.

  reply	other threads:[~2020-05-13 16:09 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-05-11 11:59 improve msg_control kernel vs user pointer handling Christoph Hellwig
2020-05-11 11:59 ` [PATCH 1/3] net: add a CMSG_USER_DATA macro Christoph Hellwig
2020-05-12  8:28   ` Sergei Shtylyov
2020-05-13  6:03     ` Christoph Hellwig
2020-05-11 11:59 ` [PATCH 2/3] net/scm: cleanup scm_detach_fds Christoph Hellwig
2020-05-13  9:29   ` Ido Schimmel
2020-05-13  9:49     ` Christoph Hellwig
2020-05-13  9:58       ` Ido Schimmel
2020-05-13 10:10         ` Christoph Hellwig
2020-05-13 10:17           ` Christoph Hellwig
2020-05-13 10:31             ` Ido Schimmel
2020-05-11 11:59 ` [PATCH 3/3] net: cleanly handle kernel vs user buffers for ->msg_control Christoph Hellwig
2020-05-13 15:41   ` Eric Dumazet
2020-05-13 16:09     ` Christoph Hellwig [this message]
2020-05-13 16:18       ` Eric Dumazet
2020-05-13 16:58         ` Christoph Hellwig
2020-05-12  0:00 ` improve msg_control kernel vs user pointer handling David Miller

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=20200513160938.GA22381@lst.de \
    --to=hch@lst.de \
    --cc=davem@davemloft.net \
    --cc=eric.dumazet@gmail.com \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@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.