SGVsbG8sCgpJJ20gc2VuZGluZyBhIHNtYWxsIHNldCBvZiAzIHBhdGNoZXMgZm9yIGEgcHJv
YmxlbSwgdGhhdCBJIGhhdmUKcmVwb3J0ZWQgYSBmZXcgd2Vla3MgYWdvLgpycGMubW91bnRk
IGNhbiBiZSBibG9ja2VkIGJ5IGEgYmFkIGNsaWVudCwgdGhhdCBzZW5kcyBsb3RzIG9mClJQ
QyByZXF1ZXN0cywgYnV0IG5ldmVyIHJlYWRzIHRoZSByZXBsaWVzIGZyb20gdGhlIHNvY2tl
dCBlaXRoZXIKaW50ZW50aW9uYWxseSBvciBlLmcuIGNhdXNlZCBieSBhIHdyb25nIGNvbmZp
Z3VyZWQgTVRVLgoKV2hpbGUgbG9va2luZyBmb3IgYSBwb3NzaWJsZSBzb2x1dGlvbiwgSSBm
b3VuZCBhbm90aGVyIHdlYWtuZXNzCmluIHJwYy5tb3VudGQgaWYgaXQgaXMgdXNlZCAibXVs
dGl0aHJlYWRlZCIgKC10IG5uKS4KClRoZSBmaXJzdCB0d28gcGF0Y2hlcyBmaXggdGhhdCB3
ZWFrbmVzcyBpbiB0aGUgY2FzZSBvZiAhSEFWRV9MSUJUSVJQQwphbmQgSEFWRV9MSUJUSVJQ
Qy4KVGhlIHRoaXJkIHBhdGNoIG1vcmUgYSBraW5kIG9mIHN1Z2dlc3Rpb24gaG93IHRoZSBt
YWluIHByb2JsZW0gY291bGQKYmUgZml4ZWQuIEkgZG9uJ3Qga25vdyB3aGV0aGVyIHdlIGNh
biBzZXQgTUFYUkVDIHdpdGhvdXQgY2F1c2luZwpuZXcgdHJvdWJsZXMuIFdoZW4gdGhpcyBw
YXRjaCBpcyB1c2VkLCBhICBmdXJ0aGVyIHBhdGNoIGZvciBsaWJ0aXJwYwphbHNvIHNob3Vs
ZCBiZSB1c2VkLiBZb3UgY2FuIGZpbmQgaXQgaGVyZToKICAgIGh0dHA6Ly9zb3VyY2Vmb3Jn
ZS5uZXQvcC9saWJ0aXJwYy9tYWlsbWFuL2xpYnRpcnBjLWRldmVsLz92aWV3bW9udGg9MjAx
NDA5CgpCZXN0IHJlZ2FyZHMsCkJvZG8K
Hello,
On 11/05/2014 03:21 PM, [email protected] wrote:
> Hello,
>
> I'm sending a small set of 3 patches for a problem, that I have
> reported a few weeks ago.
> rpc.mountd can be blocked by a bad client, that sends lots of
> RPC requests, but never reads the replies from the socket either
> intentionally or e.g. caused by a wrong configured MTU.
>
> While looking for a possible solution, I found another weakness
> in rpc.mountd if it is used "multithreaded" (-t nn).
>
> The first two patches fix that weakness in the case of !HAVE_LIBTIRPC
> and HAVE_LIBTIRPC.
> The third patch more a kind of suggestion how the main problem could
> be fixed. I don't know whether we can set MAXREC without causing
> new troubles. When this patch is used, a further patch for libtirpc
> also should be used. You can find it here:
> http://sourceforge.net/p/libtirpc/mailman/libtirpc-devel/?viewmonth=201409
Over the weekend I took a good hard look at these 3 patches and
the one for libtirpc with the reproducer Bodo supplied me.
(Bodo, thanks all the help getting things set up!).
I agree with the libtirpc patch so its been committed,
but other three made me nervous so I wanted to take a
good hard look at them. (Neil's question, "do we really
what to make these non-block" haunted me ;-) ).
It turns out, at least in my setup, these patches do not
and can not stop the mountd DOS that Bodo's reproducer
creates.
Here is why they do not work:
The fd that the write() is getting hung on, is not be created
by the 3 nfs_svc_create() calls in mountd:main(). Its
being created by the accept() when the tcp connection
is created (via SVC_RECV()), so making those fds coming
out of the nfs_svc_create() non-blocking does nothing.
Here is the hang can't be stop (for now):
Here is the stack:
Stack trace of thread 4150:
#0 0x00000036816f0e90 write (libc.so.6)
#1 0x00007fd53837da6d write_vc (libtirpc.so.1)
#2 0x00007fd53838103f flush_out (libtirpc.so.1)
#3 0x00007fd53837dd81 svc_vc_reply (libtirpc.so.1)
#4 0x00007fd53837b096 svcerr_noprog (libtirpc.so.1)
#5 0x00007fd53837b360 svc_getreq_common (libtirpc.so.1)
#6 0x00000000004086d9 my_svc_getreqset (mountd)
#7 0x0000000000403d80 main (mountd)
#8 0x000000368161ffe0 __libc_start_main (libc.so.6)
#9 0x00000000004040f5 _start (mountd)
The fd that created by the SVC_RECV() is never
given back to the app (in case mountd) to make
it non-blocking. Its used in the error path
where it get hung up in the write(). The
app has no control over that.
Now I definitely see a problem, but these
patches (with the exception of the libtirpc
patch) don't address the problem I am seeing.
So unless I'm not seeing something, I'm not in
favor of taking these 3 patches.
thoughts?
steved.
Thank you for your thorough work.
Bodo
> -----Original Message-----
> From: Steve Dickson [mailto:[email protected]]
> Sent: Wednesday, November 12, 2014 7:57 PM
> To: Str?sser, Bodo; [email protected]; [email protected]
> Cc: [email protected]
> Subject: Re: [nfs-utils] [PATCH 0/3] rpc.mountd: fix some vulnerabilities
>
>
>
> On 11/05/2014 03:21 PM, [email protected] wrote:
> > Hello,
> >
> > I'm sending a small set of 3 patches for a problem, that I have
> > reported a few weeks ago.
> > rpc.mountd can be blocked by a bad client, that sends lots of
> > RPC requests, but never reads the replies from the socket either
> > intentionally or e.g. caused by a wrong configured MTU.
> >
> > While looking for a possible solution, I found another weakness
> > in rpc.mountd if it is used "multithreaded" (-t nn).
> >
> > The first two patches fix that weakness in the case of !HAVE_LIBTIRPC
> > and HAVE_LIBTIRPC.
> > The third patch more a kind of suggestion how the main problem could
> > be fixed. I don't know whether we can set MAXREC without causing
> > new troubles. When this patch is used, a further patch for libtirpc
> > also should be used. You can find it here:
> > http://sourceforge.net/p/libtirpc/mailman/libtirpc-devel/?viewmonth=201409
> After applying all three patches, the DOS does stop... All three committed!
> Nice work! Thank you... very much!!
>
> steved.
>
> >
> > Best regards,
> > Bodo
> > N?????r??y????b?X??ǧv?^?
> ){.n?+????{???"??^n?r???z???h?????&?? ?G???h?(?階?ݢj"???m??????z?ޖ
> ???f???h???~?mml==
> >
On Thu, Nov 06, 2014 at 08:47:41AM +1100, NeilBrown wrote:
> On Wed, 5 Nov 2014 22:25:26 +0100 "Strösser, Bodo"
> <[email protected]> wrote:
>
> > > -----Original Message-----
> > > From: J. Bruce Fields [mailto:[email protected]]
> > > Sent: Wednesday, November 05, 2014 10:06 PM
> > > To: Strösser, Bodo
> > > Cc: [email protected]; [email protected]
> > > Subject: Re: [nfs-utils] [PATCH 0/3] rpc.mountd: fix some vulnerabilities
> > >
> > > On Wed, Nov 05, 2014 at 09:21:37PM +0100, [email protected] wrote:
> > > > Hello,
> > > >
> > > > I'm sending a small set of 3 patches for a problem, that I have
> > > > reported a few weeks ago.
> > > > rpc.mountd can be blocked by a bad client, that sends lots of
> > > > RPC requests, but never reads the replies from the socket either
> > > > intentionally or e.g. caused by a wrong configured MTU.
> > > >
> > > > While looking for a possible solution, I found another weakness
> > > > in rpc.mountd if it is used "multithreaded" (-t nn).
> > > >
> > > > The first two patches fix that weakness in the case of !HAVE_LIBTIRPC
> > > > and HAVE_LIBTIRPC.
> > >
> > > They look fine to me.
> > >
> > > > The third patch more a kind of suggestion how the main problem could
> > > > be fixed.
> > >
> > > Sounds reasonable to me.
> > >
> > > > I don't know whether we can set MAXREC without causing
> > > > new troubles. When this patch is used, a further patch for libtirpc
> > > > also should be used. You can find it here:
> > > > http://sourceforge.net/p/libtirpc/mailman/libtirpc-devel/?viewmonth=201409
> > >
> > > So applying this last patch and then building against an unpatched
> > > libtirpc would expose us to a serious bug? Do we need to do something
> > > to make that less likely to happen?
> >
> > The bug in libtirpc is triggered, if the write() to the socket returns -EAGAIN
> > but one of the many retries in the following 2 seconds succeeds. Then the
> > normal reply is sent with a prefix of (maybe many) bytes from rpc.mountd's
> > memory space before the reply buffer. That means, that some data from memory is
> > sent (authentication data?) or rpc.mountd might get SIGSEGV, I don't know.
> >
> > But I think it won't be easy to trigger this. rpcbind uses the nonblocking mode
> > of libtirpc and no one complains about the bug in libtirpc.
> >
> > Anyway, it of course would be better to use a fixed libtirpc, as I can't see a
> > way to have a work around in rpc.mountd for the libtirpc bug.
> >
>
> I agree that hitting the bug is fairly unlikely. If there is something easy
> we can do to help avoid it, it might be worth it.
>
> Once the fix gets into a libtirpc release, we could put a hard dependency on
> that release number in configure for nfs-utils. Is that too harsh??
Yeah, that's more what I was wondering about.
If building new mountd against an old libtirpc meant suddenly mountd was
crashing all the time then it'd seem worth something like that.
If it's an unusual corner case that people have been exposed to in other
ways for a while anyway, then maybe we just trust to distros to get
their libtirpc's fixed and don't worry so much.
--b.
>
> Steve: you are maintainer for both (?). Would you be able to make a
> libtirpc release soonish with the mentioned fix, then would you be happy for
> future nfs-utils to require at least that version?
On 11/11/2014 06:00 PM, NeilBrown wrote:
>> Here is why they do not work:
>> >
>> > The fd that the write() is getting hung on, is not be created
>> > by the 3 nfs_svc_create() calls in mountd:main(). Its
>> > being created by the accept() when the tcp connection
>> > is created (via SVC_RECV()), so making those fds coming
>> > out of the nfs_svc_create() non-blocking does nothing.
> Just to add to what Bodo replied:
>
> SVC_RECV is a macro which calls .xp_recv, which in the TCP listen
> case is rendezvous_request.
>
> rendezvous_request does the accept:
>
> if ((sock = accept(xprt->xp_fd, (struct sockaddr *)(void *)&addr,
> &len)) < 0) {
>
> and then
> if (cd->maxrec != 0) {
> flags = fcntl(sock, F_GETFL, 0);
> if (flags == -1)
> return (FALSE);
> if (fcntl(sock, F_SETFL, flags | O_NONBLOCK) == -1)
> return (FALSE);
>
> so if cd->maxrec is not zero, O_NONBLOCK get set.
>
> It gets set through an SVC_CONTROL -> xp_control -> svc_vc_rendezvous_control
>
> call, which no-one ever makes, and it initialised to __svc_maxrec, which is
> set by the rpc_control() call that Bodo mentions.
> So providing rpc_control is called before the service is created, it should
> work fine.
>
> I wonder why it didn't work for you...
Because I didn't apply the 3rd patch because it was marked
as experimental. I figured I would get the first two
patches working and deal with it later...
Plus although I did look at this code, I didn't notice the
setting of the O_NONBLOCK on the fd from the accept.
Thanks for pointing it out....
steved.
On 11/05/2014 04:47 PM, NeilBrown wrote:
> Steve: you are maintainer for both (?). Would you be able to make a
> libtirpc release soonish with the mentioned fix, then would you be happy for
> future nfs-utils to require at least that version?
Let me get the libtirpc fix in.. and then take a look at
adding something to the aclocal/libtirpc.m4 requiring
a particular version??
steved.
> -----Original Message-----
> From: Steve Dickson [mailto:[email protected]]
> Sent: Tuesday, November 11, 2014 7:25 PM
> To: Str?sser, Bodo; [email protected]; [email protected]
> Cc: [email protected]
> Subject: Re: [nfs-utils] [PATCH 0/3] rpc.mountd: fix some vulnerabilities
>
> Hello,
>
> On 11/05/2014 03:21 PM, [email protected] wrote:
> > Hello,
> >
> > I'm sending a small set of 3 patches for a problem, that I have
> > reported a few weeks ago.
> > rpc.mountd can be blocked by a bad client, that sends lots of
> > RPC requests, but never reads the replies from the socket either
> > intentionally or e.g. caused by a wrong configured MTU.
> >
> > While looking for a possible solution, I found another weakness
> > in rpc.mountd if it is used "multithreaded" (-t nn).
> >
> > The first two patches fix that weakness in the case of !HAVE_LIBTIRPC
> > and HAVE_LIBTIRPC.
> > The third patch more a kind of suggestion how the main problem could
> > be fixed. I don't know whether we can set MAXREC without causing
> > new troubles. When this patch is used, a further patch for libtirpc
> > also should be used. You can find it here:
> > http://sourceforge.net/p/libtirpc/mailman/libtirpc-devel/?viewmonth=201409
>
> Over the weekend I took a good hard look at these 3 patches and
> the one for libtirpc with the reproducer Bodo supplied me.
> (Bodo, thanks all the help getting things set up!).
>
> I agree with the libtirpc patch so its been committed,
> but other three made me nervous so I wanted to take a
> good hard look at them. (Neil's question, "do we really
> what to make these non-block" haunted me ;-) ).
>
> It turns out, at least in my setup, these patches do not
> and can not stop the mountd DOS that Bodo's reproducer
> creates.
>
> Here is why they do not work:
>
> The fd that the write() is getting hung on, is not be created
> by the 3 nfs_svc_create() calls in mountd:main(). Its
> being created by the accept() when the tcp connection
> is created (via SVC_RECV()), so making those fds coming
> out of the nfs_svc_create() non-blocking does nothing.
Sorry, Steve, unfortunately I have to disagree.
The hung we tried to reproduce on your config yesterday is fixed by
patch 3/3 only. It simply uses
rpc_control(RPC_SVC_CONNMAXREC_SET, &maxrec);
to make libtirpc set O_NONBLOCK on the tcp connections for us. That
_definitely_ works well to avoid the hung, but maybe triggers the problem
in libtirpc which is fixed by the patch you committed.
I for myself was uncertain regarding this patch, but meanwhile Neil
pointed out, that setting MAXREC only affects the receive buffer size.
For receive the used RPC_MAXDATASIZE (= 9000) always will be enough.
The two other patches (1/3 and 2/3) are related to the use of "-t nn"
with rpc.mountd. They change the listening socket (TCP) and the reading
socket (UDP) to nonblocking mode. See the following comment from
source support/nfs/svc_socket.c, which is an old comment that doesn't
come from me:
/* This socket might be shared among multiple processes
* if mountd is run multi-threaded. So it is safest to
* make it non-blocking, else all threads might wake
* one will get the data, and the others will block
* indefinitely.
* In all cases, transaction on this socket are atomic
* (accept for TCP, packet-read and packet-write for UDP)
* so O_NONBLOCK will not confuse unprepared code causing
* it to corrupt messages.
* It generally safest to have O_NONBLOCK when doing an accept
* as if we get a RST after the SYN and before accept runs,
* we can block despite being told there was an acceptable
* connection.
*/
This comment describs precisely, why
"We really do want non-blocking sockets here!"
as NeilBrown said. I don't think, that this was a question (Neil,
please correct me if I'm wrong).
The problem is, that the code switches to nonblocking mode only
if rpc.mountd was built without libtirpc and the "-p <portno>"
option is not used.
If you use the option or rpc.mountd is built with libtirpc, the
listening sockets are blocking. I have tested it simply by using
the socket features of bash and strace'ing rpc.mountd.
When connecting to a multithreaded rpc.mountd, all threads wake up
and call accept(), but of course only one thread gets the connection,
all others sleep on the accept().
So, we talk about switching to nonblocking mode for the listeners
(Patch 1/3 and 2/3) and the TCP connections (Patch 3/3).
Bodo
>
> Here is the hang can't be stop (for now):
>
> Here is the stack:
> Stack trace of thread 4150:
> #0 0x00000036816f0e90 write (libc.so.6)
> #1 0x00007fd53837da6d write_vc (libtirpc.so.1)
> #2 0x00007fd53838103f flush_out (libtirpc.so.1)
> #3 0x00007fd53837dd81 svc_vc_reply (libtirpc.so.1)
> #4 0x00007fd53837b096 svcerr_noprog (libtirpc.so.1)
> #5 0x00007fd53837b360 svc_getreq_common (libtirpc.so.1)
> #6 0x00000000004086d9 my_svc_getreqset (mountd)
> #7 0x0000000000403d80 main (mountd)
> #8 0x000000368161ffe0 __libc_start_main (libc.so.6)
> #9 0x00000000004040f5 _start (mountd)
>
> The fd that created by the SVC_RECV() is never
> given back to the app (in case mountd) to make
> it non-blocking. Its used in the error path
> where it get hung up in the write(). The
> app has no control over that.
>
> Now I definitely see a problem, but these
> patches (with the exception of the libtirpc
> patch) don't address the problem I am seeing.
>
> So unless I'm not seeing something, I'm not in
> favor of taking these 3 patches.
>
> thoughts?
>
> steved.
On 11/05/2014 03:21 PM, [email protected] wrote:
> Hello,
>
> I'm sending a small set of 3 patches for a problem, that I have
> reported a few weeks ago.
> rpc.mountd can be blocked by a bad client, that sends lots of
> RPC requests, but never reads the replies from the socket either
> intentionally or e.g. caused by a wrong configured MTU.
>
> While looking for a possible solution, I found another weakness
> in rpc.mountd if it is used "multithreaded" (-t nn).
>
> The first two patches fix that weakness in the case of !HAVE_LIBTIRPC
> and HAVE_LIBTIRPC.
> The third patch more a kind of suggestion how the main problem could
> be fixed. I don't know whether we can set MAXREC without causing
> new troubles. When this patch is used, a further patch for libtirpc
> also should be used. You can find it here:
> http://sourceforge.net/p/libtirpc/mailman/libtirpc-devel/?viewmonth=201409
After applying all three patches, the DOS does stop... All three committed!
Nice work! Thank you... very much!!
steved.
>
> Best regards,
> Bodo
> N?????r??y????b?X??ǧv?^?){.n?+????{???"??^n?r???z???h?????&?? ?G???h?(?階?ݢj"???m??????z?ޖ???f???h???~?mml==
>
On Tue, 11 Nov 2014 13:24:31 -0500 Steve Dickson <[email protected]> wrote:
> Hello,
>
> On 11/05/2014 03:21 PM, [email protected] wrote:
> > Hello,
> >
> > I'm sending a small set of 3 patches for a problem, that I have
> > reported a few weeks ago.
> > rpc.mountd can be blocked by a bad client, that sends lots of
> > RPC requests, but never reads the replies from the socket either
> > intentionally or e.g. caused by a wrong configured MTU.
> >
> > While looking for a possible solution, I found another weakness
> > in rpc.mountd if it is used "multithreaded" (-t nn).
> >
> > The first two patches fix that weakness in the case of !HAVE_LIBTIRPC
> > and HAVE_LIBTIRPC.
> > The third patch more a kind of suggestion how the main problem could
> > be fixed. I don't know whether we can set MAXREC without causing
> > new troubles. When this patch is used, a further patch for libtirpc
> > also should be used. You can find it here:
> > http://sourceforge.net/p/libtirpc/mailman/libtirpc-devel/?viewmonth=201409
>
> Over the weekend I took a good hard look at these 3 patches and
> the one for libtirpc with the reproducer Bodo supplied me.
> (Bodo, thanks all the help getting things set up!).
>
> I agree with the libtirpc patch so its been committed,
> but other three made me nervous so I wanted to take a
> good hard look at them. (Neil's question, "do we really
> what to make these non-block" haunted me ;-) ).
>
> It turns out, at least in my setup, these patches do not
> and can not stop the mountd DOS that Bodo's reproducer
> creates.
>
> Here is why they do not work:
>
> The fd that the write() is getting hung on, is not be created
> by the 3 nfs_svc_create() calls in mountd:main(). Its
> being created by the accept() when the tcp connection
> is created (via SVC_RECV()), so making those fds coming
> out of the nfs_svc_create() non-blocking does nothing.
Just to add to what Bodo replied:
SVC_RECV is a macro which calls .xp_recv, which in the TCP listen
case is rendezvous_request.
rendezvous_request does the accept:
if ((sock = accept(xprt->xp_fd, (struct sockaddr *)(void *)&addr,
&len)) < 0) {
and then
if (cd->maxrec != 0) {
flags = fcntl(sock, F_GETFL, 0);
if (flags == -1)
return (FALSE);
if (fcntl(sock, F_SETFL, flags | O_NONBLOCK) == -1)
return (FALSE);
so if cd->maxrec is not zero, O_NONBLOCK get set.
It gets set through an SVC_CONTROL -> xp_control -> svc_vc_rendezvous_control
call, which no-one ever makes, and it initialised to __svc_maxrec, which is
set by the rpc_control() call that Bodo mentions.
So providing rpc_control is called before the service is created, it should
work fine.
I wonder why it didn't work for you...
Thanks,
NeilBrown
>
> Here is the hang can't be stop (for now):
>
> Here is the stack:
> Stack trace of thread 4150:
> #0 0x00000036816f0e90 write (libc.so.6)
> #1 0x00007fd53837da6d write_vc (libtirpc.so.1)
> #2 0x00007fd53838103f flush_out (libtirpc.so.1)
> #3 0x00007fd53837dd81 svc_vc_reply (libtirpc.so.1)
> #4 0x00007fd53837b096 svcerr_noprog (libtirpc.so.1)
> #5 0x00007fd53837b360 svc_getreq_common (libtirpc.so.1)
> #6 0x00000000004086d9 my_svc_getreqset (mountd)
> #7 0x0000000000403d80 main (mountd)
> #8 0x000000368161ffe0 __libc_start_main (libc.so.6)
> #9 0x00000000004040f5 _start (mountd)
>
> The fd that created by the SVC_RECV() is never
> given back to the app (in case mountd) to make
> it non-blocking. Its used in the error path
> where it get hung up in the write(). The
> app has no control over that.
>
> Now I definitely see a problem, but these
> patches (with the exception of the libtirpc
> patch) don't address the problem I am seeing.
>
> So unless I'm not seeing something, I'm not in
> favor of taking these 3 patches.
>
> thoughts?
>
> steved.