Commit Graph

219 Commits

Author SHA1 Message Date
Mark Johnston
7fabaac221 rpc: Convert an SOLISTENING check to an assertion
Per the comment, this socket should always be a listening socket.

MFC after:	1 week
Sponsored by:	The FreeBSD Foundation
2021-09-17 14:19:05 -04:00
Gordon Bergling
631504fb34 Fix a common typo in source code comments
- s/existant/existent/

MFC after:	3 days
2021-09-04 12:56:57 +02:00
Mark Johnston
20d728b559 rpc: Make function tables const
No functional change intended.

MFC after:	1 week
Sponsored by:	The FreeBSD Foundation
2021-08-14 11:26:12 -04:00
Mark Johnston
f4bb1869dd Consistently use the SOLISTENING() macro
Some code was using it already, but in many places we were testing
SO_ACCEPTCONN directly.  As a small step towards fixing some bugs
involving synchronization with listen(2), make the kernel consistently
use SOLISTENING().  No functional change intended.

MFC after:	1 week
Sponsored by:	The FreeBSD Foundation
2021-06-14 17:32:27 -04:00
Rick Macklem
e1a907a25c krpc: Acquire ref count of CLIENT for backchannel use
Michael Dexter <editor@callfortesting.org> reported
a crash in FreeNAS, where the first argument to
clnt_bck_svccall() was no longer valid.
This argument is a pointer to the callback CLIENT
structure, which is free'd when the associated
NFSv4 ClientID is free'd.

This appears to have occurred because a callback
reply was still in the socket receive queue when
the CLIENT structure was free'd.

This patch acquires a reference count on the CLIENT
that is not CLNT_RELEASE()'d until the socket structure
is destroyed. This should guarantee that the CLIENT
structure is still valid when clnt_bck_svccall() is called.
It also adds a check for closed or closing to
clnt_bck_svccall() so that it will not process the callback
RPC reply message after the ClientID is free'd.

Comments by:	mav
MFC after:	2 weeks
Differential Revision:	https://reviews.freebsd.org/D30153
2021-06-11 16:57:14 -07:00
Rick Macklem
984c71f903 nfsd: Fix the failure return for non-fh NFSv4 operations
Without this patch, nfsd_checkrootexp() returns failure
and then the NFSv4 operation would reply NFSERR_WRONGSEC.
RFC5661 Sec. 2.6 only allows a few NFSv4 operations, none
of which call nfsv4_checktootexp(), to return NFSERR_WRONGSEC.
This patch modifies nfsd_checkrootexp() to return the
error instead of a boolean and sets the returned error to an RPC
layer AUTH_ERR, as discussed on nfsv4@ietf.org.
The patch also fixes nfsd_errmap() so that the pseudo
error NFSERR_AUTHERR is handled correctly such that an RPC layer
AUTH_ERR is replied to the NFSv4 client.

The two new "enum auth_stat" values have not yet been assigned
by IANA, but are the expected next two values.

The effect on extant NFSv4 clients of this change appears
limited to reporting a different failure error when a
mount that does not use adequate security is attempted.

MFC after:	2 weeks
2021-06-02 15:28:07 -07:00
Mark Johnston
ba5bc6e8f9 rpcsec_gss: Use a designated initializer for rpc_gss_ops
No functional change intended.

MFC after:	1 week
Sponsored by:	The FreeBSD Foundation
2021-05-26 10:45:40 -04:00
Rick Macklem
db8c27f499 nfsd: fix a NFSv4.1 Linux client mount stuck in CLOSE_WAIT
It was reported that a NFSv4.1 Linux client mount against
a FreeBSD12 server was hung, with the TCP connection in
CLOSE_WAIT state on the server.
When a NFSv4.1/4.2 mount is done and the back channel is
bound to the TCP connection, the soclose() is delayed until
a new TCP connection is bound to the back channel, due to
a reference count being held on the SVCXPRT structure in
the krpc for the socket. Without the soclose() call, the socket
will remain in CLOSE_WAIT and this somehow caused the Linux
client to hang.

