Trishali #23
Open
Trishali #23
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: Jeff Layton <[email protected]>
Signed-off-by: Jeff Layton <[email protected]>
An xprt has a ref for the hash table (that's released by SVC_DESTROY()); but when it's first created, only 1 ref was taken, so there wasn't a ref for the caller. Add an extra ref for the caller when the xprt is first created. Signed-off-by: Daniel Gryniewicz <[email protected]>
svc_xprt_lookup - Add extra ref on create
The check to see if the given address will fit in the storage is backwards. Fix it, so that IPv4 can fit in the storage, and so that a huge address doesn't overflow. Signed-off-by: Daniel Gryniewicz <[email protected]>
timespec[add|sub|cmp] are now public in FreeBSD 12, they now have an additional variable similar to NetBSD. We use the system macros by default, otherwise the updated version has been implemented. Signed-off-by: Jack Halford <[email protected]>
Update timespec macros
Make URCU a mandatory dependency
Fix addr size check in clnt_dg_control()
When _LGPL_SOURCE is defined the lttng headers will inline some of the RCU functions with the "bulletproof" functions. This means that we can't currently select a urcu flavor at runtime that will be used universally. I'm a little unclear on what happens if you mix different flavored urcu calls, but I doubt that it's anything good. Let's switch back to using urcu-bp universally for now until this has a better solution. It's not ideal for performance, but our use of URCU is pretty minimal for now, and it's better to be safe than fast. Signed-off-by: Jeff Layton <[email protected]>
Force urcu-bp for now
XPRT refcounting could leak a ref in some cases. Fix this by dropping the extra ref in error cases, and by passing the event ref (which should have been dropped, because events are oneshot) to the event processor. This fixes the FD depletion while doing mount/unmount in a tight loop. Signed-off-by: Daniel Gryniewicz <[email protected]>
TCP has a double-ref setup, where the tree has one, and the event loop has one. This allows processors to destroy the xprt on error without freeing it, ensuring no use-after-free. UDP does not have a XPRT tree, since it re-uses a single socket, so it only has one ref. This means that destroy-on-error immediately frees. Add an extra ref to the UDP path so that it matches TCP, and to allow destroy. Signed-off-by: Daniel Gryniewicz <[email protected]>
When destroying a client, the XPRT is not also destroyed, but just unref'd. This is fine, if multiple clients share a XPRT, but local clients all have unique FDs, and therefore unique XPRTs. This means that their XPRTs are never destroyed. Ganesha uses these a lot, leading to many leaked XPRTs and therefore FDs. Add a flag on a client that indicates it's local, and use that flag to desrtoy the XPRT when the client is destroyed. Signed-off-by: Daniel Gryniewicz <[email protected]>
XPRT Refcount fixes
Signed-off-by: Frank S. Filz <[email protected]>
Instead of calling request_cb which alllocates an svc_req and then calls SVC_DECODE and then frees the request, we call alloc_cb, call SVC_DECODE ourselves, and then call free_cb. This makes it a little easier to handle suspending an async request in the future. Signed-off-by: Frank S. Filz <[email protected]>
If the protocol layer needs to save some data (it probably does), it should use SVCXPRT->xp_u1 or SVCXPRT->xp_u2 (Ganesha only uses xp_u2 so it could use xp_u1). If a request is to be suspended, the SVCXPRT and svc_req should not be used once it is possible for another thread to be processing an async completion of the request. This is because if such an async completion happened to run before the original thread returned up the stack, the request could actually be requeued and even executing on another RPC thread while the first thread is still unwinding. Signed-off-by: Frank S. Filz <[email protected]>
Fix security warnings from AppScan tool
Convert unsafe strncpy to strlcpy
Signed-off-by: Daniel Gryniewicz <[email protected]>
Make a header for strl*()
Coverity Scan fix: In __rpc_taddr2uaddr_af() free the allocated memory for variable 'ret' when returning NULL from the function. Signed-off-by: Madhu Thorat <[email protected]>
Free 'ret' when returning NULL in __rpc_taddr2uaddr_af()
On modern Linux, statd only listens on TCP. Forcing UDP for connections causes them to time out. Any system that claims to support TCP but doesn't is so out-of-date we probably have other issues there. Fix it so that TCP actually uses TCP. Signed-off-by: Daniel Gryniewicz <[email protected]>
NSM - Don't force UDP portmapper lookups
We are doing accept and returning XPRT_DIED without closing fd, which will be unmonitored and cause fd leak. Client will see connection succeded but IO will hang on it.
Close fd if makefd_xprt fails.
"gss_sign" is a deprecated GSS-API function. This needs to be replaced by "gss_get_mic". Signed-off-by: Sachin Punadikar <[email protected]>
Currently in _svcauth_gss() after having a valid 'gd' we increment 'gd->refcnt' and set 'req->rq_auth=gd->auth'. In case of failure for a valid 'gd' we call unref_svc_rpc_gss_data() which decrements 'gd->refcnt' and may destroy 'gd' if 'gd->refcnt' becomes 0. But later on when free_nfs_request() is called, as 'req->rq_auth' is still valid, we call SVCAUTH_RELEASE() which either leads to a crash if 'gd' was already freed or if 'gd' was not freed then we decrement 'gd->refcnt' for the second time. Fixed this by setting 'req->rq_auth=NULL' in case of failure in _svcauth_gss(). Signed-off-by: Madhu Thorat <[email protected]>
In _svcauth_gss() in case of failure set 'req->rq_auth' to NULL
If a vector includes some partially filled buffers, setpos must position into the next buffer if it would land at or past the tail of a buffer that is not the terminal buffer. This will be required if putbufs is made to work. It is also required for gss_wrap_iov where we need to insert a header just before the response data after the response data has already been encoded. This means the response data MUST start in a new buffer, leaving the previous buffer potentially partially full. Signed-off-by: Frank S. Filz <[email protected]>
Add four new interfaces to xdr_ops XDR_NEWBUFS - forces a vector xdr like xdr_ioq to start a new buffer so that later a GSS HEADER buffer can be inserted. XDR_IOVCOUNT - counts the number of xdr_iov buffers occupied in an xdr stream from pos for datalen bytes. XDR_FILLBUFS - fills a new xdr_iov vector with the buffer pointers to an xdr stream from pos for datalen bytes. XDR_ALLOCHDRS - allocates space in an xdr stream for HEADER and TRAILER buffers. If the xdr allows, a HEADER buffer may be inserted just before pos. All xdr types should be able to support appending TRAILER buffers. Signed-off-by: Frank S. Filz <[email protected]>
This makes it easy to serialize writes for a given SVCXPRT. This will eventually be compressed into the single non-blocking patch. Signed-off-by: Frank S. Filz <[email protected]>
This should have actually been removed by this commit: d16697b Simplify epoll task processing Signed-off-by: Frank S. Filz <[email protected]>
The xdr_ioq keeps track of the progress of send and if any calls to sendmsg ever would have blocked. sendmsg will send as much data as it can without blocking, and then a subsequent call to send more data will likely return EAGAIN or EWOULDBLOCK to indicate it might block. A pointer to the duplex record is also kept for ease of finding it when the epoll event to unblock is triggered. Each duplex record now has two events, one for receive and one for send. It also has a pointer to the in progress send xdr_ioq. The SVCXPRT has a duplicate xp_fd_send to be used along with event_send to allow epoll for both receive and waiting for send to unblock. svc_ioq_flushv has the most changes to manage the conversion to sendmsg and dealing with partial write of the xdr_ioq. With the possibility of blocked send xdr_ioq, the writeq must be managed differently. Every xdr_ioq is queued into the writeq, but before doing so we remember if the writeq was empty so we can either immediately start working on the send (svc_ioq_write_now) or submit a work pool task (svc_ioq_write_submit). When the send unblocks, svc_rqst_xprt_task_send directly calls svc_ioq_write since the blocked send is at the head of the writeq. Signed-off-by: Daniel Gryniewicz <[email protected]> Signed-off-by: Frank S. Filz <[email protected]>
gss_unwrap_iov still requires a single buffer, but at least it can decrypt in place. gss_mic_verify_iov requires the MIC token to be in a single buffer. If it is not, we extract it, othwewise we use it in place. The data may be in one or more buffers. If for some reason xdr_rpc_gss_wrap is called for PRIVACY on an xdr that does not support XDR_NEBUFS, it will use the old code which makes a copy of the data with a separate wrapbuf and then copies it back into the xdr stream. Since gss_get_mic_iov doesn't need to insert a header, it can work with streams that don't support XDR_NEWBUF since it will just append the MIC token at the end (and conceivably there might be a vector xdr that would put that in a new buffer even though it can't support XDR_NEWBUFS to insert a HEADER buffer). Signed-off-by: Frank S. Filz <[email protected]>
Signed-off-by: Frank S. Filz <[email protected]>
Nonblocking and gss_iov
Some callers need to try and append to an xdr stream and if the data doesn't fit, back up. These callers expect an error to occur so let's not throw worrying messages into the log. Signed-off-by: Frank S. Filz <[email protected]>
Remove error messages from several xdr functions
Signed-off-by: Daniel Gryniewicz <[email protected]>
In several error cases, it returned the wrong thing. Fix all cases to return 0 on success, <0 on failure, or EWOULDBLOCK if blocked. Signed-off-by: Daniel Gryniewicz <[email protected]>
Nonblocking write was missing a ref in the case where the first entry in the queue finished, causing the epoll to be unhooked. If the next entry blocked, then epoll is hooked again, but the hook path didn't take a ref, causing an extra unref later. This destroys the xprt, closing the connection, and hanging the client. Signed-off-by: Daniel Gryniewicz <[email protected]>
The new API for requests is asymmetric; that is, it's expected that the alloc callback will take a ref on the xprt, but svc_request() frees that. Fix this so that it's expected that the free callback will release this ref. Requires a fix in Ganesha. Signed-off-by: Daniel Gryniewicz <[email protected]>
xprt refcount related fixes
Signed-off-by: Yang Ruifeng <[email protected]>
Signed-off-by: Frank S. Filz <[email protected]>
This doesn't change the code execution really but makes the debug look nicer. The VIO_TRAILER_LEN is not included in the gss_iov. Signed-off-by: Frank S. Filz <[email protected]>
When calling allochdrs(), it needs to update the posision to the end of the XDR, or else only partial data is sent. The position had been previously updated to the location of the length field. Signed-off-by: Daniel Gryniewicz <[email protected]> Signed-off-by: Frank S. Filz <[email protected]>
SVC IOQ - Dequeue the xioq before process
Signed-off-by: Frank S. Filz <[email protected]>
Oops - need to SETPOS just to totlen
Signed-off-by: Daniel Gryniewicz <[email protected]>
rc1 was tagged, but did not update the version, so set to rc2 so we can tag. Signed-off-by: Daniel Gryniewicz <[email protected]>
Signed-off-by: Daniel Gryniewicz <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.