linux-nfs.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: NeilBrown <neilb@suse.com>
To: Olga Kornievskaia <aglo@umich.edu>
Cc: Trond Myklebust <trond.myklebust@primarydata.com>,
	Anna Schumaker <anna.schumaker@netapp.com>,
	linux-nfs <linux-nfs@vger.kernel.org>,
	Olga Kornievskaia <kolga@netapp.com>
Subject: Re: [PATCH] SUNRPC: fix refcounting problems with auth_gss messages.
Date: Tue, 06 Dec 2016 09:04:15 +1100	[thread overview]
Message-ID: <877f7e58yo.fsf@notabene.neil.brown.name> (raw)
In-Reply-To: <CAN-5tyHrWMj54cvj3PVO+8Yxk+0NK9ey=X7VU6WXKv-mswh-xw@mail.gmail.com>

[-- Attachment #1: Type: text/plain, Size: 2068 bytes --]

On Tue, Dec 06 2016, Olga Kornievskaia wrote:

> On Sun, Dec 4, 2016 at 11:10 PM, NeilBrown <neilb@suse.com> wrote:
>>
>> There are two problems with refcounting of auth_gss messages.
>>
>> First, the reference on the pipe->pipe list (taken by a call
>> to rpc_queue_upcall()) is not counted.  It seems to be
>> assumed that a message in pipe->pipe will always also be in
>> pipe->in_downcall, where it is correctly reference counted.
>>
>> However there is no guaranty of this.  I have a report of a
>> NULL dereferences in rpc_pipe_read() which suggests a msg
>> that has been freed is still on the pipe->pipe list.
>>
>> One way I imagine this might happen is:
>> - message is queued for uid=U and auth->service=S1
>> - rpc.gssd reads this message and starts processing.
>>   This removes the message from pipe->pipe
>> - message is queued for uid=U and auth->service=S2
>> - rpc.gssd replies to the first message. gss_pipe_downcall()
>>   calls __gss_find_upcall(pipe, U, NULL) and it finds the
>>   *second* message, as new messages are placed at the head
>>   of ->in_downcall, and the service type is not checked.
>
> Correct "service" was/is not a part of the protocol between the kernel
> and gssd. So that's why the kernel can't match what's coming from gssd
> to a particular waiting upcall.
>
>> - This second message is removed from ->in_downcall and freed
>>   by gss_release_msg() (even though it is still on pipe->pipe)
>> - rpc.gssd tries to read another message, and dereferences a pointer
>>   to this message that has just been freed.
>
> Agreed. This is a problem.
>
> Doesn't the problem still exist even with this patch because
> gss_add_msg() adds the msg onto the in_downcall() list? So gssd in
> __gss_fin_upcall() can find the 2nd upcall even before the 2nd msg is
> added to the pipe->pipe()?

The use-after-free problem is solved I think.  It doesn't really make
any difference if the down-call arrives before or after
rpc_queue_upcall() is called.  The msg will still not be freed before it
is removed from both lists.

Thanks,
NeilBrown

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

  reply	other threads:[~2016-12-05 22:04 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-12-05  4:10 [PATCH] SUNRPC: fix refcounting problems with auth_gss messages NeilBrown
2016-12-05 17:29 ` Olga Kornievskaia
2016-12-05 22:04   ` NeilBrown [this message]
2016-12-06 18:07     ` Olga Kornievskaia
2016-12-06 22:12       ` NeilBrown
2016-12-07 14:58         ` Olga Kornievskaia

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=877f7e58yo.fsf@notabene.neil.brown.name \
    --to=neilb@suse.com \
    --cc=aglo@umich.edu \
    --cc=anna.schumaker@netapp.com \
    --cc=kolga@netapp.com \
    --cc=linux-nfs@vger.kernel.org \
    --cc=trond.myklebust@primarydata.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).