This patch adds calls to soshutdown(.., SHUT_WR) that
are performed when the server side krpc sees that the
socket is no longer usable.  Since this can be done
before the back channel is bound to a new TCP connection,
it allows the TCP connection to proceed to CLOSED state.

PR:	254590
Reported by:	jbreitman@tildenparkcapital.com
Reviewed by:	tuexen
Comments by:	kevans
MFC after:	2 weeks
Differential Revision:	https://reviews.freebsd.org/D29526
2021-04-27 15:32:35 -07:00
Rick Macklem
7763814fc9 nfsv4 client: do the BindConnectionToSession as required
During a recent testing event, it was reported that the NFSv4.1/4.2
server erroneously bound the back channel to a new TCP connection.
RFC5661 specifies that the fore channel is implicitly bound to a
new TCP connection when an RPC with Sequence (almost any of them)
is done on it.  For the back channel to be bound to the new TCP
connection, an explicit BindConnectionToSession must be done as
the first RPC on the new connection.

Since new TCP connections are created by the "reconnect" layer
(sys/rpc/clnt_rc.c) of the krpc, this patch adds an optional
upcall done by the krpc whenever a new connection is created.
The patch also adds the specific upcall function that does a
BindConnectionToSession and configures the krpc to call it
when required.

This is necessary for correct interoperability with NFSv4.1/NFSv4.2
servers when the nfscbd daemon is running.

If doing NFSv4.1/NFSv4.2 mounts without this patch, it is
recommended that the nfscbd daemon not be running and that
the "pnfs" mount option not be specified.

PR:	254840
Comments by:	asomers
MFC after:	2 weeks
Differential Revision:	https://reviews.freebsd.org/D29475
2021-04-11 14:34:57 -07:00
Rick Macklem
f6dc363f6d nfs-over-tls: handle res.gid.gid_val correctly for memory allocation
When the server side nfs-over-tls does an upcall to rpc.tlsservd(8)
for the handshake and the rpc.tlsservd "-u" command line option has
been specified, a list of gids may be returned.
The list will be returned in malloc'd memory pointed to by
res.gid.gid_val. To ensure the malloc occurs, res.gid.gid_val must
be NULL before the call. Then, the malloc'd memory needs to be free'd.
mem_free() just calls free(9), so a NULL pointer argument is fine
and a length argument == 0 is ok, since the "len" argument is not used.

This bug would have only affected nfs-over-tls and only when
rpc.tlsservd(8) is running with the "-u" command line option.
2021-01-12 13:59:52 -08:00
Rick Macklem
665b1365fe Add a new "tlscertname" NFS mount option.
When using NFS-over-TLS, an NFS client can optionally provide an X.509
certificate to the server during the TLS handshake.  For some situations,
such as different NFS servers or different certificates being mapped
to different user credentials on the NFS server, there may be a need
for different mounts to provide different certificates.

This new mount option called "tlscertname" may be used to specify a
non-default certificate be provided.  This alernate certificate will
be stored in /etc/rpc.tlsclntd in a file with a name based on what is
provided by this mount option.
2020-12-23 13:42:55 -08:00
Rick Macklem
22f085c43b Fix a potential memory leak in the NFS over TLS handling code.
For the TLS case where there is a "user@domain" name specified in the
X.509 v3 certificate presented by the client in the otherName component
of subjectAltName, a gid list is allocated via mem_alloc().
This needs to be free'd. Otherwise xp_gidp == NULL and free() handles that.
(The size argument to mem_free() is not used by FreeBSD, so it can be 0.)

This leak would not have occurred for any other case than NFS over TLS
with the "user@domain" in the client's certificate.
2020-09-05 00:50:52 +00:00
Mitchell Horne
51bb2fccfd Remove a duplicate declaration
This is already declared in sys/file.h, which is included directly.
Compiling with GCC9 emits an error.

Discussed with: rmacklem
2020-09-03 22:40:51 +00:00
Rick Macklem
ab0c29af05 Add TLS support to the kernel RPC.
An internet draft titled "Towards Remote Procedure Call Encryption By Default"
describes how TLS is to be used for Sun RPC, with NFS as an intended use case.
This patch adds client and server support for this to the kernel RPC,
using KERN_TLS and upcalls to daemons for the handshake, peer reset and
other non-application data record cases.

