All of lore.kernel.org
 help / color / mirror / Atom feed
From: Dan Carpenter <dan.carpenter@oracle.com>
To: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>
Cc: Samuel Ortiz <sameo@linux.intel.com>,
	"David S. Miller" <davem@davemloft.net>,
	Jakub Kicinski <kuba@kernel.org>,
	"John W. Linville" <linville@tuxdriver.com>,
	netdev@vger.kernel.org, kernel-janitors@vger.kernel.org
Subject: Re: [PATCH net] nfc: avoid potential race condition
Date: Thu, 23 Sep 2021 15:22:21 +0300	[thread overview]
Message-ID: <20210923122220.GB2083@kadam> (raw)
In-Reply-To: <3760c70c-299c-89bf-5a4a-22e8d564ef92@canonical.com>

On Thu, Sep 23, 2021 at 09:26:51AM +0200, Krzysztof Kozlowski wrote:
> On 23/09/2021 08:50, Dan Carpenter wrote:
> > This from static analysis inspired by CVE-2021-26708 where there was a
> > race condition because it didn't lock_sock(sk) before saving
> > "vsk->transport".  Here it is saving "llcp_sock->local" but the concept
> > is the same that it needs to take the lock first.
> 
> I think the difference between this llcp_sock code and above transport,
> is lack of writer to llcp_sock->local with whom you could race.
> 
> Commits c0cfa2d8a788fcf4 and 6a2c0962105ae8ce causing the
> multi-transport race show nicely assigns to vsk->transport when module
> is unloaded.
> 
> Here however there is no writer to llcp_sock->local, except bind and
> connect and their error paths. The readers which you modify here, have
> to happen after bind/connect. You cannot have getsockopt() or release()
> before bind/connect, can you? Unless you mean here the bind error path,
> where someone calls getsockopt() in the middle of bind()? Is it even
> possible?
> 

I don't know if this is a real issue either.

Racing with bind would be harmless.  The local pointer would be NULL and
it would return harmlessly.  You would have to race with release and
have a third trying to release local devices.  (Again that might be
wild imagination.  It may not be possible).

regards,
dan carpenter


  reply	other threads:[~2021-09-23 12:23 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-09-23  6:50 [PATCH net] nfc: avoid potential race condition Dan Carpenter
2021-09-23  7:26 ` Krzysztof Kozlowski
2021-09-23 12:22   ` Dan Carpenter [this message]
2021-09-24  8:21     ` Krzysztof Kozlowski
2021-09-24 20:14       ` Jakub Kicinski
2021-09-27  7:44         ` Krzysztof Kozlowski
2021-09-27 14:26           ` Jakub Kicinski
2021-09-27 14:58             ` Krzysztof Kozlowski
2021-09-27 15:13               ` Dan Carpenter
2021-09-27 15:27                 ` Krzysztof Kozlowski
2021-09-27 15:38                   ` Dan Carpenter
2021-09-27 14:49           ` Dan Carpenter
2021-09-27 12:14         ` Dan Carpenter

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=20210923122220.GB2083@kadam \
    --to=dan.carpenter@oracle.com \
    --cc=davem@davemloft.net \
    --cc=kernel-janitors@vger.kernel.org \
    --cc=krzysztof.kozlowski@canonical.com \
    --cc=kuba@kernel.org \
    --cc=linville@tuxdriver.com \
    --cc=netdev@vger.kernel.org \
    --cc=sameo@linux.intel.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.