From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: netdev@vger.kernel.org
Cc: linux-kernel@vger.kernel.org, davem@davemloft.net,
kuba@kernel.org, Miklos Szeredi <mszeredi@redhat.com>,
stable@vger.kernel.org,
Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Subject: [PATCH net] af_unix: fix garbage collect vs. MSG_PEEK
Date: Mon, 26 Jul 2021 17:36:21 +0200 [thread overview]
Message-ID: <20210726153621.2658658-1-gregkh@linuxfoundation.org> (raw)
From: Miklos Szeredi <mszeredi@redhat.com>
Gc assumes that in-flight sockets that don't have an external ref can't
gain one while unix_gc_lock is held. That is true because
unix_notinflight() will be called before detaching fds, which takes
unix_gc_lock.
Only MSG_PEEK was somehow overlooked. That one also clones the fds, also
keeping them in the skb. But through MSG_PEEK an external reference can
definitely be gained without ever touching unix_gc_lock.
Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
Cc: <stable@vger.kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
net/unix/af_unix.c | 16 ++++++++++++++--
1 file changed, 14 insertions(+), 2 deletions(-)
Note, this is a resend of this old submission that somehow fell through
the cracks:
https://lore.kernel.org/netdev/CAOssrKcfncAYsQWkfLGFgoOxAQJVT2hYVWdBA6Cw7hhO8RJ_wQ@mail.gmail.com/
and was never submitted "properly" and this issue never seemed to get
resolved properly.
I've cleaned it up and made the change much smaller and localized to
only one file. I kept Miklos's authorship as he did the hard work on
this, I just removed lines and fixed a formatting issue :)
diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c
index 23c92ad15c61..cdea997aa5bf 100644
--- a/net/unix/af_unix.c
+++ b/net/unix/af_unix.c
@@ -1526,6 +1526,18 @@ static int unix_getname(struct socket *sock, struct sockaddr *uaddr, int peer)
return err;
}
+static void unix_peek_fds(struct scm_cookie *scm, struct sk_buff *skb)
+{
+ scm->fp = scm_fp_dup(UNIXCB(skb).fp);
+
+ /* During garbage collection it is assumed that in-flight sockets don't
+ * get a new external reference. So we need to wait until current run
+ * finishes.
+ */
+ spin_lock(&unix_gc_lock);
+ spin_unlock(&unix_gc_lock);
+}
+
static int unix_scm_to_skb(struct scm_cookie *scm, struct sk_buff *skb, bool send_fds)
{
int err = 0;
@@ -2175,7 +2187,7 @@ static int unix_dgram_recvmsg(struct socket *sock, struct msghdr *msg,
sk_peek_offset_fwd(sk, size);
if (UNIXCB(skb).fp)
- scm.fp = scm_fp_dup(UNIXCB(skb).fp);
+ unix_peek_fds(&scm, skb);
}
err = (flags & MSG_TRUNC) ? skb->len - skip : size;
@@ -2418,7 +2430,7 @@ static int unix_stream_read_generic(struct unix_stream_read_state *state,
/* It is questionable, see note in unix_dgram_recvmsg.
*/
if (UNIXCB(skb).fp)
- scm.fp = scm_fp_dup(UNIXCB(skb).fp);
+ unix_peek_fds(&scm, skb);
sk_peek_offset_fwd(sk, chunk);
--
2.32.0
next reply other threads:[~2021-07-26 15:38 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-07-26 15:36 Greg Kroah-Hartman [this message]
2021-07-26 19:27 ` [PATCH net] af_unix: fix garbage collect vs. MSG_PEEK Kees Cook
2021-07-29 14:29 ` Miklos Szeredi
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=20210726153621.2658658-1-gregkh@linuxfoundation.org \
--to=gregkh@linuxfoundation.org \
--cc=davem@davemloft.net \
--cc=kuba@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mszeredi@redhat.com \
--cc=netdev@vger.kernel.org \
--cc=stable@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.