The upcalls to the daemons use three fields to uniquely identify the
TCP connection. They are the time.tv_sec, time.tv_usec of the connection
establshment, plus a 64bit sequence number. The time fields avoid problems
with re-use of the sequence number after a daemon restart.
For the server side, once a Null RPC with AUTH_TLS is received, kernel
reception on the socket is blocked and an upcall to the rpctlssd(8) daemon
is done to perform the TLS handshake.  Upon completion, the completion
status of the handshake is stored in xp_tls as flag bits and the reply to
the Null RPC is sent.
For the client, if CLSET_TLS has been set, a new TCP connection will
send the Null RPC with AUTH_TLS to initiate the handshake.  The client
kernel RPC code will then block kernel I/O on the socket and do an upcall
to the rpctlscd(8) daemon to perform the handshake.
If the upcall is successful, ct_rcvstate will be maintained to indicate
if/when an upcall is being done.

If non-application data records are received, the code does an upcall to
the appropriate daemon, which will do a SSL_read() of 0 length to handle
the record(s).

When the socket is being shut down, upcalls are done to the daemons, so
that they can perform SSL_shutdown() calls to perform the "peer reset".

The rpctlssd(8) and rpctlscd(8) daemons require a patched version of the
openssl library and, as such, will not be committed to head at this time.

Although the changes done by this patch are fairly numerous, there should
be no semantics change to the kernel RPC at this time.
A future commit to the NFS code will optionally enable use of TLS for NFS.
2020-08-22 03:57:55 +00:00
Rick Macklem
02511d2112 Add an argument to newnfs_connect() that indicates use TLS for the connection.
For NFSv4.0, the server creates a server->client TCP connection for callbacks.
If the client mount on the server is using TLS, enable TLS for this callback
TCP connection.
TLS connections from clients will not be supported until the kernel RPC
changes are committed.

Since this changes the internal ABI between the NFS kernel modules that
will require a version bump, delete newnfs_trimtrailing(), which is no
longer used.

Since LCL_TLSCB is not yet set, these changes should not have any semantic
affect at this time.
2020-08-11 00:26:45 +00:00
Rick Macklem
b94b9a80b2 Fix up a comment added by r362455. 2020-06-21 02:49:56 +00:00
Rick Macklem
4302e8b671 Modify the way the client side krpc does soreceive() for TCP.
Without this patch, clnt_vc_soupcall() first does a soreceive() for
4 bytes (the Sun RPC over TCP record mark) and then soreceive(s) for
the RPC message.
This first soreceive() almost always results in an mbuf allocation,
since having the 4byte record mark in a separate mbuf in the socket
rcv queue is unlikely.
This is somewhat inefficient and rather odd. It also will not work
for the ktls rx, since the latter returns a TLS record for each
soreceive().

This patch replaces the above with code similar to what the server side
of the krpc does for TCP, where it does a soreceive() for as much data
as possible and then parses RPC messages out of the received data.
A new field of the TCP socket structure called ct_raw is the list of
received mbufs that the RPC message(s) are parsed from.
I think this results in cleaner code and is needed for support of
nfs-over-tls.
It also fixes the code for the case where a server sends an RPC message
in multiple RPC message fragments. Although this is allowed by RFC5531,
no extant NFS server does this. However, it is probably good to fix this
in case some future NFS server does do this.
2020-06-21 00:06:04 +00:00
Rick Macklem
c19cba61e9 Add the .h file that describes the operations for the rpctls_syscall.
This .h file will be used by the nfs-over-tls daemons to do the system
call that was added by r361599.
2020-05-31 01:12:52 +00:00
Gleb Smirnoff
732a02b4e7 Split XDR into separate kernel module. Make krpc depend on xdr.
Reviewed by:	rmacklem
Differential Revision:	https://reviews.freebsd.org/D24408
2020-04-17 06:04:20 +00:00
Gleb Smirnoff
e5c3941009 Move M_RPC malloc type into XDR. Both RPC and XDR libraries use
this type, but since RPC depends on XDR (not vice versa) we need
it defined in XDR to make the module loadable without RPC.

Reviewed by:	rmacklem
Differential Revision:	https://reviews.freebsd.org/D24408
2020-04-17 06:02:13 +00:00
Rick Macklem
9c2065607f Change the xid for client side krpc over UDP to a global value.
Without this patch, the xid used for the client side krpc requests over
UDP was initialized for each "connection". A "connection" for UDP is
rather sketchy and for the kernel NLM a new one is created every 2minutes.
A problem with client side interoperability with a Netapp server for the NLM
was reported and it is believed to be caused by reuse of the same xid.
Although this was never completely diagnosed by the reporter, I could see
how the same xid might get reused, since it is initialized to a value
based on the TOD clock every two minutes.
I suspect initializing the value for every "connection" was inherited from
userland library code, where having a global xid was not practical.
However, implementing a global "xid" for the kernel rpc is straightforward
and will ensure that an xid value is not reused for a long time. This
patch does that and is hoped it will fix the Netapp interoperability
problem.

PR:		245022
Reported by:	danny@cs.huji.ac.il
MFC after:	2 weeks
2020-04-05 21:08:17 +00:00
Warner Losh
713bd56728 Remove obsolete old-freebsd version compat shim. 2020-03-01 23:01:51 +00:00
Pawel Biernacki
7029da5c36 Mark more nodes as CTLFLAG_MPSAFE or CTLFLAG_NEEDGIANT (17 of many)
r357614 added CTLFLAG_NEEDGIANT to make it easier to find nodes that are
still not MPSAFE (or already are but aren’t properly marked).
Use it in preparation for a general review of all nodes.

This is non-functional change that adds annotations to SYSCTL_NODE and
SYSCTL_PROC nodes using one of the soon-to-be-required flags.

Mark all obvious cases as MPSAFE.  All entries that haven't been marked
as MPSAFE before are by default marked as NEEDGIANT

Approved by:	kib (mentor, blanket)
Commented by:	kib, gallatin, melifaro
Differential Revision:	https://reviews.freebsd.org/D23718
2020-02-26 14:26:36 +00:00
Rick Macklem
841c3621b4 Change r355157 to make svc_rpc_gss_lifetime_max a static.
MFC after:	2 weeks
2019-11-28 02:18:51 +00:00
Rick Macklem
04cb0c38eb Add a cap on credential lifetime for Kerberized NFS.
The kernel RPCSEC_GSS code sets the credential (called a client) lifetime
to the lifetime of the Kerberos ticket, which is typically several hours.
As such, when a user's credentials change such as being added to a new group,
it can take several hours for this change to be recognized by the NFS server.
This patch adds a sysctl called kern.rpc.gss.lifetime_max which can be set
by a sysadmin to put a cap on the time to expire for the credentials, so that
a sysadmin can reduce the timeout.
It also fixes a bug, where time_uptime is added twice when GSS_C_INDEFINITE
is returned for a lifetime. This has no effect in practice, sine Kerberos
never does this.

Tested by:	pen@lysator.liu.se
PR:		242132
Submitted by:	pen@lysator.liu.se
MFC after:	2 weeks
2019-11-28 02:05:31 +00:00
Mark Johnston
918988576c Avoid relying on header pollution from sys/refcount.h.
MFC after:	3 days
Sponsored by:	The FreeBSD Foundation
2019-07-29 20:26:01 +00:00
Rick Macklem
52cab12c09 Fix malloc stats for the RPCSEC_GSS server code when DEBUG is enabled.
The code enabled when "DEBUG" is defined uses mem_alloc(), which is a
malloc(.., M_RPC, M_WAITOK | M_ZERO), but then calls gss_release_buffer()
which does a free(.., M_GSSAPI) to free the memory.
This patch fixes the problem by replacing mem_alloc() with a
malloc(.., M_GSSAPI, M_WAITOK | M_ZERO).
This bug affects almost no one, since the sources are not normally built
with "DEBUG" defined.

Submitted by:	peter@ifm.liu.se
MFC after:	2 weeks
2019-04-04 01:23:06 +00:00
Rick Macklem
1406895958 Add a comment to the r345818 patch to explain why cl_refs is initialized to 2.
PR:		235582
MFC after:	2 weeks
2019-04-03 03:50:16 +00:00
Rick Macklem
b0e14530a0 Fix a race in the RPCSEC_GSS server code that caused crashes.
When a new client structure was allocated, it was added to the list
so that it was visible to other threads before the expiry time was
initialized, with only a single reference count.
The caller would increment the reference count, but it was possible
for another thread to decrement the reference count to zero and free
the structure before the caller incremented the reference count.
This could occur because the expiry time was still set to zero when
the new client structure was inserted in the list and the list was
unlocked.

This patch fixes the race by initializing the reference count to two
and initializing all fields, including the expiry time, before inserting
it in the list.

Tested by:	peter@ifm.liu.se
PR:		235582
MFC after:	2 weeks
2019-04-02 23:51:08 +00:00
Edward Tomasz Napierala
e998861bbb Bump the default kern.rpc.gss.client_max from 128 to 1024.
The old value resulted in bad performance, with high kernel
and gssd(8) load, with more than ~64 clients; it also triggered
crashes, which are to be fixed by a different patch.

PR:		235582
Discussed with:	rmacklem@
MFC after:	2 weeks
2019-02-19 11:07:02 +00:00
Edward Tomasz Napierala
52eb49951a Add kern.rpc.gss.client_hash tunable, to make it possible to bump
it easily.  This can lower the load on gssd(8) on large NFS servers.

Submitted by:	Per Andersson <pa at chalmers dot se>
Reviewed by:	rmacklem@
MFC after:	2 weeks
Sponsored by:	Chalmers University of Technology
2019-02-19 10:17:49 +00:00
Edward Tomasz Napierala
b329fb2885 Add kern.rpc.gss.client_max, to make it possible to bump it easily.
This can drastically lower the load on gssd(8) on large NFS servers.

Submitted by:	Per Andersson <pa at chalmers dot se>
Reviewed by:	rmacklem@
MFC after:	2 weeks
Sponsored by:	Chalmers University of Technology
Differential Revision:	https://reviews.freebsd.org/D18393
2018-12-15 11:32:11 +00:00
Rick Macklem
1a59bccc42 Set SO_SNDTIMEO in the client side krpc when CLSET_TIMEOUT is done.
During testing of the pNFS client, it was observed that an RPC could get
stuck in sosend() for a very long time if the network connection to a DS
had failed. This is fixed by setting SO_SNDTIMEO on the TCP socket.
This is only done when CLSET_TIMEOUT is done and this is not done by any
use of the krpc currently in the source tree, so there should be no effect
on extant uses.
A future patch will use CLSET_TIMEOUT for TCP connections to DSs.

Reviewed by:	kib
MFC after:	2 weeks
Differential Revision:	https://reviews.freebsd.org/D16293
2018-07-20 12:03:16 +00:00
Rick Macklem
1b09d9df3d Fix the server side krpc so that the kernel nfsd threads terminate.
Occationally the kernel nfsd threads would not terminate when a SIGKILL
was posted for the kernel process (called nfsd (slave)). When this occurred,
the thread associated with the process (called "ismaster") had returned from
svc_run_internal() and was sleeping waiting for the other threads to terminate.
The other threads (created by kthread_start()) were still in svc_run_internal()
handling NFS RPCs.
The only way this could occur is for the "ismaster" thread to return from
svc_run_internal() without having called svc_exit().
There was only one place in the code where this could happen and this patch
stops that from happening.
Since the problem is intermittent, I cannot be sure if this has fixed the
problem, but I have not seen an occurrence of the problem with this patch
applied.

Reviewed by:	kib
MFC after:	2 weeks
Differential Revision:	https://reviews.freebsd.org/D16087
2018-07-02 17:50:46 +00:00
Alexander Kabaev
151ba7933a Do pass removing some write-only variables from the kernel.
This reduces noise when kernel is compiled by newer GCC versions,
such as one used by external toolchain ports.

Reviewed by: kib, andrew(sys/arm and sys/arm64), emaste(partial), erj(partial)
Reviewed by: jhb (sys/dev/pci/* sys/kern/vfs_aio.c and sys/kern/kern_synch.c)
Differential Revision: https://reviews.freebsd.org/D10385
2017-12-25 04:48:39 +00:00
Pedro F. Giffuni
fe267a5590 sys: general adoption of SPDX licensing ID tags.
Mainly focus on files that use BSD 2-Clause license, however the tool I
was using misidentified many licenses so this was mostly a manual - error
prone - task.

The Software Package Data Exchange (SPDX) group provides a specification
to make it easier for automated tools to detect and summarize well known
opensource licenses. We are gradually adopting the specification, noting
that the tags are considered only advisory and do not, in any way,
superceed or replace the license texts.

No functional change intended.
2017-11-27 15:23:17 +00:00
Pedro F. Giffuni
51369649b0 sys: further adoption of SPDX licensing ID tags.
Mainly focus on files that use BSD 3-Clause license.

The Software Package Data Exchange (SPDX) group provides a specification
to make it easier for automated tools to detect and summarize well known
opensource licenses. We are gradually adopting the specification, noting
that the tags are considered only advisory and do not, in any way,
superceed or replace the license texts.

Special thanks to Wind River for providing access to "The Duke of
Highlander" tool: an older (2014) run over FreeBSD tree was useful as a
starting point.
2017-11-20 19:43:44 +00:00
Gleb Smirnoff
779f106aa1 Listening sockets improvements.
o Separate fields of struct socket that belong to listening from
  fields that belong to normal dataflow, and unionize them.  This
  shrinks the structure a bit.
  - Take out selinfo's from the socket buffers into the socket. The
    first reason is to support braindamaged scenario when a socket is
    added to kevent(2) and then listen(2) is cast on it. The second
    reason is that there is future plan to make socket buffers pluggable,
    so that for a dataflow socket a socket buffer can be changed, and
    in this case we also want to keep same selinfos through the lifetime
    of a socket.
  - Remove struct struct so_accf. Since now listening stuff no longer
    affects struct socket size, just move its fields into listening part
    of the union.
  - Provide sol_upcall field and enforce that so_upcall_set() may be called
    only on a dataflow socket, which has buffers, and for listening sockets
    provide solisten_upcall_set().

o Remove ACCEPT_LOCK() global.
  - Add a mutex to socket, to be used instead of socket buffer lock to lock
    fields of struct socket that don't belong to a socket buffer.
  - Allow to acquire two socket locks, but the first one must belong to a
    listening socket.
  - Make soref()/sorele() to use atomic(9).  This allows in some situations
    to do soref() without owning socket lock.  There is place for improvement
    here, it is possible to make sorele() also to lock optionally.
  - Most protocols aren't touched by this change, except UNIX local sockets.
    See below for more information.

o Reduce copy-and-paste in kernel modules that accept connections from
  listening sockets: provide function solisten_dequeue(), and use it in
  the following modules: ctl(4), iscsi(4), ng_btsocket(4), ng_ksocket(4),
  infiniband, rpc.

o UNIX local sockets.
  - Removal of ACCEPT_LOCK() global uncovered several races in the UNIX
    local sockets.  Most races exist around spawning a new socket, when we
    are connecting to a local listening socket.  To cover them, we need to
    hold locks on both PCBs when spawning a third one.  This means holding
    them across sonewconn().  This creates a LOR between pcb locks and
    unp_list_lock.
  - To fix the new LOR, abandon the global unp_list_lock in favor of global
    unp_link_lock.  Indeed, separating these two locks didn't provide us any
    extra parralelism in the UNIX sockets.
  - Now call into uipc_attach() may happen with unp_link_lock hold if, we
    are accepting, or without unp_link_lock in case if we are just creating
    a socket.
  - Another problem in UNIX sockets is that uipc_close() basicly did nothing
    for a listening socket.  The vnode remained opened for connections.  This
    is fixed by removing vnode in uipc_close().  Maybe the right way would be
    to do it for all sockets (not only listening), simply move the vnode
    teardown from uipc_detach() to uipc_close()?

Sponsored by:		Netflix
Differential Revision:	https://reviews.freebsd.org/D9770
2017-06-08 21:30:34 +00:00
Xin LI
6448ec89e7 * limit size of buffers to RPC_MAXDATASIZE
* don't leak memory
 * be more picky about bad parameters

From:

https://raw.githubusercontent.com/guidovranken/rpcbomb/master/libtirpc_patch.txt
https://github.com/guidovranken/rpcbomb/blob/master/rpcbind_patch.txt

via NetBSD.

Reviewed by:	emaste, cem (earlier version)
Differential Revision:	https://reviews.freebsd.org/D10922
MFC after:	3 days
2017-06-01 06:12:25 +00:00
Ed Maste
3e85b721d6 Remove register keyword from sys/ and ANSIfy prototypes
A long long time ago the register keyword told the compiler to store
the corresponding variable in a CPU register, but it is not relevant
for any compiler used in the FreeBSD world today.

ANSIfy related prototypes while here.

Reviewed by:	cem, jhb
Sponsored by:	The FreeBSD Foundation
Differential Revision:	https://reviews.freebsd.org/D10193
2017-05-17 00:34:34 +00:00
Rick Macklem
dfd174d6e0 Fix the client side krpc from doing TCP reconnects for ERESTART from sosend().
When sosend() replies ERESTART in the client side krpc, it indicates that
the RPC message hasn't yet been sent and that the send queue is full or
locked while a signal is posted for the process.
Without this patch, this would result in a RPC_CANTSEND reply from
clnt_vc_call(), which would cause clnt_reconnect_call() to create a new
TCP transport connection. For most NFS servers, this wasn't a serious problem,
although it did imply retries of outstanding RPCs, which could possibly
have missed the DRC.
For an NFSv4.1 mount to AmazonEFS, this caused a serious problem, since
AmazonEFS often didn't retain the NFSv4.1 session and would reply with
NFS4ERR_BAD_SESSION. This implies to the client a crash/reboot which
requires open/lock state recovery.

Three options were considered to fix this:
- Return the ERESTART all the way up to the system call boundary and then
  have the system call redone. This is fraught with risk, due to convoluted
  code paths, asynchronous I/O RPCs etc. cperciva@ worked on this, but it
  is still a work in prgress and may not be feasible.
- Set SB_NOINTR for the socket buffer. This fixes the problem, but makes
  the sosend() completely non interruptible, which kib@ considered
  inappropriate. It also would break forced dismount when a thread
  was blocked in sosend().
- Modify the retry loop in clnt_vc_call(), so that it loops for this case
  for up to 15sec. Testing showed that the sosend() usually succeeded by
  the 2nd retry. The extreme case observed was 111 loop iterations, or
  about 100msec of delay.
This third alternative is what is implemented in this patch, since the
change is:
- localized
- straightforward
- forced dismount is not broken by it.

This patch has been tested by cperciva@ extensively against AmazonEFS.

Reported by:	cperciva
Tested by:	cperciva
MFC after:	2 weeks
2017-05-07 12:12:45 +00:00
Rick Macklem
34f1fddb1e Fix a crash during unmount of an NFSv4.1 mount.
Larry Rosenman reported a crash on freebsd-current@ which was caused by
a premature release of the krpc backchannel socket structure.
I believe this was caused by a race between the SVC_RELEASE() in clnt_vc.c
and the xprt_unregister() in the higher layer (clnt_rc.c), which tried
to lock the mutex in the xprt structure and crashed.
This patch fixes this by removing the xprt_unregister() in the clnt_vc
layer and allowing this to always be done by the clnt_rc (higher reconnect
layer).

Reported by:	ler@lerctr.org
Tested by:	ler@letctr.org
MFC after:	2 weeks
2017-04-10 22:47:18 +00:00
Warner Losh
fbbd9655e5 Renumber copyright clause 4
Renumber cluase 4 to 3, per what everybody else did when BSD granted
them permission to remove clause 3. My insistance on keeping the same
numbering for legal reasons is too pedantic, so give up on that point.

Submitted by:	Jan Schaumann <jschauma@stevens.edu>
Pull Request:	https://github.com/freebsd/freebsd/pull/96
2017-02-28 23:42:47 +00:00
Andriy Gapon
90f90687b3 add svcpool_close to handle killed nfsd threads
This patch adds a new function to the server krpc called
svcpool_close().  It is similar to svcpool_destroy(), but does not free
the data structures, so that the pool can be used again.

This function is then used instead of svcpool_destroy(),
svcpool_create() when the nfsd threads are killed.

PR:		204340
Reported by:	Panzura
Approved by:	rmacklem
Obtained from:	rmacklem
MFC after:	1 week
2017-02-14 17:49:08 +00:00
Konstantin Belousov
584b675ed6 Hide the boottime and bootimebin globals, provide the getboottime(9)
and getboottimebin(9) KPI. Change consumers of boottime to use the
KPI.  The variables were renamed to avoid shadowing issues with local
variables of the same name.

Issue is that boottime* should be adjusted from tc_windup(), which
requires them to be members of the timehands structure.  As a
preparation, this commit only introduces the interface.

Some uses of boottime were found doubtful, e.g. NLM uses boottime to
identify the system boot instance.  Arguably the identity should not
change on the leap second adjustment, but the commit is about the
timekeeping code and the consumers were kept bug-to-bug compatible.

Tested by:	pho (as part of the bigger patch)
Reviewed by:	jhb (same)
Discussed with:	bde
Sponsored by:	The FreeBSD Foundation
MFC after:	1 month
X-Differential revision:	https://reviews.freebsd.org/D7302
2016-07-27 11:08:59 +00:00
Enji Cooper
789872f2e9 Don't test for xpt not being NULL before calling svc_xprt_free(..)
svc_xprt_alloc(..) will always return initialized memory as it uses
mem_alloc(..) under the covers, which uses malloc(.., M_WAITOK, ..).

MFC after: 1 week
Reported by: Coverity
CID: 1007341
Sponsored by: EMC / Isilon Storage Division
2016-07-11 07:24:56 +00:00
Enji Cooper
462984cb6f Convert svc_xprt_alloc(..) and svc_xprt_free(..)'s prototypes to
ANSI C style prototypes

MFC after: 1 week
Sponsored by: EMC / Isilon Storage Division
2016-07-11 07:17:52 +00:00
Enji Cooper
7d3db23544 Deobfuscate cleanup path in clnt_vc_create(..)
Similar to r300836, r301800, and r302550, cl and ct will always
be non-NULL as they're allocated using the mem_alloc routines,
which always use `malloc(..., M_WAITOK)`.

MFC after: 1 week
Reported by: Coverity
CID: 1007342
Sponsored by: EMC / Isilon Storage Division
2016-07-11 07:07:15 +00:00
Enji Cooper
f99529597c Deobfuscate cleanup path in clnt_dg_create(..)
Similar to r300836 and r301800, cl and cu will always be non-NULL as they're
allocated using the mem_alloc routines, which always use
`malloc(..., M_WAITOK)`.

Deobfuscating the cleanup path fixes a leak where if cl was NULL and
cu was not, cu would not be free'd, and also removes a duplicate test for
cl not being NULL.

MFC after: 1 week
Reported by: Coverity
CID: 1007033, 1007344
Sponsored by: EMC / Isilon Storage Division
2016-07-11 06:58:24 +00:00
Enji Cooper
ea85a1540a Deobfuscate cleanup path in clnt_bck_create(..)
Similar to r300836, cl and ct will always be non-NULL as they're allocated
using the mem_alloc routines, which always use `malloc(..., M_WAITOK)`.

Deobfuscating the cleanup path fixes a leak where if cl was NULL and
ct was not, ct would not be free'd, and also removes a duplicate test for
cl not being NULL.

Approved by: re (gjb)
Differential Revision: https://reviews.freebsd.org/D6801
MFC after: 1 week
Reported by: Coverity
CID: 1229999
Reviewed by: cem
Sponsored by: EMC / Isilon Storage Division
2016-06-10 17:53:28 +00:00